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