git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 08/10] merge-recursive: improve rename/rename(1to2)/add[/add] handling

From
Elijah Newren <newren@gmail.com>
Date
Nov 2, 2018, 18:53 UTC
Message-ID
<20181102185317.31015-9-newren@gmail.com>
In-Reply-To
<20181102185317.31015-1-newren@gmail.com>

When we have a rename/rename(1to2) conflict, each of the renames can collide with a file addition. Each of these rename/add conflicts suffered from the same kinds of problems that normal rename/add suffered from. Make the code use handle_file_conflicts() as well so that we get all the same fixes and consistent behavior between the different conflict types.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c                    | 154 +++++++++++++--------------
 t/t6042-merge-rename-corner-cases.sh |  29 +++--
 t/t6043-merge-rename-directories.sh  |  24 +++--
 3 files changed, 113 insertions(+), 94 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c78b347112..59811116b6 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1709,80 +1709,17 @@ static int handle_rename_add(struct merge_options *o,
 				     ci->dst_entry1->stages[other_stage].mode);
 }
 
-static int handle_file(struct merge_options *o,
-			struct diff_filespec *rename,
-			int stage,
-			struct rename_conflict_info *ci)
-{
-	char *dst_name = rename->path;
-	struct stage_data *dst_entry;
-	const char *cur_branch, *other_branch;
-	struct diff_filespec other;
-	struct diff_filespec *add;
-	int ret;
-
-	if (stage == 2) {
-		dst_entry = ci->dst_entry1;
-		cur_branch = ci->branch1;
-		other_branch = ci->branch2;
-	} else {
-		dst_entry = ci->dst_entry2;
-		cur_branch = ci->branch2;
-		other_branch = ci->branch1;
-	}
-
-	add = filespec_from_entry(&other, dst_entry, stage ^ 1);
-	if (add) {
-		int ren_src_was_dirty = was_dirty(o, rename->path);
-		char *add_name = unique_path(o, rename->path, other_branch);
-		if (update_file(o, 0, &add->oid, add->mode, add_name))
-			return -1;
-
-		if (ren_src_was_dirty) {
-			output(o, 1, _("Refusing to lose dirty file at %s"),
-			       rename->path);
-		}
-		/*
-		 * Because the double negatives somehow keep confusing me...
-		 *    1) update_wd iff !ren_src_was_dirty.
-		 *    2) no_wd iff !update_wd
-		 *    3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
-		 */
-		remove_file(o, 0, rename->path, ren_src_was_dirty);
-		dst_name = unique_path(o, rename->path, cur_branch);
-	} else {
-		if (dir_in_way(rename->path, !o->call_depth, 0)) {
-			dst_name = unique_path(o, rename->path, cur_branch);
-			output(o, 1, _("%s is a directory in %s adding as %s instead"),
-			       rename->path, other_branch, dst_name);
-		} else if (!o->call_depth &&
-			   would_lose_untracked(rename->path)) {
-			dst_name = unique_path(o, rename->path, cur_branch);
-			output(o, 1, _("Refusing to lose untracked file at %s; "
-				       "adding as %s instead"),
-			       rename->path, dst_name);
-		}
-	}
-	if ((ret = update_file(o, 0, &rename->oid, rename->mode, dst_name)))
-		; /* fall through, do allow dst_name to be released */
-	else if (stage == 2)
-		ret = update_stages(o, rename->path, NULL, rename, add);
-	else
-		ret = update_stages(o, rename->path, NULL, add, rename);
-
-	if (dst_name != rename->path)
-		free(dst_name);
-
-	return ret;
-}
-
 static int handle_rename_rename_1to2(struct merge_options *o,
 				     struct rename_conflict_info *ci)
 {
 	/* One file was renamed in both branches, but to different names. */
+	struct merge_file_info mfi;
+	struct diff_filespec other;
+	struct diff_filespec *add;
 	struct diff_filespec *one = ci->pair1->one;
 	struct diff_filespec *a = ci->pair1->two;
 	struct diff_filespec *b = ci->pair2->two;
+	char *path_desc;
 
 	output(o, 1, _("CONFLICT (rename/rename): "
 	       "Rename \"%s\"->\"%s\" in branch \"%s\" "
@@ -1790,15 +1727,16 @@ static int handle_rename_rename_1to2(struct merge_options *o,
 	       one->path, a->path, ci->branch1,
 	       one->path, b->path, ci->branch2,
 	       o->call_depth ? _(" (left unresolved)") : "");
-	if (o->call_depth) {
-		struct merge_file_info mfi;
-		struct diff_filespec other;
-		struct diff_filespec *add;
-		if (merge_mode_and_contents(o, one, a, b, one->path,
-					    ci->branch1, ci->branch2,
-					    o->call_depth * 2, &mfi))
-			return -1;
 
+	path_desc = xstrfmt("%s and %s, both renamed from %s",
+			    a->path, b->path, one->path);
+	if (merge_mode_and_contents(o, one, a, b, path_desc,
+				    ci->branch1, ci->branch2,
+				    o->call_depth * 2, &mfi))
+		return -1;
+	free(path_desc);
+
+	if (o->call_depth) {
 		/*
 		 * FIXME: For rename/add-source conflicts (if we could detect
 		 * such), this is wrong.  We should instead find a unique
@@ -1830,8 +1768,70 @@ static int handle_rename_rename_1to2(struct merge_options *o,
 		}
 		else
 			remove_file_from_cache(b->path);
-	} else if (handle_file(o, a, 2, ci) || handle_file(o, b, 3, ci))
-		return -1;
+	} else {
+		/*
+		 * For each destination path, we need to see if there is a
+		 * rename/add collision.  If not, we can write the file out
+		 * to the specified location.
+		 */
+		add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1);
+		if (add) {
+			if (handle_file_collision(o, a->path,
+						  NULL, NULL,
+						  ci->branch1, ci->branch2,
+						  &mfi.oid, mfi.mode,
+						  &add->oid, add->mode) < 0)
+				return -1;
+		} else {
+			char *new_path = NULL;
+			if (dir_in_way(a->path, !o->call_depth, 0)) {
+				new_path = unique_path(o, a->path, ci->branch1);
+				output(o, 1, _("%s is a directory in %s adding "
+					       "as %s instead"),
+				       a->path, ci->branch2, new_path);
+			} else if (would_lose_untracked(a->path)) {
+				new_path = unique_path(o, a->path, ci->branch1);
+				output(o, 1, _("Refusing to lose untracked file"
+					       " at %s; adding as %s instead"),
+				       a->path, new_path);
+			}
+
+			if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : a->path))
+				return -1;
+			free(new_path);
+			if (update_stages(o, a->path, NULL, a, NULL))
+				return -1;
+		}
+
+		add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1);
+		if (add) {
+			if (handle_file_collision(o, b->path,
+						  NULL, NULL,
+						  ci->branch1, ci->branch2,
+						  &add->oid, add->mode,
+						  &mfi.oid, mfi.mode) < 0)
+				return -1;
+		} else {
+			char *new_path = NULL;
+			if (dir_in_way(b->path, !o->call_depth, 0)) {
+				new_path = unique_path(o, b->path, ci->branch2);
+				output(o, 1, _("%s is a directory in %s adding "
+					       "as %s instead"),
+				       b->path, ci->branch1, new_path);
+			} else if (would_lose_untracked(b->path)) {
+				new_path = unique_path(o, b->path, ci->branch2);
+				output(o, 1, _("Refusing to lose untracked file"
+					       " at %s; adding as %s instead"),
+				       b->path, new_path);
+			}
+
+			if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : b->path))
+				return -1;
+			free(new_path);
+			if (update_stages(o, b->path, NULL, NULL, b))
+				return -1;
+		}
+	}
 
 	return 0;
 }
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index b93139f628..7cc34e7579 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -684,7 +684,7 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
 		git ls-files -u c >out &&
 		test_line_count = 2 out &&
 		git ls-files -o >out &&
