Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Jeff King <>
To: Adam Johnson via GitGitGadget <>
	"Randall S. Becker" <>,
	Adam Johnson <>
Subject: Re: [PATCH] stash: fix "--staged" with binary files
Date: Tue, 23 Apr 2024 17:44:19 -0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Mon, Apr 22, 2024 at 10:28:14AM +0000, Adam Johnson via GitGitGadget wrote:

> From: Adam Johnson <>
> "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 <>
> Helped-By: Randall S. Becker <>
> Signed-off-by: Adam Johnson <>

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".


      parent reply	other threads:[~2024-04-23 21:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-22 10:28 [PATCH] stash: fix "--staged" with binary files Adam Johnson via GitGitGadget
2024-04-22 19:00 ` Junio C Hamano
2024-04-22 20:48   ` Adam Johnson
2024-04-23 21:44 ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).