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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

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

>
> -- >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@xxxxxxxxx>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]