Re: [ANNOUNCE] Git v2.22.0-rc3

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jun 4, 2019, 02:30 UTC
Message-ID: <20190604023039.GR951@szeder.dev>
In-Reply-To: <CAFkS+y+dS51+nHTDy+A0HrXZpfsEE1UYPN-Xk0257o36ScUOHg@mail.gmail.com>

On Tue, Jun 04, 2019 at 11:32:24AM +1000, Ben Humphreys wrote:

Show 29 quoted lines

> Hi folks,
> 
> I’m one of the Bitbucket Server developers and I just wanted to let
> you know of one of our test cases that is now failing on the 2.22.0
> release candidates (tested rc2 and rc3). I’m still looking into it,
> but figure the release is probably imminent so worth reporting sooner
> than later.
> 
> The problem seems to be related to some recent changes in
> merge-recursive.c. I’ve made available our test dataset and the
> following steps will reproduce the problem:
> 
> $ git --version
> git version 2.22.0.rc3
> 
> $ git clone https://bitbucket.org/ben_humphreys/merge-dataset.git
> $ cd merge-dataset
> $ git checkout branch_that_has_rename_add_triggering_content_conflict_trgt
> $ git merge origin/branch_that_has_rename_add_triggering_content_conflict_src
> Assertion failed: (a->path && b->path), function merge_3way, file
> merge-recursive.c, line 1044.
> Abort trap: 6
> 
> The assertion is failing because b->path is null:
> 
> (lldb) print a->path
> (char *const) $2 = 0x00007f8e177025f8 "count.txt"
> (lldb) print b->path
> (char *const) $3 = 0x0000000000000000 <no value available>

merge_3way() is called from merge_mode_and_contents(), which in turn is called from handle_rename_add(), which was modified in 8daec1df03 (merge-recursive: switch from (oid,mode) pairs to a diff_filespec, 2019-04-05) like this:

Show 19 quoted lines

> diff --git a/merge-recursive.c b/merge-recursive.c
> index ada1c19ed2..1d2c9e1772 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> 
> @@ -1654,7 +1625,6 @@ static int handle_rename_add(struct merge_options *opt,
>  	/* a was renamed to c, and a separate c was added. */
>  	struct diff_filespec *a = ci->ren1->pair->one;
>  	struct diff_filespec *c = ci->ren1->pair->two;
> -	struct diff_filespec tmp;
>  	char *path = c->path;
>  	char *prev_path_desc;
>  	struct merge_file_info mfi;
> @@ -1669,23 +1639,21 @@ static int handle_rename_add(struct merge_options *opt,
>  	       a->path, c->path, rename_branch,
>  	       c->path, add_branch);
>  
> -	filespec_from_entry(&tmp, ci->ren1->src_entry, other_stage);
> -	tmp.path = a->path;

Note that 'tmp.path' used to be set ...

> -
>  	prev_path_desc = xstrfmt("version of %s from %s", path, a->path);
> -	if (merge_mode_and_contents(opt, a, c, &tmp,

... and that this 'tmp' used to become 'b' in merge_mode_and_contents() and then in merge_3way().

Show 7 quoted lines

> +	if (merge_mode_and_contents(opt, a, c,
> +				    &ci->ren1->src_entry->stages[other_stage],
>  				    prev_path_desc,
>  				    opt->branch1, opt->branch2,
>  				    1 + opt->call_depth * 2, &mfi))
>  		return -1;
>  	free(prev_path_desc);

This one-liner patch below the issue, the merge fails with conflicts as expected, but, honestly, I have no idea what I am doing :) At least the test suite still passes, but that might not mean all that much since it missed this issue in the first place...

diff --git a/merge-recursive.c b/merge-recursive.c
index a7bcfcbeb4..d2e380b7ed 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1660,6 +1660,7 @@ static int handle_rename_add(struct merge_options *opt,
 	       c->path, add_branch);
 
 	prev_path_desc = xstrfmt("version of %s from %s", path, a->path);
+	ci->ren1->src_entry->stages[other_stage].path = a->path;
 	if (merge_mode_and_contents(opt, a, c,
 				    &ci->ren1->src_entry->stages[other_stage],
 				    prev_path_desc,


> On Git 2.21.0 the merge succeeds (with conflicts as expected):
> 
> $ git merge origin/branch_that_has_rename_add_triggering_content_conflict_src
> CONFLICT (rename/add): Rename numbers.txt->count.txt in
> origin/branch_that_has_rename_add_triggering_content_conflict_src.
> Added count.txt in HEAD
> Auto-merging version of count.txt from numbers.txt
> Auto-merging count.txt
> Automatic merge failed; fix conflicts and then commit the result.
> 
> I’ll let you know if I get any further investigating this.