Skip to content

Commit

Permalink
merge-recursive: fix the fix to the diff3 common ancestor label
Browse files Browse the repository at this point in the history
In commit 8e4ec33 ("merge-recursive: fix the diff3 common ancestor
label for virtual commits", 2019-10-01), which was a fix to commit
743474c ("merge-recursive: provide a better label for diff3
common ancestor", 2019-08-17), the label for the common ancestor was
changed from always being

         "merged common ancestors"

to instead be based on the number of merge bases and whether the merge
base was a real commit or a virtual one:

    >=2: "merged common ancestors"
      1, via merge_recursive_generic: "constructed merge base"
      1, otherwise: <abbreviated commit hash>
      0: "<empty tree>"

The handling for "constructed merge base" worked by allowing
opt->ancestor to be set in merge_recursive_generic(), so we paid
attention to the setting of that variable in merge_recursive_internal().
Now, for the outer merge, the code flow was simply the following:

	ancestor_name = "merged merge bases"
	loop over merge_bases: merge_recursive_internal()

The first merge base not needing recursion would determine its own
ancestor_name however necessary and thus run

	ancestor_name = $SOMETHING
	empty loop over merge_bases...
	opt->ancestor = ancestor_name
        merge_trees_internal()

Now, the next set of merge_bases that would need to be merged after this
particular merge had completed would note that opt->ancestor has been
set to something (to a local ancestor_name variable that has since been
popped off the stack), and thus it would run:

	... else if (opt->ancestor) {
		ancestor_name = opt->ancestor;  /* OOPS! */
        loop over merge_bases: merge_recursive_internal()
        opt->ancestor = ancestor_name
        merge_trees_internal()

This resulted in garbage strings being printed for the virtual merge
bases, which was visible in git.git by just merging commit b744c3a
into commit 6d8cb22.  There are two ways to fix this: set
opt->ancestor to NULL after using it to avoid re-use, or add a
!opt->priv->call_depth check to the if block for using a pre-defined
opt->ancestor.  Apply both fixes.

Signed-off-by: Elijah Newren <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
newren authored and gitster committed Oct 8, 2019
1 parent 8e4ec33 commit b657047
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion merge-recursive.c
Original file line number Diff line number Diff line change
Expand Up @@ -3550,7 +3550,7 @@ static int merge_recursive_internal(struct merge_options *opt,
merged_merge_bases = make_virtual_commit(opt->repo, tree,
"ancestor");
ancestor_name = "empty tree";
} else if (opt->ancestor) {
} else if (opt->ancestor && !opt->priv->call_depth) {
ancestor_name = opt->ancestor;
} else if (merge_bases) {
ancestor_name = "merged common ancestors";
Expand Down Expand Up @@ -3600,6 +3600,7 @@ static int merge_recursive_internal(struct merge_options *opt,
merged_merge_bases),
&result_tree);
strbuf_release(&merge_base_abbrev);
opt->ancestor = NULL; /* avoid accidental re-use of opt->ancestor */
if (clean < 0) {
flush_output(opt);
return clean;
Expand Down

0 comments on commit b657047

Please sign in to comment.