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 bardiff --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 bardiff --git a/baz b/baz index 7601807..1f55335 100644 --- a/baz +++ b/baz@@ -1 +1,2 @@ baz +bazdiff --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.