-		test_line_count = 5 out &&
+		test_line_count = 1 out &&
 
 		git rev-parse >expect               \
 			A:a   C:b   B:b   C:c   B:c &&
@@ -692,14 +692,27 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
 			:1:a  :2:b  :3:b  :2:c  :3:c &&
 		test_cmp expect actual &&
 
-		git rev-parse >expect               \
-			C:c     B:c     C:b     B:b &&
-		git hash-object >actual                \
-			c~HEAD  c~B\^0  b~HEAD  b~B\^0 &&
-		test_cmp expect actual &&
+		# Record some contents for re-doing merges
+		git cat-file -p A:a >stuff &&
+		git cat-file -p C:b >important_info &&
+		git cat-file -p B:c >precious_data &&
+		>empty &&
 
-		test_path_is_missing b &&
-		test_path_is_missing c
+		# Test the merge in b
+		test_must_fail git merge-file \
+			-L "HEAD" \
+			-L "" \
+			-L "B^0" \
+			important_info empty stuff &&
+		test_cmp important_info b &&
+
+		# Test the merge in c
+		test_must_fail git merge-file \
+			-L "HEAD" \
+			-L "" \
+			-L "B^0" \
+			stuff empty precious_data &&
+		test_cmp stuff c
 	)
 '
 
diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index fedaeafc55..5c01a0c14a 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -1078,7 +1078,7 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam
 		git ls-files -u >out &&
 		test_line_count = 6 out &&
 		git ls-files -o >out &&
-		test_line_count = 3 out &&
+		test_line_count = 1 out &&
 
 		git rev-parse >actual \
 			:0:y/b :0:y/c :0:y/e &&
@@ -1094,9 +1094,9 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam
 		test_cmp expect actual &&
 
 		git hash-object >actual \
-			w/d~HEAD w/d~B^0 z/d &&
+			z/d &&
 		git rev-parse >expect \
-			O:x/d    B:w/d   O:x/d &&
+			O:x/d &&
 		test_cmp expect actual &&
 		test_path_is_missing x/d &&
 		test_path_is_file y/d &&
@@ -3672,7 +3672,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena
 		git ls-files -u >out &&
 		test_line_count = 4 out &&
 		git ls-files -o >out &&
