{"thread":{"id":"59823","subject":"Bug: 'git stash --staged' crashes with binary files","startedAt":"2023-06-01T21:26:50Z","lastAt":"2023-10-11T18:23:21Z","messageCount":3,"participants":["Adam Johnson","Jeff King","rsbecker@nexbridge.com"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"477902","messageId":"CAMyDDM3DFyru6zph4qqf_QoaOeezvYkT7SmwinCfJNnFsnuRjQ@mail.gmail.com","threadId":"59823","inReplyTo":null,"subject":"Bug: 'git stash --staged' crashes with binary files","fromName":"Adam Johnson","fromEmail":"me@adamj.eu","sentAt":"2023-06-01T21:26:13Z","receivedAt":"2023-06-01T21:26:50Z","isPatch":false,"sender":{"key":"me@adamj.eu","avatar":"https://avatars.githubusercontent.com/u/857609?v=4"},"body":"Stage a binary file and run 'git stash --staged'. The stash is created but\nthe command fails to remove changes from the index:\n\n$ echo -n \"\\0\" >file\n\n$ git add file\n\n$ git stash --staged\nSaved working directory and index state WIP on main: e7911b6 Whatever\nerror: cannot apply binary patch to 'file' without full index line\nerror: file: patch does not apply\nCannot remove worktree changes\n\n$ git status\n...\nChanges to be committed:\n        new file:   file\n\nThe \"remove from the index\" step seems not to be binary-compatible.\n\nThe below patch adds a reproducing test.\n\ndiff --git t/t3903-stash.sh t/t3903-stash.sh\nindex 376cc8f4ab..5e3ea64f38 100755\n--- t/t3903-stash.sh\n+++ t/t3903-stash.sh\n@@ -392,6 +392,13 @@ setup_stash()\n     git stash pop &&\n     test bar,bar4 = $(cat file),$(cat file2)\n '\n+test_expect_success 'stash --staged with binary file' '\n+    echo -n \"\\0\" >file &&\n+    git add file &&\n+    git stash --staged &&\n+    git stash pop &&\n+    test \"\\0\" = $(cat file)\n+'\n\n test_expect_success 'dont assume push with non-option args' '\n     test_must_fail git stash -q drop 2>err &&\n"},{"id":"483066","messageId":"20231011181733.GA514244@coredump.intra.peff.net","threadId":"59823","inReplyTo":"CAMyDDM3DFyru6zph4qqf_QoaOeezvYkT7SmwinCfJNnFsnuRjQ@mail.gmail.com","subject":"Re: Bug: 'git stash --staged' crashes with binary files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-10-11T18:17:33Z","receivedAt":"2023-10-11T18:18:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 01, 2023 at 10:26:13PM +0100, Adam Johnson wrote:\n\n> Stage a binary file and run 'git stash --staged'. The stash is created but\n> the command fails to remove changes from the index:\n> \n> $ echo -n \"\\0\" >file\n> \n> $ git add file\n> \n> $ git stash --staged\n> Saved working directory and index state WIP on main: e7911b6 Whatever\n> error: cannot apply binary patch to 'file' without full index line\n> error: file: patch does not apply\n> Cannot remove worktree changes\n> \n> $ git status\n> ...\n> Changes to be committed:\n>         new file:   file\n> \n> The \"remove from the index\" step seems not to be binary-compatible.\n\nThis seems like a bug. It looks like stash does pass \"--binary\" in some\ncases, but not all. So it's probably a matter of finding the right\ninvocation and adding it.\n\n> The below patch adds a reproducing test.\n\nI think finding the bug and writing the test is probably 75% of the\nwork. :)\n\n> diff --git t/t3903-stash.sh t/t3903-stash.sh\n> index 376cc8f4ab..5e3ea64f38 100755\n> --- t/t3903-stash.sh\n> +++ t/t3903-stash.sh\n> @@ -392,6 +392,13 @@ setup_stash()\n>      git stash pop &&\n>      test bar,bar4 = $(cat file),$(cat file2)\n>  '\n> +test_expect_success 'stash --staged with binary file' '\n> +    echo -n \"\\0\" >file &&\n\nUnfortunately \"echo -n\" isn't portable. But you can use:\n\n  printf \"\\0\" >file\n\ninstead.\n\n> +    git add file &&\n> +    git stash --staged &&\n> +    git stash pop &&\n> +    test \"\\0\" = $(cat file)\n> +'\n\nI doubt this \"test\" will work, as the shell won't interpolate that into\na NUL (and anyway, I think having NULs in shell variables isn't\nportable). You could perhaps do:\n\n  printf \"\\0\" >expect &&\n  test_cmp expect file\n\n-Peff\n"},{"id":"483067","messageId":"046101d9fc6f$fd812910$f8837b30$@nexbridge.com","threadId":"59823","inReplyTo":"20231011181733.GA514244@coredump.intra.peff.net","subject":"RE: Bug: 'git stash --staged' crashes with binary files","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-10-11T18:23:05Z","receivedAt":"2023-10-11T18:23:21Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Wednesday, October 11, 2023 2:18 PM, Peff wrote:\n>On Thu, Jun 01, 2023 at 10:26:13PM +0100, Adam Johnson wrote:\n>\n>> Stage a binary file and run 'git stash --staged'. The stash is created\n>> but the command fails to remove changes from the index:\n>>\n>> $ echo -n \"\\0\" >file\n>>\n>> $ git add file\n>>\n>> $ git stash --staged\n>> Saved working directory and index state WIP on main: e7911b6 Whatever\n>> error: cannot apply binary patch to 'file' without full index line\n>> error: file: patch does not apply\n>> Cannot remove worktree changes\n>>\n>> $ git status\n>> ...\n>> Changes to be committed:\n>>         new file:   file\n>>\n>> The \"remove from the index\" step seems not to be binary-compatible.\n>\n>This seems like a bug. It looks like stash does pass \"--binary\" in some cases, but not\n>all. So it's probably a matter of finding the right invocation and adding it.\n>\n>> The below patch adds a reproducing test.\n>\n>I think finding the bug and writing the test is probably 75% of the work. :)\n>\n>> diff --git t/t3903-stash.sh t/t3903-stash.sh index\n>> 376cc8f4ab..5e3ea64f38 100755\n>> --- t/t3903-stash.sh\n>> +++ t/t3903-stash.sh\n>> @@ -392,6 +392,13 @@ setup_stash()\n>>      git stash pop &&\n>>      test bar,bar4 = $(cat file),$(cat file2)  '\n>> +test_expect_success 'stash --staged with binary file' '\n>> +    echo -n \"\\0\" >file &&\n>\n>Unfortunately \"echo -n\" isn't portable. But you can use:\n>\n>  printf \"\\0\" >file\n>\n>instead.\n>\n>> +    git add file &&\n>> +    git stash --staged &&\n>> +    git stash pop &&\n>> +    test \"\\0\" = $(cat file)\n>> +'\n>\n>I doubt this \"test\" will work, as the shell won't interpolate that into a NUL (and\n>anyway, I think having NULs in shell variables isn't portable). You could perhaps do:\n>\n>  printf \"\\0\" >expect &&\n>  test_cmp expect file\n\nAgreed. The specific test construct does not appear portable, but piping printf \"\\0\" to expect works.\n/home/randall: test \"\\0\" != `printf \"\\0\"`\nbash: warning: command substitution: ignored null byte in input\nbash: test: \\0: unary operator expected\n\n--Randall\n\n"}]}