On Tue, Jun 04, 2019 at 11:32:24AM +1000, Ben Humphreys wrote:
> 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:
> 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().
> + 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.
