Thread (4 messages) 4 messages, 2 authors, 2021-06-26

Re: v2.25 regression: cherry-pick alters patch and leaves working tree dirty

From: Elijah Newren <hidden>
Date: 2021-06-13 06:58:07

On Sat, Jun 12, 2021 at 12:34 AM Anders Kaseorg [off-list ref] wrote:
quoted hunk ↗ jump to hunk
I ran into a problem where git cherry-pick incorrectly alters a patch that should apply cleanly, and leaves the working tree dirty despite claiming to finish successfully. I minimized the problem to the reproduction recipe below. Bisecting git.git shows the problem was introduced by 49b8133a9ece199a17db8bb2545202c6eac67485 (v2.25.0-rc0~144^2~1) “merge-recursive: fix merging a subdirectory into the root directory”, and it remains a problem in v2.32.0.

To reproduce:

git init
echo foo >foo
echo bar >bar
echo baz >baz
git add foo bar baz
git commit -m 'Initial commit'
mkdir dir
git mv foo dir/foo
git commit -am 'Move foo'
git mv bar dir/bar
echo baz >>baz
git commit -am 'Move bar'
git tag move-bar
git reset --hard move-bar~2
git cherry-pick move-bar

The rename part of the patch has been unexpectedly lost, and ‘bar’ has been unexpectedly deleted from the working tree:

$ git show
commit f06592b45db70540983a6c3e8bcf62712c694257
Author: Anders Kaseorg [off-list ref]
Date:   Sat Jun 12 00:20:13 2021 -0700

    Move bar
diff --git a/baz b/baz
index 7601807..1f55335 100644
--- a/baz
+++ b/baz
@@ -1 +1,2 @@
 baz
+baz
$ git status
On branch master
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        deleted:    bar

no changes added to commit (use "git add" and/or "git commit -a")

Before 49b8133a9e, the results are as expected:

$ git show
commit b7a2a3505dae3589203ca4469cb49e8a8e2727c3
Author: Anders Kaseorg [off-list ref]
Date:   Sat Jun 12 00:23:07 2021 -0700

    Move bar
diff --git a/baz b/baz
index 7601807..1f55335 100644
--- a/baz
+++ b/baz
@@ -1 +1,2 @@
 baz
+baz
diff --git a/bar b/dir/bar
similarity index 100%
rename from bar
rename to dir/bar
$ git status
On branch master
nothing to commit, working tree clean

Anders
Thanks for the detailed report, and going the extra mile to come up
with a simple testcase.  Very helpful.

There's two bugs here, and they actually predate v2.25.  If you put
all your files in a subdirectory (e.g. 'subdir/bar' instead of 'bar'
and 'subdir/dir/bar' instead of 'dir/bar'), then the same bug
reproduces going back many more versions.  The changes in v2.25 to
support directory rename detection of a subdirectory to the root just
allowed this bug to happen at the toplevel as well.

Also, one of the two bugs affects merge-ort (in addition to merge-recursive).

I've got a one-liner fix to merge-ort.c, plus two additional lines
changed to audit for similar bugs in merge-ort and make sure they just
can't happen.  I was hoping to also have patches to fix
merge-recursive as well, but despite spending more time on it than on
merge-ort, I wasn't yet able to track down either of those two bugs.
I'll try again next week.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help