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

From: Elijah Newren <newren@gmail.com>
Date: Jun 13, 2021, 06:57 UTC
Message-ID: <CABPp-BGDfucqae=HNES_QmmsjpDbdHrR6CG=H3gtiDygHzquVg@mail.gmail.com>
In-Reply-To: <148044c89c194a82aa15ab0ca016669d@oc11expo7.exchange.mit.edu>

On Sat, Jun 12, 2021 at 12:34 AM Anders Kaseorg <andersk@mit.edu> wrote:

Show 73 quoted lines

>
> 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 <andersk@mit.edu>
> 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 <andersk@mit.edu>
> 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.