[PATCH] stash: disable renames when calling git-diff
- From
Jeff King <peff@peff.net>
- Date
- Dec 6, 2016, 15:41 UTC
- Message-ID
- <20161206154120.yyuca35ugyuifpq6@sigill.intra.peff.net>
- In-Reply-To
- <20161206152530.snccf7buiosst3e4@sigill.intra.peff.net>
On Tue, Dec 06, 2016 at 10:25:30AM -0500, Jeff King wrote:
Show 22 quoted lines
> I agree that the second stash not saving the deletion seems like a bug. > Oddly, just stashing a deletion, like: > > rm a > git stash > git stash apply > > does store it. So it's not like we can't represent the deletion. Somehow > the presence of "b" in the index matters. > > It looks like the culprit may be this part of create_stash(): > > git diff --name-only -z HEAD -- >"$TMP-stagenames" > > That is using the "git diff" porcelain, which will respect renames, and > the --name-only bit mentions only "b". > > If you run: > > git -c diff.renames=false stash > > then it works.
And here's a patch to fix it.
-- >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
-- 2.11.0.191.gdb26c57