Subject: [PATCH v3] merge-recursive: fix the diff3 common ancestor label for virtual commits

From: Elijah Newren <[email-masked]>
Date: Tue, 1 Oct 2019 11:17:27 -0700

In commit 743474cbfa8b ("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:

    >=2: "merged common ancestors"
      1: <abbreviated commit hash>
      0: "<empty tree>"

Unfortunately, this did not take into account that when we have a single
merge base, that merge base could be fake or constructed.  In such
cases, this resulted in a label of "00000000".  Of course, the previous
label of "merged common ancestors" was also misleading for this case.
Since we have an API that is explicitly about creating fake merge base
commits in merge_recursive_generic(), we should provide a better label
when using that API with one merge base.  So, when
merge_recursive_generic() is called with one merge base, set the label
to:

         "constructed merge base"

Note that callers of merge_recursive_generic() include the builtin
commands git-am (in combination with git apply --build-fake-ancestor),
git-merge-recursive, and git-stash.

Helped-by: Jeff King <[email-masked]>
Signed-off-by: Elijah Newren <[email-masked]>
---
Note: Applies to the top of en/merge-recursive-cleanup, which is in next.

Changes since v2:
  - Squashed in the testcase Peff provided and changed his attribution
    from Reported-by to Helped-by due to the testcase

Range-diff:
1:  3f4444bfd7 ! 1:  208e69a4eb merge-recursive: fix the diff3 common ancestor label for virtual commits
    @@ Commit message
         commands git-am (in combination with git apply --build-fake-ancestor),
         git-merge-recursive, and git-stash.
     
    -    Reported-by: Jeff King <[email-masked]>
    +    Helped-by: Jeff King <[email-masked]>
         Signed-off-by: Elijah Newren <[email-masked]>
     
      ## merge-recursive.c ##
    @@ merge-recursive.c: int merge_recursive_generic(struct merge_options *opt,
      	}
      
      	repo_hold_locked_index(opt->repo, &lock, LOCK_DIE_ON_ERROR);
    +
    + ## t/t6047-diff3-conflict-markers.sh ##
    +@@ t/t6047-diff3-conflict-markers.sh: test_expect_success 'check multiple merge bases' '
    + 	)
    + '
    + 
    ++test_expect_success 'rebase describes fake ancestor base' '
    ++	test_create_repo rebase &&
    ++	(
    ++		cd rebase &&
    ++		test_commit base file &&
    ++		test_commit master file &&
    ++		git checkout -b side HEAD^ &&
    ++		test_commit side file &&
    ++		test_must_fail git -c merge.conflictstyle=diff3 rebase master &&
    ++		grep "||||||| constructed merge base" file
    ++	)
    ++'
    ++
    + test_done

 merge-recursive.c                 |  7 ++++++-
 t/t6047-diff3-conflict-markers.sh | 13 +++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index b058741f00..e12d91f48a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3550,6 +3550,8 @@ 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) {
+		ancestor_name = opt->ancestor;
 	} else if (merge_bases) {
 		ancestor_name = "merged common ancestors";
 	} else {
@@ -3689,7 +3691,8 @@ int merge_recursive(struct merge_options *opt,
 {
 	int clean;
 
-	assert(opt->ancestor == NULL);
+	assert(opt->ancestor == NULL ||
+	       !strcmp(opt->ancestor, "constructed merge base"));
 
 	if (merge_start(opt, repo_get_commit_tree(opt->repo, h1)))
 		return -1;
@@ -3741,6 +3744,8 @@ int merge_recursive_generic(struct merge_options *opt,
 					   oid_to_hex(merge_bases[i]));
 			commit_list_insert(base, &ca);
 		}
+		if (num_merge_bases == 1)
+			opt->ancestor = "constructed merge base";
 	}
 
 	repo_hold_locked_index(opt->repo, &lock, LOCK_DIE_ON_ERROR);
diff --git a/t/t6047-diff3-conflict-markers.sh b/t/t6047-diff3-conflict-markers.sh
index 3fb68e0aae..860542aad0 100755
--- a/t/t6047-diff3-conflict-markers.sh
+++ b/t/t6047-diff3-conflict-markers.sh
@@ -186,4 +186,17 @@ test_expect_success 'check multiple merge bases' '
 	)
 '
 
+test_expect_success 'rebase describes fake ancestor base' '
+	test_create_repo rebase &&
+	(
+		cd rebase &&
+		test_commit base file &&
+		test_commit master file &&
+		git checkout -b side HEAD^ &&
+		test_commit side file &&
+		test_must_fail git -c merge.conflictstyle=diff3 rebase master &&
+		grep "||||||| constructed merge base" file
+	)
+'
+
 test_done
-- 
2.23.0.25.g3f4444bfd7.dirty