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

[PATCH 35/48] merge-recursive: Cleanup and consolidation of rename_conflict_info

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

The consolidation of process_entry() and process_df_entry() allows us to consolidate more code paths concerning rename conflicts, and to do a few additional related cleanups. It also means we are using rename_df_conflict_info in some cases where there is no D/F conflict; rename it to rename_conflict_info.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c |  134 ++++++++++++++++++++++++++---------------------------
 1 files changed, 66 insertions(+), 68 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 5673ccb..5d0a62c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -66,10 +66,11 @@ static int sha_eq(const unsigned char *a, const unsigned char *b)
 enum rename_type {
 	RENAME_NORMAL = 0,
 	RENAME_DELETE,
+	RENAME_ONE_FILE_TO_ONE,
 	RENAME_ONE_FILE_TO_TWO
 };
 
-struct rename_df_conflict_info {
+struct rename_conflict_info {
 	enum rename_type rename_type;
 	struct diff_filepair *pair1;
 	struct diff_filepair *pair2;
@@ -88,34 +89,33 @@ struct stage_data {
 		unsigned mode;
 		unsigned char sha[20];
 	} stages[4];
-	struct rename_df_conflict_info *rename_df_conflict_info;
+	struct rename_conflict_info *rename_conflict_info;
 	unsigned processed:1;
-	unsigned involved_in_rename:1;
 };
 
-static inline void setup_rename_df_conflict_info(enum rename_type rename_type,
-						 struct diff_filepair *pair1,
-						 struct diff_filepair *pair2,
-						 const char *branch1,
-						 const char *branch2,
-						 struct stage_data *dst_entry1,
-						 struct stage_data *dst_entry2)
+static inline void setup_rename_conflict_info(enum rename_type rename_type,
+					      struct diff_filepair *pair1,
+					      struct diff_filepair *pair2,
+					      const char *branch1,
+					      const char *branch2,
+					      struct stage_data *dst_entry1,
+					      struct stage_data *dst_entry2)
 {
-	struct rename_df_conflict_info *ci = xcalloc(1, sizeof(struct rename_df_conflict_info));
+	struct rename_conflict_info *ci = xcalloc(1, sizeof(struct rename_conflict_info));
 	ci->rename_type = rename_type;
 	ci->pair1 = pair1;
 	ci->branch1 = branch1;
 	ci->branch2 = branch2;
 
 	ci->dst_entry1 = dst_entry1;
-	dst_entry1->rename_df_conflict_info = ci;
+	dst_entry1->rename_conflict_info = ci;
 	dst_entry1->processed = 0;
 
 	assert(!pair2 == !dst_entry2);
 	if (dst_entry2) {
 		ci->dst_entry2 = dst_entry2;
 		ci->pair2 = pair2;
-		dst_entry2->rename_df_conflict_info = ci;
+		dst_entry2->rename_conflict_info = ci;
 	}
 }
 
@@ -939,10 +939,29 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
 	/* One file was renamed in both branches, but to different names. */
 	char *del[2];
 	int delp = 0;
+	const char *src      = pair1->one->path;
 	const char *ren1_dst = pair1->two->path;
 	const char *ren2_dst = pair2->two->path;
 	const char *dst_name1 = ren1_dst;
 	const char *dst_name2 = ren2_dst;
+
+	output(o, 1, "CONFLICT (rename/rename): "
+	       "Rename \"%s\"->\"%s\" in branch \"%s\" "
+	       "rename \"%s\"->\"%s\" in \"%s\"%s",
+	       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",
@@ -1083,20 +1102,16 @@ static int process_renames(struct merge_options *o,
 		if (ren2) {
 			const char *ren2_src = ren2->pair->one->path;
 			const char *ren2_dst = ren2->pair->two->path;
+			enum rename_type rename_type;
 			/* Renamed in 1 and renamed in 2 */
 			if (strcmp(ren1_src, ren2_src) != 0)
 				die("ren1.src != ren2.src");
 			ren2->dst_entry->processed = 1;
 			ren2->processed = 1;
 			if (strcmp(ren1_dst, ren2_dst) != 0) {
-				setup_rename_df_conflict_info(RENAME_ONE_FILE_TO_TWO,
-							      ren1->pair,
-							      ren2->pair,
-							      branch1,
-							      branch2,
-							      ren1->dst_entry,
-							      ren2->dst_entry);
+				rename_type = RENAME_ONE_FILE_TO_TWO;
 			} else {
+				rename_type = RENAME_ONE_FILE_TO_ONE;
 				/* BUG: We should only remove ren1_src in
 				 * the base stage (think of rename +
 				 * add-source cases).
@@ -1106,8 +1121,14 @@ static int process_renames(struct merge_options *o,
 					     ren1->pair->one,
 					     ren1->pair->two,
 					     ren2->pair->two);
-				ren1->dst_entry->involved_in_rename = 1;
 			}
+			setup_rename_conflict_info(rename_type,
+						   ren1->pair,
+						   ren2->pair,
+						   branch1,
+						   branch2,
+						   ren1->dst_entry,
+						   ren2->dst_entry);
 		} else {
 			/* Renamed in 1, maybe changed in 2 */
 			struct string_list_item *item;
@@ -1138,19 +1159,13 @@ static int process_renames(struct merge_options *o,
 			try_merge = 0;
 
 			if (sha_eq(src_other.sha1, null_sha1)) {
-				if (dir_in_way(ren1_dst, 0 /*check_wc*/)) {
-					ren1->dst_entry->processed = 0;
-					setup_rename_df_conflict_info(RENAME_DELETE,
-								      ren1->pair,
-								      NULL,
-								      branch1,
-								      branch2,
-								      ren1->dst_entry,
-								      NULL);
-				} else {
-					clean_merge = 0;
-					conflict_rename_delete(o, ren1->pair, branch1, branch2);
-				}
+				setup_rename_conflict_info(RENAME_DELETE,
+							   ren1->pair,
+							   NULL,
+							   branch1,
+							   branch2,
+							   ren1->dst_entry,
+							   NULL);
 			} else if ((item = string_list_lookup(renames2Dst, ren1_dst))) {
 				char *ren2_src, *ren2_dst;
 				ren2 = item->util;
@@ -1224,16 +1239,13 @@ static int process_renames(struct merge_options *o,
 					a = &src_other;
 				}
 				update_entry(ren1->dst_entry, one, a, b);
-				ren1->dst_entry->involved_in_rename = 1;
-				if (dir_in_way(ren1_dst, 0 /*check_wc*/)) {
-					setup_rename_df_conflict_info(RENAME_NORMAL,
-								      ren1->pair,
-								      NULL,
-								      branch1,
-								      NULL,
-								      ren1->dst_entry,
-								      NULL);
-				}
+				setup_rename_conflict_info(RENAME_NORMAL,
+							   ren1->pair,
+							   NULL,
+							   branch1,
+							   NULL,
+							   ren1->dst_entry,
+							   NULL);
 			}
 		}
 	}
@@ -1321,12 +1333,11 @@ static void handle_delete_modify(struct merge_options *o,
 }
 
 static int merge_content(struct merge_options *o,
-			 unsigned involved_in_rename,
 			 const char *path,
 			 unsigned char *o_sha, int o_mode,
 			 unsigned char *a_sha, int a_mode,
 			 unsigned char *b_sha, int b_mode,
-			 const char *df_rename_conflict_branch)
+			 struct rename_conflict_info *rename_conflict_info)
 {
 	const char *reason = "content";
 	struct merge_file_info mfi;
@@ -1346,8 +1357,7 @@ static int merge_content(struct merge_options *o,
 	b.mode = b_mode;
 
 	mfi = merge_file(o, &one, &a, &b, o->branch1, o->branch2);
-	if (df_rename_conflict_branch &&
-	    dir_in_way(path, !o->call_depth)) {
+	if (rename_conflict_info && dir_in_way(path, !o->call_depth)) {
 		df_conflict_remains = 1;
 	}
 
@@ -1366,7 +1376,7 @@ static int merge_content(struct merge_options *o,
 			reason = "submodule";
 		output(o, 1, "CONFLICT (%s): Merge conflict in %s",
 				reason, path);
-		if (involved_in_rename && !df_conflict_remains)
+		if (rename_conflict_info && !df_conflict_remains)
 			update_stages(path, &one, &a, &b);
 	}
 
@@ -1389,7 +1399,7 @@ static int merge_content(struct merge_options *o,
 			}
 
 		}
-		new_path = unique_path(o, path, df_rename_conflict_branch);
+		new_path = unique_path(o, path, rename_conflict_info->branch1);
 		output(o, 1, "Adding as %s instead", new_path);
 		update_file(o, 0, mfi.sha, mfi.mode, new_path);
 		free(new_path);
@@ -1419,14 +1429,14 @@ static int process_entry(struct merge_options *o,
 	unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
 
 	entry->processed = 1;
-	if (entry->rename_df_conflict_info) {
-		struct rename_df_conflict_info *conflict_info = entry->rename_df_conflict_info;
-		char *src;
+	if (entry->rename_conflict_info) {
+		struct rename_conflict_info *conflict_info = entry->rename_conflict_info;
 		switch (conflict_info->rename_type) {
 		case RENAME_NORMAL:
-			clean_merge = merge_content(o, entry->involved_in_rename, path,
+		case RENAME_ONE_FILE_TO_ONE:
+			clean_merge = merge_content(o, path,
 						    o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
-						    conflict_info->branch1);
+						    conflict_info);
 			break;
 		case RENAME_DELETE:
 			clean_merge = 0;
@@ -1435,19 +1445,7 @@ static int process_entry(struct merge_options *o,
 					       conflict_info->branch2);
 			break;
 		case RENAME_ONE_FILE_TO_TWO:
-			src = conflict_info->pair1->one->path;
 			clean_merge = 0;
-			output(o, 1, "CONFLICT (rename/rename): "
-			       "Rename \"%s\"->\"%s\" in branch \"%s\" "
-			       "rename \"%s\"->\"%s\" in \"%s\"%s",
-			       src, conflict_info->pair1->two->path, conflict_info->branch1,
-			       src, conflict_info->pair2->two->path, conflict_info->branch2,
-			       o->call_depth ? " (left unresolved)" : "");
-			if (o->call_depth) {
-				remove_file_from_cache(src);
-				update_file(o, 0, conflict_info->pair1->one->sha1,
-					    conflict_info->pair1->one->mode, src);
-			}
 			conflict_rename_rename_1to2(o, conflict_info->pair1,
 						    conflict_info->branch1,
 						    conflict_info->pair2,
@@ -1525,7 +1523,7 @@ static int process_entry(struct merge_options *o,
 	} else if (a_sha && b_sha) {
 		/* Case C: Added in both (check for same permissions) and */
 		/* case D: Modified in both, but differently. */
-		clean_merge = merge_content(o, entry->involved_in_rename, path,
+		clean_merge = merge_content(o, path,
 					    o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
 					    NULL);
 	} else if (!o_sha && !a_sha && !b_sha) {

-- 
1.7.6.rc0.62.g2d69f
Previous: Junio C HamanoNext: Elijah Newren
Message 68 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.