[PATCH v2 2/2] merge-recursive: avoid showing conflicts with merge branch before HEAD

From: Elijah Newren <newren@gmail.com>
Date: Oct 16, 2018, 20:19 UTC
Message-ID: <20181016201948.16025-3-newren@gmail.com>
In-Reply-To: <20181016201948.16025-1-newren@gmail.com>

We want to load unmerged entries from HEAD into the index at stage 2 and from MERGE_HEAD into stage 3. Similarly, folks expect merge conflicts to look like

    <<<<<<<< HEAD
    content from our side
    ========
    content from their side
    >>>>>>>> MERGE_HEAD

not

    <<<<<<<< MERGE_HEAD
    content from their side
    ========
    content from our side
    >>>>>>>> HEAD

The correct order usually comes naturally and for free, but with renames we often have data in the form {rename_branch, other_branch}, and working relative to the rename first (e.g. for rename/add) is more convenient elsewhere in the code. Address the slight impedance mismatch by having some functions re-call themselves with flipped arguments when the branch order is reversed.

Note that setup_rename_conflict_info() has one asymmetry in it, in setting dst_entry1->processed=0 but not doing similarly for dst_entry2->processed. When dealing with rename/rename and similar conflicts, we do not want the processing to happen twice, so the desire to only set one of the entries to unprocessed is intentional. So, while this change modifies which branch's entry will be marked as unprocessed, that dovetails nicely with putting HEAD first so that we get the index stage entries and conflict markers in the right order.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c                 | 33 ++++++++++++++++++++++++++++++-
 t/t6036-recursive-corner-cases.sh |  8 ++++----
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 8a47e54e2f..16980db7f9 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -228,7 +228,26 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,
 					      struct stage_data *src_entry1,
 					      struct stage_data *src_entry2)
 {
-	struct rename_conflict_info *ci = xcalloc(1, sizeof(struct rename_conflict_info));
+	struct rename_conflict_info *ci;
+
+	/*
+	 * When we have two renames involved, it's easiest to get the
+	 * correct things into stage 2 and 3, and to make sure that the
+	 * content merge puts HEAD before the other branch if we just
+	 * ensure that branch1 == o->branch1.  So, simply flip arguments
+	 * around if we don't have that.
+	 */
+	if (dst_entry2 && branch1 != o->branch1) {
+		setup_rename_conflict_info(rename_type,
+					   pair2,      pair1,
+					   branch2,    branch1,
+					   dst_entry2, dst_entry1,
+					   o,
+					   src_entry2, src_entry1);
+		return;
+	}
+
+	ci = xcalloc(1, sizeof(struct rename_conflict_info));
 	ci->rename_type = rename_type;
 	ci->pair1 = pair1;
 	ci->branch1 = branch1;
@@ -1283,6 +1302,18 @@ static int merge_mode_and_contents(struct merge_options *o,
 				   const char *branch2,
 				   struct merge_file_info *result)
 {
+	if (o->branch1 != branch1) {
+		/*
+		 * It's weird getting a reverse merge with HEAD on the bottom
+		 * side of the conflict markers and the other branch on the
+		 * top.  Fix that.
+		 */
+		return merge_mode_and_contents(o, one, b, a,
+					       filename,
+					       branch2, branch1,
+					       extra_marker_size, result);
+	}
+
 	result->merge = 0;
 	result->clean = 1;
 
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index 59e52c5a09..e1cef58f2a 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -230,13 +230,13 @@ test_expect_success 'git detects differently handled merges conflict' '
 			:2:new_a :3:new_a &&
 		test_cmp expect actual &&
 
-		git cat-file -p B:new_a >ours &&
-		git cat-file -p C:new_a >theirs &&
+		git cat-file -p C:new_a >ours &&
+		git cat-file -p B:new_a >theirs &&
 		>empty &&
 		test_must_fail git merge-file \
-			-L "Temporary merge branch 2" \
-			-L "" \
 			-L "Temporary merge branch 1" \
+			-L "" \
+			-L "Temporary merge branch 2" \
 			ours empty theirs &&
 		sed -e "s/^\([<=>]\)/\1\1\1/" ours >expect &&
 		git cat-file -p :1:new_a >actual &&

-- 
2.19.1.280.g0c175526bf