git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] stash: disable renames when calling git-diff

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 6, 2016, 19:20 UTC
Message-ID
<xmqqh96g27bh.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20161206154120.yyuca35ugyuifpq6@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 7 quoted lines
>> If you run:
>> 
>>   git -c diff.renames=false stash
>> 
>> then it works.
>
> And here's a patch to fix it.

Yuck. This obviously has easier to bite more people since we enabled the renames by default. Thanks for a quick fix.

I wonder why we are using "git diff" here, not the plumbing, though.

Show 69 quoted lines
>
> -- >8 --
> Subject: [PATCH] stash: disable renames when calling git-diff
>
> When creating a stash, we need to look at the diff between
> the working tree and HEAD. There's no plumbing command that
> covers this case, so we do so by calling the git-diff
> porcelain. This means we also inherit the usual list of
> porcelain options, which can impact the output in confusing
> ways.
>
> There's at least one known problem: --name-only will not
> mention the source side of a rename, meaning that we will
> fail to stash a deletion in such a case.
>
> The correct solution is to move to an appropriate plumbing
> command. But since there isn't one available, in the short
> term we can plug this particular problem by manually asking
> git-diff not to respect renames.
>
> Reported-by: Matthew Patey <matthew.patey2167@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> There may be other similar problems lurking depending on the various
> config options you have set, but I don't think so. Most of the options
> only affect --patch output.
>
> I do find it interesting that --name-only does not mention the source
> side of a rename. The longer forms like --name-status mention both
> sides. Certainly --name-status does not have any way of saying "this is
> the source side of a rename", but I'd think it would simply mention both
> sides. Anyway, even if that were changed (which would fix this bug), I
> think adding --no-renames is still a good thing to be doing here.
>
>  git-stash.sh     | 2 +-
>  t/t3903-stash.sh | 9 +++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 4546abaae..40ab2710e 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -115,7 +115,7 @@ create_stash () {
>  			git read-tree --index-output="$TMPindex" -m $i_tree &&
>  			GIT_INDEX_FILE="$TMPindex" &&
>  			export GIT_INDEX_FILE &&
> -			git diff --name-only -z HEAD -- >"$TMP-stagenames" &&
> +			git diff --name-only -z --no-renames HEAD -- >"$TMP-stagenames" &&
>  			git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
>  			git write-tree &&
>  			rm -f "$TMPindex"
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index e1a6ccc00..2de3e18ce 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -766,4 +766,13 @@ test_expect_success 'stash list --cc shows combined diff' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'stash is not confused by partial renames' '
> +	mv file renamed &&
> +	git add renamed &&
> +	git stash &&
> +	git stash apply &&
> +	test_path_is_file renamed &&
> +	test_path_is_missing file
> +'
> +
>  test_done
Previous: Jeff KingNext: Jeff King
Message 5 of 13 in “Bug: stash staged file move loses original file deletion”
  1. Matthew PateyDec 5, 2016
  2. Jeff KingDec 6, 2016
  3. Jeff KingDec 6, 2016
  4. stash: disable renames when calling git-diffJeff King, Dec 6, 2016
  5. Junio C HamanoDec 6, 2016
  6. Jeff KingDec 6, 2016
  7. Junio C HamanoDec 6, 2016
  8. Jeff KingDec 6, 2016
  9. Junio C HamanoDec 6, 2016
  10. Jeff KingDec 6, 2016
  11. Junio C HamanoDec 6, 2016
  12. Jeff KingDec 6, 2016
  13. Junio C HamanoDec 6, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.