| Message ID | CAMyDDM3DFyru6zph4qqf_QoaOeezvYkT7SmwinCfJNnFsnuRjQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bug: 'git stash --staged' crashes with binary files | expand |
On Thu, Jun 01, 2023 at 10:26:13PM +0100, Adam Johnson wrote: > Stage a binary file and run 'git stash --staged'. The stash is created but > the command fails to remove changes from the index: > > $ echo -n "\0" >file > > $ git add file > > $ git stash --staged > Saved working directory and index state WIP on main: e7911b6 Whatever > error: cannot apply binary patch to 'file' without full index line > error: file: patch does not apply > Cannot remove worktree changes > > $ git status > ... > Changes to be committed: > new file: file > > The "remove from the index" step seems not to be binary-compatible. This seems like a bug. It looks like stash does pass "--binary" in some cases, but not all. So it's probably a matter of finding the right invocation and adding it. > The below patch adds a reproducing test. I think finding the bug and writing the test is probably 75% of the work. :) > diff --git t/t3903-stash.sh t/t3903-stash.sh > index 376cc8f4ab..5e3ea64f38 100755 > --- t/t3903-stash.sh > +++ t/t3903-stash.sh > @@ -392,6 +392,13 @@ setup_stash() > git stash pop && > test bar,bar4 = $(cat file),$(cat file2) > ' > +test_expect_success 'stash --staged with binary file' ' > + echo -n "\0" >file && Unfortunately "echo -n" isn't portable. But you can use: printf "\0" >file instead. > + git add file && > + git stash --staged && > + git stash pop && > + test "\0" = $(cat file) > +' I doubt this "test" will work, as the shell won't interpolate that into a NUL (and anyway, I think having NULs in shell variables isn't portable). You could perhaps do: printf "\0" >expect && test_cmp expect file -Peff
On Wednesday, October 11, 2023 2:18 PM, Peff wrote: >On Thu, Jun 01, 2023 at 10:26:13PM +0100, Adam Johnson wrote: > >> Stage a binary file and run 'git stash --staged'. The stash is created >> but the command fails to remove changes from the index: >> >> $ echo -n "\0" >file >> >> $ git add file >> >> $ git stash --staged >> Saved working directory and index state WIP on main: e7911b6 Whatever >> error: cannot apply binary patch to 'file' without full index line >> error: file: patch does not apply >> Cannot remove worktree changes >> >> $ git status >> ... >> Changes to be committed: >> new file: file >> >> The "remove from the index" step seems not to be binary-compatible. > >This seems like a bug. It looks like stash does pass "--binary" in some cases, but not >all. So it's probably a matter of finding the right invocation and adding it. > >> The below patch adds a reproducing test. > >I think finding the bug and writing the test is probably 75% of the work. :) > >> diff --git t/t3903-stash.sh t/t3903-stash.sh index >> 376cc8f4ab..5e3ea64f38 100755 >> --- t/t3903-stash.sh >> +++ t/t3903-stash.sh >> @@ -392,6 +392,13 @@ setup_stash() >> git stash pop && >> test bar,bar4 = $(cat file),$(cat file2) ' >> +test_expect_success 'stash --staged with binary file' ' >> + echo -n "\0" >file && > >Unfortunately "echo -n" isn't portable. But you can use: > > printf "\0" >file > >instead. > >> + git add file && >> + git stash --staged && >> + git stash pop && >> + test "\0" = $(cat file) >> +' > >I doubt this "test" will work, as the shell won't interpolate that into a NUL (and >anyway, I think having NULs in shell variables isn't portable). You could perhaps do: > > printf "\0" >expect && > test_cmp expect file Agreed. The specific test construct does not appear portable, but piping printf "\0" to expect works. /home/randall: test "\0" != `printf "\0"` bash: warning: command substitution: ignored null byte in input bash: test: \0: unary operator expected --Randall
diff --git t/t3903-stash.sh t/t3903-stash.sh index 376cc8f4ab..5e3ea64f38 100755 --- t/t3903-stash.sh +++ t/t3903-stash.sh @@ -392,6 +392,13 @@ setup_stash() git stash pop && test bar,bar4 = $(cat file),$(cat file2) ' +test_expect_success 'stash --staged with binary file' ' + echo -n "\0" >file && + git add file && + git stash --staged && + git stash pop && + test "\0" = $(cat file) +' test_expect_success 'dont assume push with non-option args' ' test_must_fail git stash -q drop 2>err &&
Stage a binary file and run 'git stash --staged'. The stash is created but the command fails to remove changes from the index: $ echo -n "\0" >file $ git add file $ git stash --staged Saved working directory and index state WIP on main: e7911b6 Whatever error: cannot apply binary patch to 'file' without full index line error: file: patch does not apply Cannot remove worktree changes $ git status ... Changes to be committed: new file: file The "remove from the index" step seems not to be binary-compatible. The below patch adds a reproducing test.