[PATCH 39/48] merge-recursive: Fix rename/rename(1to2) resolution for virtual merge base

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When renaming one file to two files, we really should be doing a content
merge.  Also, in the recursive case, undoing the renames and recording the
merged file in the index with the source of the rename (while deleting
both destinations) allows the renames to be re-detected in the
non-recursive merge and will result in fewer spurious conflicts.

Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
---
 merge-recursive.c                 |   30 +++++++++++++-----------------
 t/t6036-recursive-corner-cases.sh |    2 +-
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 3a364fa..576e02d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -971,17 +971,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
 	       src, pair1->two->path, branch1,
 	       src, pair2->two->path, branch2,
 	       o->call_depth ? " (left unresolved)" : "");
-	if (o->call_depth) {
-		/*
-		 * FIXME: Why remove file from cache, and then
-		 * immediately readd it?  Why not just overwrite using
-		 * update_file only?  Also...this is buggy for
-		 * rename/add-source situations...
-		 */
-		remove_file_from_cache(src);
-		update_file(o, 0, pair1->one->sha1, pair1->one->mode, src);
-	}
-
 	if (dir_in_way(ren1_dst, !o->call_depth)) {
 		dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1);
 		output(o, 1, "%s is a directory in %s adding as %s instead",
@@ -993,14 +982,21 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
 		       ren2_dst, branch1, dst_name2);
 	}
 	if (o->call_depth) {
-		remove_file_from_cache(dst_name1);
-		remove_file_from_cache(dst_name2);
+		struct merge_file_info mfi;
+		mfi = merge_file(o, src,
+				 pair1->one->sha1, pair1->one->mode,
+				 pair1->two->sha1, pair1->two->mode,
+				 pair2->two->sha1, pair2->two->mode,
+				 branch1, branch2);
 		/*
-		 * Uncomment to leave the conflicting names in the resulting tree
-		 *
-		 * update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1);
-		 * update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2);
+		 * FIXME: For rename/add-source conflicts (if we could detect
+		 * such), this is wrong.  We should instead find a unique
+		 * pathname and then either rename the add-source file to that
+		 * unique path, or use that unique path instead of src here.
 		 */
+		update_file(o, 0, mfi.sha, mfi.mode, src);
+		remove_file_from_cache(ren1_dst);
+		remove_file_from_cache(ren2_dst);
 	} else {
 		update_stages(ren1_dst, NULL, pair1->two, NULL);
 		update_stages(ren2_dst, NULL, NULL, pair2->two);
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index d27477b..a1b609f 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -611,7 +611,7 @@ test_expect_success 'setup rename/rename(1to2)/modify followed by what looks lik
 	git tag E
 '
 
-test_expect_failure 'handle rename/rename(1to2)/modify followed by what looks like rename/rename(2to1)/modify' '
+test_expect_success 'handle rename/rename(1to2)/modify followed by what looks like rename/rename(2to1)/modify' '
 	git checkout D^0 &&
 
 	git merge -s recursive E^0 &&
-- 
1.7.6.rc0.62.g2d69f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]