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

[PATCH 31/48] merge-recursive: Make dead code for rename/rename(2to1) conflicts undead

From
Elijah Newren <newren@gmail.com>
Date
Jun 8, 2011, 07:31 UTC
Message-ID
<1307518278-23814-32-git-send-email-newren@gmail.com>
In-Reply-To
<1307518278-23814-1-git-send-email-newren@gmail.com>

The code for rename_rename_2to1 conflicts (two files both being renamed to the same filename) was dead since the rename/add path was always being independently triggered for each of the renames instead. Further, reviving the dead code showed that it was inherently buggy and would always segfault -- among a few other bugs.

Move the else-if branch for the rename/rename block before the rename/add block to make sure it is checked first, and fix up the rename/rename(2to1) code segments to make it handle most cases. Work is still needed to handle higher dimensional corner cases such as rename/rename/modify/modify issues.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c                 |   70 +++++++++++++++++++++++++-----------
 t/t6036-recursive-corner-cases.sh |   17 +++++----
 2 files changed, 57 insertions(+), 30 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 749e501..4c42838 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -983,17 +983,36 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
 					struct rename *ren2,
 					const char *branch2)
 {
+	char *path = ren1->pair->two->path; /* same as ren2->pair->two->path */
 	/* Two files were renamed to the same thing. */
-	char *new_path1 = unique_path(o, ren1->pair->two->path, branch1);
-	char *new_path2 = unique_path(o, ren2->pair->two->path, branch2);
-	output(o, 1, "Renaming %s to %s and %s to %s instead",
-	       ren1->pair->one->path, new_path1,
-	       ren2->pair->one->path, new_path2);
-	remove_file(o, 0, ren1->pair->two->path, 0);
-	update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);
-	update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);
-	free(new_path2);
-	free(new_path1);
+	if (o->call_depth) {
+		struct merge_file_info mfi;
+		struct diff_filespec one, a, b;
+
+		one.path = a.path = b.path = path;
+		hashcpy(one.sha1, null_sha1);
+		one.mode = 0;
+		hashcpy(a.sha1, ren1->pair->two->sha1);
+		a.mode = ren1->pair->two->mode;
+		hashcpy(b.sha1, ren2->pair->two->sha1);
+		b.mode = ren2->pair->two->mode;
+		mfi = merge_file(o, &one, &a, &b, branch1, branch2);
+		output(o, 1, "Adding merged %s", path);
+		update_file(o, 0, mfi.sha, mfi.mode, path);
+	} else {
+		char *new_path1 = unique_path(o, path, branch1);
+		char *new_path2 = unique_path(o, path, branch2);
+		output(o, 1, "Renaming %s to %s and %s to %s instead",
+		       ren1->pair->one->path, new_path1,
+		       ren2->pair->one->path, new_path2);
+		remove_file(o, 0, path, 0);
+		update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode,
+			    new_path1);
+		update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode,
+			    new_path2);
+		free(new_path2);
+		free(new_path1);
+	}
 }
 
 static int process_renames(struct merge_options *o,
@@ -1008,12 +1027,12 @@ static int process_renames(struct merge_options *o,
 	for (i = 0; i < a_renames->nr; i++) {
 		sre = a_renames->items[i].util;
 		string_list_insert(&a_by_dst, sre->pair->two->path)->util
-			= sre->dst_entry;
+			= (void*)sre;
 	}
 	for (i = 0; i < b_renames->nr; i++) {
 		sre = b_renames->items[i].util;
 		string_list_insert(&b_by_dst, sre->pair->two->path)->util
-			= sre->dst_entry;
+			= (void*)sre;
 	}
 
 	for (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {
@@ -1125,6 +1144,23 @@ static int process_renames(struct merge_options *o,
 					clean_merge = 0;
 					conflict_rename_delete(o, ren1->pair, branch1, branch2);
 				}
+			} else if ((item = string_list_lookup(renames2Dst, ren1_dst))) {
+				char *ren2_src, *ren2_dst;
+				ren2 = item->util;
+				ren2_src = ren2->pair->one->path;
+				ren2_dst = ren2->pair->two->path;
+
+				clean_merge = 0;
+				ren2->processed = 1;
+				remove_file(o, 1, ren2_src,
+					    renamed_stage == 3 || would_lose_untracked(ren1_src));
+
+				output(o, 1, "CONFLICT (rename/rename): "
+				       "Rename %s->%s in %s. "
+				       "Rename %s->%s in %s",
+				       ren1_src, ren1_dst, branch1,
+				       ren2_src, ren2_dst, branch2);
+				conflict_rename_rename_2to1(o, ren1, branch1, ren2, branch2);
 			} else if ((dst_other.mode == ren1->pair->two->mode) &&
 				   sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {
 				/* Added file on the other side
@@ -1165,16 +1201,6 @@ static int process_renames(struct merge_options *o,
 					update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
 					free(new_path);
 				}
-			} else if ((item = string_list_lookup(renames2Dst, ren1_dst))) {
-				ren2 = item->util;
-				clean_merge = 0;
-				ren2->processed = 1;
-				output(o, 1, "CONFLICT (rename/rename): "
-				       "Rename %s->%s in %s. "
-				       "Rename %s->%s in %s",
-				       ren1_src, ren1_dst, branch1,
-				       ren2->pair->one->path, ren2->pair->two->path, branch2);
-				conflict_rename_rename_2to1(o, ren1, branch1, ren2, branch2);
 			} else
 				try_merge = 1;
 
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index dea2a65..8d1d303 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -60,13 +60,13 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
 	test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
 	test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
 
-	cp two merged &&
+	cp one merged &&
 	>empty &&
 	test_must_fail git merge-file \
-		-L "Temporary merge branch 2" \
-		-L "" \
 		-L "Temporary merge branch 1" \
-		merged empty one &&
+		-L "" \
+		-L "Temporary merge branch 2" \
+		merged empty two &&
 	test $(git rev-parse :1:three) = $(git hash-object merged)
 '
 
@@ -139,11 +139,12 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
 	cp one merge-me &&
 	>empty &&
 	test_must_fail git merge-file \
-		-L "Temporary merge branch 2" \
-		-L "" \
 		-L "Temporary merge branch 1" \
-		merged empty merge-me &&
-	test $(git rev-parse :1:three) = $(git hash-object merged)
+		-L "" \
+		-L "Temporary merge branch 2" \
+		merge-me empty merged &&
+
+	test $(git rev-parse :1:three) = $(git hash-object merge-me)
 '
 
 #
-- 
1.7.6.rc0.62.g2d69f
Previous: Junio C HamanoNext: Elijah Newren
Message 63 of 97 in “Handling more corner cases in merge-recursive.c”
  1. 00/48 Handling more corner cases in merge-recursive.cElijah Newren, Jun 8, 2011
  2. 01/48 t6039: Add a testcase where git deletes an untracked fileElijah Newren, Jun 8, 2011
  3. 02/48 t6039: Add failing testcase for rename/modify/add-source conflictElijah Newren, Jun 8, 2011
  4. 03/48 t6039: Add a pair of cases where undetected renames cause issuesElijah Newren, Jun 8, 2011
  5. 04/48 t6039: Add a testcase where undetected rename causes silent file deletionElijah Newren, Jun 8, 2011
  6. 05/48 t6039: Add tests for content issues with modify/rename/directory conflictsElijah Newren, Jun 8, 2011
  7. Junio C HamanoJul 18, 2011
  8. Elijah NewrenAug 8, 2011
  9. 06/48 t6039: Add failing testcases for rename/rename/add-{source,dest} conflictsElijah Newren, Jun 8, 2011
  10. Junio C HamanoJul 18, 2011
  11. 07/48 t6039: Ensure rename/rename conflicts leave index and workdir in sane stateElijah Newren, Jun 8, 2011
  12. Junio C HamanoJul 18, 2011
  13. Elijah NewrenAug 8, 2011
  14. 08/48 t6036: Add differently resolved modify/delete conflict in criss-cross testElijah Newren, Jun 8, 2011
  15. Junio C HamanoJul 18, 2011
  16. 09/48 t6036: criss-cross with weird content can fool git into clean mergeElijah Newren, Jun 8, 2011
  17. Junio C HamanoJul 18, 2011
  18. Elijah NewrenAug 8, 2011
  19. 10/48 t6036: tests for criss-cross merges with various directory/file conflictsElijah Newren, Jun 8, 2011
  20. Junio C HamanoJul 18, 2011
  21. Elijah NewrenAug 8, 2011
  22. 11/48 t6036: criss-cross w/ rename/rename(1to2)/modify+rename/rename(2to1)/modifyElijah Newren, Jun 8, 2011
  23. Junio C HamanoJul 18, 2011
  24. 12/48 t6036: criss-cross + rename/rename(1to2)/add-source + modify/modifyElijah Newren, Jun 8, 2011
  25. Junio C HamanoJul 18, 2011
  26. Phil HordJul 20, 2011
  27. 13/48 t6022: Remove unnecessary untracked files to make test cleanerElijah Newren, Jun 8, 2011
  28. 14/48 t6022: New tests checking for unnecessary updates of filesElijah Newren, Jun 8, 2011
  29. 15/48 t6022: Add testcase for merging a renamed file with a simple changeElijah Newren, Jun 8, 2011
  30. 16/48 merge-recursive: Make BUG message more legible by adding a newlineElijah Newren, Jun 8, 2011
  31. 17/48 merge-recursive: Correct a commentElijah Newren, Jun 8, 2011
  32. 18/48 merge-recursive: Mark some diff_filespec struct arguments constElijah Newren, Jun 8, 2011
  33. Junio C HamanoJul 18, 2011
  34. 19/48 merge-recursive: Remember to free generated unique path namesElijah Newren, Jun 8, 2011
  35. Junio C HamanoJul 18, 2011
  36. 20/48 merge-recursive: Avoid working directory changes during recursive caseElijah Newren, Jun 8, 2011
  37. 21/48 merge-recursive: Fix recursive case with D/F conflict via add/add conflictElijah Newren, Jun 8, 2011
  38. Junio C HamanoJul 18, 2011
  39. 22/48 merge-recursive: Fix sorting order and directory change assumptionsElijah Newren, Jun 8, 2011
  40. Johannes SixtJul 11, 2011
  41. Johannes SixtJul 12, 2011
  42. Johannes SixtJul 13, 2011
  43. Junio C HamanoJul 13, 2011
  44. Junio C HamanoJul 18, 2011
  45. Elijah NewrenAug 8, 2011
  46. 23/48 merge-recursive: Fix code checking for D/F conflicts still being presentElijah Newren, Jun 8, 2011
  47. 24/48 merge-recursive: Save D/F conflict filenames instead of unlinking themElijah Newren, Jun 8, 2011
  48. 25/48 merge-recursive: Split was_tracked() out of would_lose_untracked()Elijah Newren, Jun 8, 2011
  49. 26/48 merge-recursive: Allow make_room_for_path() to remove D/F entriesElijah Newren, Jun 8, 2011
  50. Johannes SixtJul 11, 2011
  51. Johannes SixtJul 13, 2011
  52. Elijah NewrenAug 8, 2011
  53. Johannes SixtAug 9, 2011
  54. Junio C HamanoJul 18, 2011
  55. 27/48 merge-recursive: Consolidate different update_stages functionsElijah Newren, Jun 8, 2011
  56. Junio C HamanoJul 18, 2011
  57. 28/48 merge-recursive: Split update_stages_and_entry; only update stages at endElijah Newren, Jun 8, 2011
  58. Junio C HamanoJul 18, 2011
  59. 29/48 merge-recursive: When we detect we can skip an update, actually skip itElijah Newren, Jun 8, 2011
  60. Junio C HamanoJul 18, 2011
  61. 30/48 merge-recursive: Fix deletion of untracked file in rename/delete conflictsElijah Newren, Jun 8, 2011
  62. Junio C HamanoJul 21, 2011
  63. 31/48 merge-recursive: Make dead code for rename/rename(2to1) conflicts undeadElijah Newren, Jun 8, 2011
  64. 32/48 merge-recursive: Add comments about handling rename/add-source casesElijah Newren, Jun 8, 2011
  65. 33/48 merge-recursive: Improve handling of rename target vs. directory additionElijah Newren, Jun 8, 2011
  66. 34/48 merge-recursive: Consolidate process_entry() and process_df_entry()Elijah Newren, Jun 8, 2011
  67. Junio C HamanoJul 21, 2011
  68. 35/48 merge-recursive: Cleanup and consolidation of rename_conflict_infoElijah Newren, Jun 8, 2011
  69. 36/48 merge-recursive: Provide more info in conflict markers with file renamesElijah Newren, Jun 8, 2011
  70. Junio C HamanoJul 21, 2011
  71. 37/48 merge-recursive: Fix modify/delete resolution in the recursive caseElijah Newren, Jun 8, 2011
  72. Junio C HamanoJul 21, 2011
  73. Elijah NewrenAug 8, 2011
  74. 38/48 merge-recursive: Introduce a merge_file convenience functionElijah Newren, Jun 8, 2011
  75. 39/48 merge-recursive: Fix rename/rename(1to2) resolution for virtual merge baseElijah Newren, Jun 8, 2011
  76. Junio C HamanoJul 25, 2011
  77. Elijah NewrenAug 8, 2011
  78. 40/48 merge-recursive: Small cleanups for conflict_rename_rename_1to2Elijah Newren, Jun 8, 2011
  79. 41/48 merge-recursive: Defer rename/rename(2to1) handling until process_entryElijah Newren, Jun 8, 2011
  80. 42/48 merge-recursive: Record more data needed for merging with dual renamesElijah Newren, Jun 8, 2011
  81. 43/48 merge-recursive: Create function for merging with branchname:file markersElijah Newren, Jun 8, 2011
  82. 44/48 merge-recursive: Consider modifications in rename/rename(2to1) conflictsElijah Newren, Jun 8, 2011
  83. 45/48 merge-recursive: Make modify/delete handling code reusableElijah Newren, Jun 8, 2011
  84. 46/48 merge-recursive: Have conflict_rename_delete reuse modify/delete codeElijah Newren, Jun 8, 2011
  85. 47/48 merge-recursive: add handling for rename/rename/add-dest/add-destElijah Newren, Jun 8, 2011
  86. 48/48 merge-recursive: Fix working copy handling for rename/rename/add/addElijah Newren, Jun 8, 2011
  87. Junio C HamanoJun 11, 2011
  88. Junio C HamanoAug 4, 2011
  89. Junio C HamanoAug 4, 2011
  90. Elijah NewrenAug 4, 2011
  91. Elijah NewrenAug 4, 2011
  92. Junio C HamanoAug 4, 2011
  93. Elijah NewrenAug 4, 2011
  94. Junio C HamanoAug 6, 2011
  95. Elijah NewrenAug 6, 2011
  96. Junio C HamanoJun 12, 2011
  97. Junio C HamanoJun 12, 2011

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.