On Mon, Apr 22, 2024 at 10:28:14AM +0000, Adam Johnson via GitGitGadget wrote: > From: Adam Johnson <me@xxxxxxxx> > > "git stash --staged" would crash with binary files, after saving the stash. > This behaviour dates back to the addition of the feature in 41a28eb6c1 > (stash: implement '--staged' option for 'push' and 'save', 2021-10-18). > Adding the "--binary" option of "diff-tree" fixes this. The "diff-tree" call > in stash_patch() also omits "--binary", but that is fine since binary files > cannot be selected interactively. > > Helped-By: Jeff King <peff@xxxxxxxx> > Helped-By: Randall S. Becker <randall.becker@xxxxxxxxxxxx> > Signed-off-by: Adam Johnson <me@xxxxxxxx> I had to dig in the archive to remember what I might have helped with. ;) The patch looks good to me, though one thing I noticed: > +test_expect_success 'stash --staged with binary file' ' > + printf "\0" >file && > + git add file && > + git stash --staged && > + git stash pop && > + printf "\0" >expect && > + test_cmp expect file > +' I wonder if test_cmp would ever have problems with binary files on any platforms (and yes, I was the one who suggested using it). We use "diff" on most platforms, but a few (like old SunOS) set GIT_TEST_CMP to "cmp" because they don't have "diff -u" at all. I'm content to leave it as-is until somebody turns up a platform that complains, and then the escape hatch is "OK, so set GIT_TEST_CMP=cmp". -Peff
