git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>
Subject: [PATCH 1/4] merge-recursive: set paths correctly when three-way merging content
Date: Wed, 19 Sep 2018 09:14:31 -0700	[thread overview]
Message-ID: <20180919161434.3272-2-newren@gmail.com> (raw)
In-Reply-To: <20180919161434.3272-1-newren@gmail.com>

merge_3way() has code to mark different sides of the conflict with info
about where the content comes from.  If the names of the files involved
match, it simply uses the branch name.  If the names of the files do not
match, it uses branchname:filename.  Unfortunately, merge_content()
previously always called it with one.path = a.path = b.path.  Granted,
it didn't have other path information available to it for years, but
that was corrected by passing rename_conflict_info in commit
3c217c077a86 ("merge-recursive: Provide more info in conflict markers
with file renames", 2011-08-11).  In that commit, instead of just fixing
the bug with the pathnames, it created fake branch names incorporating
both the branch name and file name.

This "fake branch" workaround was extended further when I pulled that
logic out into a special function in commit dac4741554e7
("merge-recursive: Create function for merging with branchname:file
markers", 2011-08-11), and a number of other sites outside of
merge_content() have been added which call into that.  However, this
Rube-Goldberg-esque setup is not merely duplicate code and unnecessary
work, it also risked having other callsites invoke it in a way that
would result in markers of the form branchname:filename:filename (i.e.
with the filename repeated).

Fix this whole mess by:
  - setting one.path, a.path, and b.path appropriately
  - calling merge_file_1() directly
  - deleting the merge_file_special_markers() workaround wrapper

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c | 49 +++++++++--------------------------------------
 1 file changed, 9 insertions(+), 40 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 45a163c555..99a7ac5ec7 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1366,35 +1366,6 @@ static int merge_file_1(struct merge_options *o,
 	return 0;
 }
 
-static int merge_file_special_markers(struct merge_options *o,
-				      const struct diff_filespec *one,
-				      const struct diff_filespec *a,
-				      const struct diff_filespec *b,
-				      const char *target_filename,
-				      const char *branch1,
-				      const char *filename1,
-				      const char *branch2,
-				      const char *filename2,
-				      struct merge_file_info *mfi)
-{
-	char *side1 = NULL;
-	char *side2 = NULL;
-	int ret;
-
-	if (filename1)
-		side1 = xstrfmt("%s:%s", branch1, filename1);
-	if (filename2)
-		side2 = xstrfmt("%s:%s", branch2, filename2);
-
-	ret = merge_file_1(o, one, a, b, target_filename,
-			   side1 ? side1 : branch1,
-			   side2 ? side2 : branch2, mfi);
-
-	free(side1);
-	free(side2);
-	return ret;
-}
-
 static int merge_file_one(struct merge_options *o,
 			  const char *path,
 			  const struct object_id *o_oid, int o_mode,
@@ -1729,14 +1700,10 @@ static int handle_rename_rename_2to1(struct merge_options *o,
 
 	path_side_1_desc = xstrfmt("%s (was %s)", path, a->path);
 	path_side_2_desc = xstrfmt("%s (was %s)", path, b->path);
-	if (merge_file_special_markers(o, a, c1, &ci->ren1_other,
-				       path_side_1_desc,
-				       o->branch1, c1->path,
-				       o->branch2, ci->ren1_other.path, &mfi_c1) ||
-	    merge_file_special_markers(o, b, &ci->ren2_other, c2,
-				       path_side_2_desc,
-				       o->branch1, ci->ren2_other.path,
-				       o->branch2, c2->path, &mfi_c2))
+	if (merge_file_1(o, a, c1, &ci->ren1_other, path_side_1_desc,
+			 o->branch1, o->branch2, &mfi_c1) ||
+	    merge_file_1(o, b, &ci->ren2_other, c2, path_side_2_desc,
+			 o->branch1, o->branch2, &mfi_c2))
 		return -1;
 	free(path_side_1_desc);
 	free(path_side_2_desc);
@@ -3059,14 +3026,16 @@ static int merge_content(struct merge_options *o,
 		path2 = (rename_conflict_info->pair2 ||
 			 o->branch2 == rename_conflict_info->branch1) ?
 			pair1->two->path : pair1->one->path;
+		one.path = pair1->one->path;
+		a.path = (char *)path1;
+		b.path = (char *)path2;
 
 		if (dir_in_way(path, !o->call_depth,
 			       S_ISGITLINK(pair1->two->mode)))
 			df_conflict_remains = 1;
 	}
-	if (merge_file_special_markers(o, &one, &a, &b, path,
-				       o->branch1, path1,
-				       o->branch2, path2, &mfi))
+	if (merge_file_1(o, &one, &a, &b, path,
+			 o->branch1, o->branch2, &mfi))
 		return -1;
 
 	/*
-- 
2.19.0.12.gc6760fd9a9


  reply	other threads:[~2018-09-19 16:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19 16:14 [PATCH 0/4] Cleanup of merge_*() functions in merge-recursive Elijah Newren
2018-09-19 16:14 ` Elijah Newren [this message]
2018-09-19 16:14 ` [PATCH 2/4] merge-recursive: avoid wrapper function when unnecessary and wasteful Elijah Newren
2018-09-19 16:14 ` [PATCH 3/4] merge-recursive: remove final remaining caller of merge_file_one() Elijah Newren
2018-09-19 16:14 ` [PATCH 4/4] merge-recursive: rename merge_file_1() and merge_content() Elijah Newren
2018-09-19 23:40   ` Stefan Beller

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

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

  git send-email \
    --in-reply-to=20180919161434.3272-2-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).