-		test_line_count = 4 out &&
+		test_line_count = 3 out &&
 
 		echo different >expected &&
 		echo mods >>expected &&
@@ -3684,11 +3684,17 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena
 			 O:z/a  O:z/b  O:x/d  O:x/c  O:x/c  A:y/c  O:x/c &&
 		test_cmp expect actual &&
 
-		git hash-object >actual \
-			y/c~B^0 y/c~HEAD &&
-		git rev-parse >expect \
-			O:x/c   A:y/c &&
-		test_cmp expect actual
+		# See if y/c~merged has expected contents; requires manually
+		# doing the expected file merge
+		git cat-file -p A:y/c >c1 &&
+		git cat-file -p B:z/c >c2 &&
+		>empty &&
+		test_must_fail git merge-file \
+			-L "HEAD" \
+			-L "" \
+			-L "B^0" \
+			c1 empty c2 &&
+		test_cmp c1 y/c~merged
 	)
 '
 

-- 
2.19.0.232.gd14c2061fc
Previous: Elijah NewrenNext: Derrick Stolee
Message 31 of 52 in “Improve path collision conflict resolutions”
  1. 0/7 Improve path collision conflict resolutionsElijah Newren, Oct 14, 2018
  2. 1/7 Add testcases for consistency in file collision conflict handlingElijah Newren, Oct 14, 2018
  3. 4/7 merge-recursive: fix rename/add conflict handlingElijah Newren, Oct 14, 2018
  4. 3/7 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Oct 14, 2018
  5. 2/7 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Oct 14, 2018
  6. 6/7 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Oct 14, 2018
  7. 5/7 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Oct 14, 2018
  8. 7/7 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Oct 14, 2018
  9. 0/8 Improve path collision conflict resolutionsElijah Newren, Oct 19, 2018
  10. 3/8 merge-recursive: increase marker length with depth of recursionElijah Newren, Oct 19, 2018
  11. 2/8 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Oct 19, 2018
  12. Derrick StoleeOct 31, 2018
  13. Elijah NewrenNov 1, 2018
  14. 4/8 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Oct 19, 2018
  15. Derrick StoleeOct 31, 2018
  16. Derrick StoleeOct 31, 2018
  17. Elijah NewrenNov 1, 2018
  18. 1/8 Add testcases for consistency in file collision conflict handlingElijah Newren, Oct 19, 2018
  19. 6/8 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Oct 19, 2018
  20. 5/8 merge-recursive: fix rename/add conflict handlingElijah Newren, Oct 19, 2018
  21. 7/8 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Oct 19, 2018
  22. 8/8 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Oct 19, 2018
  23. Derrick StoleeOct 31, 2018
  24. Elijah NewrenNov 1, 2018
  25. Elijah NewrenNov 2, 2018
  26. Derrick StoleeNov 2, 2018
  27. 00/10 Improve path collision conflict resolutionsElijah Newren, Nov 2, 2018
  28. 07/10 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Nov 2, 2018
  29. 02/10 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Nov 2, 2018
  30. 06/10 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Nov 2, 2018
  31. 08/10 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Nov 2, 2018
  32. merge-recursive: combine error handlingDerrick Stolee, Nov 2, 2018
  33. 05/10 merge-recursive: fix rename/add conflict handlingElijah Newren, Nov 2, 2018
  34. 09/10 fixup! merge-recursive: fix rename/add conflict handlingElijah Newren, Nov 2, 2018
  35. 04/10 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Nov 2, 2018
  36. 03/10 merge-recursive: increase marker length with depth of recursionElijah Newren, Nov 2, 2018
  37. 01/10 Add testcases for consistency in file collision conflict handlingElijah Newren, Nov 2, 2018
  38. 10/10 fixup! merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Nov 2, 2018
  39. Derrick StoleeNov 2, 2018
  40. Elijah NewrenNov 2, 2018
  41. 00/10 Improve path collision conflict resolutionsElijah Newren, Nov 8, 2018
  42. 01/10 Add testcases for consistency in file collision conflict handlingElijah Newren, Nov 8, 2018
  43. 05/10 merge-recursive: fix rename/add conflict handlingElijah Newren, Nov 8, 2018
  44. 04/10 merge-recursive: new function for better colliding conflict resolutionsElijah Newren, Nov 8, 2018
  45. 07/10 merge-recursive: use handle_file_collision for add/add conflictsElijah Newren, Nov 8, 2018
  46. 09/10 t6036, t6043: increase code coverage for file collision handlingElijah Newren, Nov 8, 2018
  47. 10/10 merge-recursive: combine error handlingElijah Newren, Nov 8, 2018
  48. Junio C HamanoNov 8, 2018
  49. 06/10 merge-recursive: improve handling for rename/rename(2to1) conflictsElijah Newren, Nov 8, 2018
  50. 02/10 t6036, t6042: testcases for rename collision of already conflicting filesElijah Newren, Nov 8, 2018
  51. 08/10 merge-recursive: improve rename/rename(1to2)/add[/add] handlingElijah Newren, Nov 8, 2018
  52. 03/10 merge-recursive: increase marker length with depth of recursionElijah Newren, Nov 8, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.