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.
