git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Elijah Newren <newren@gmail.com>
To: <git@vger.kernel.org>
Cc: <gitster@pobox.com>, Elijah Newren <newren@gmail.com>
Subject: [PATCH 4/4] merge-recursive: Avoid showing conflicts with merge branch before HEAD
Date: Fri, 12 Oct 2018 14:25:51 -0700
Message-ID: <20181012212551.7689-5-newren@gmail.com> (raw)
In-Reply-To: <20181012212551.7689-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 33cd9ee81f..f795c92a69 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;
@@ -1286,6 +1305,18 @@ static int merge_mode_and_contents(struct merge_options *o,
 				   const int extra_marker_size,
 				   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 21954db624..276b4e8792 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.0.235.g7c386e1068


  parent reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-12 21:25 [PATCH 0/4] More merge cleanups Elijah Newren
2018-10-12 21:25 ` [PATCH 1/4] t6036: add testcase where virtual merge base contains nested conflicts Elijah Newren
2018-10-12 21:25 ` [PATCH 2/4] merge-recursive: increase marker length with depth of recursion Elijah Newren
2018-10-15  5:12   ` Junio C Hamano
2018-10-15 15:02     ` Elijah Newren
2018-10-16  2:16       ` Junio C Hamano
2018-10-16 18:00         ` Elijah Newren
2018-10-12 21:25 ` [PATCH 3/4] merge-recursive: improve auto-merging messages with path collisions Elijah Newren
2018-10-15  5:18   ` Junio C Hamano
2018-10-12 21:25 ` Elijah Newren [this message]
2018-10-15  5:23   ` [PATCH 4/4] merge-recursive: Avoid showing conflicts with merge branch before HEAD Junio C Hamano
2018-10-16 20:19 ` [PATCH v2 0/2] More merge cleanups Elijah Newren
2018-10-16 20:19   ` [PATCH v2 1/2] merge-recursive: improve auto-merging messages with path collisions Elijah Newren
2018-10-16 20:19   ` [PATCH v2 2/2] merge-recursive: avoid showing conflicts with merge branch before HEAD Elijah Newren
2018-10-18  6:09     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181012212551.7689-5-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git