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

[PATCH 34/48] merge-recursive: Consolidate process_entry() and process_df_entry()

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

The whole point of adding process_df_entry() was to ensure that files of D/F conflicts were processed after paths under the corresponding directory. However, given that the entries are in sorted order, all we need to do is iterate through them in reverse order to achieve the same effect. That lets us remove some duplicated code, and lets us keep track of one less thing as we read the code ("do we need to make sure this is processed before process_df_entry() or do we need to defer it until then?").

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c |  194 ++++++++++++++++------------------------------------
 1 files changed, 60 insertions(+), 134 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index e6a97b2..5673ccb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -116,7 +116,6 @@ static inline void setup_rename_df_conflict_info(enum rename_type rename_type,
 		ci->dst_entry2 = dst_entry2;
 		ci->pair2 = pair2;
 		dst_entry2->rename_df_conflict_info = ci;
-		dst_entry2->processed = 0;
 	}
 }
 
@@ -358,20 +357,17 @@ static void record_df_conflict_files(struct merge_options *o,
 				     struct string_list *entries)
 {
 	/* If there is a D/F conflict and the file for such a conflict
-	 * currently exist in the working copy, we want to allow it to
-	 * be removed to make room for the corresponding directory if
-	 * needed.  The files underneath the directories of such D/F
-	 * conflicts will be handled in process_entry(), while the
-	 * files of such D/F conflicts will be processed later in
-	 * process_df_entry().  If the corresponding directory ends up
-	 * being removed by the merge, then no additional work needs
-	 * to be done by process_df_entry() for the conflicting file.
-	 * If the directory needs to be written to the working copy,
-	 * then the conflicting file will simply be removed (e.g. in
-	 * make_room_for_path).  If the directory is written to the
-	 * working copy but the file also has a conflict that needs to
-	 * be resolved, then process_df_entry() will reinstate the
-	 * file with a new unique name.
+	 * currently exist in the working copy, we want to allow it to be
+	 * removed to make room for the corresponding directory if needed.
+	 * The files underneath the directories of such D/F conflicts will
+	 * be processed before the corresponding file involved in the D/F
+	 * conflict.  If the D/F directory ends up being removed by the
+	 * merge, then we won't have to touch the D/F file.  If the D/F
+	 * directory needs to be written to the working copy, then the D/F
+	 * file will simply be removed (in make_room_for_path()) to make
+	 * room for the necessary paths.  Note that if both the directory
+	 * and the file need to be present, then the D/F file will be
+	 * reinstated with a new unique name at the time it is processed.
 	 */
 	const char *last_file = NULL;
 	int last_len = 0;
@@ -1310,17 +1306,17 @@ static void handle_delete_modify(struct merge_options *o,
 		       "and modified in %s. Version %s of %s left in tree%s%s.",
 		       path, o->branch1,
 		       o->branch2, o->branch2, path,
-		       path == new_path ? "" : " at ",
-		       path == new_path ? "" : new_path);
-		update_file(o, 0, b_sha, b_mode, new_path);
+		       NULL == new_path ? "" : " at ",
+		       NULL == new_path ? "" : new_path);
+		update_file(o, 0, b_sha, b_mode, new_path ? new_path : path);
 	} else {
 		output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "
 		       "and modified in %s. Version %s of %s left in tree%s%s.",
 		       path, o->branch2,
 		       o->branch1, o->branch1, path,
-		       path == new_path ? "" : " at ",
-		       path == new_path ? "" : new_path);
-		update_file(o, 0, a_sha, a_mode, new_path);
+		       NULL == new_path ? "" : " at ",
+		       NULL == new_path ? "" : new_path);
+		update_file(o, 0, a_sha, a_mode, new_path ? new_path : path);
 	}
 }
 
@@ -1422,93 +1418,6 @@ static int process_entry(struct merge_options *o,
 	unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
 	unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
 
-	if (entry->rename_df_conflict_info)
-		return 1; /* Such cases are handled elsewhere. */
-
-	entry->processed = 1;
-	if (o_sha && (!a_sha || !b_sha)) {
-		/* Case A: Deleted in one */
-		if ((!a_sha && !b_sha) ||
-		    (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) ||
-		    (!a_sha && blob_unchanged(o_sha, b_sha, normalize, path))) {
-			/* Deleted in both or deleted in one and
-			 * unchanged in the other */
-			if (a_sha)
-				output(o, 2, "Removing %s", path);
-			/* do not touch working file if it did not exist */
-			remove_file(o, 1, path, !a_sha);
-		} else if (dir_in_way(path, 0 /*check_wc*/)) {
-			entry->processed = 0;
-			return 1; /* Assume clean until processed */
-		} else {
-			/* Deleted in one and changed in the other */
-			clean_merge = 0;
-			handle_delete_modify(o, path, path,
-					     a_sha, a_mode, b_sha, b_mode);
-		}
-
-	} else if ((!o_sha && a_sha && !b_sha) ||
-		   (!o_sha && !a_sha && b_sha)) {
-		/* Case B: Added in one. */
-		unsigned mode;
-		const unsigned char *sha;
-
-		if (a_sha) {
-			mode = a_mode;
-			sha = a_sha;
-		} else {
-			mode = b_mode;
-			sha = b_sha;
-		}
-		if (dir_in_way(path, 0 /*check_wc*/)) {
-			/* Handle D->F conflicts after all subfiles */
-			entry->processed = 0;
-			return 1; /* Assume clean until processed */
-		} else {
-			output(o, 2, "Adding %s", path);
-			update_file(o, 1, sha, mode, path);
-		}
-	} 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,
-					    o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
-					    NULL);
-	} else if (!o_sha && !a_sha && !b_sha) {
-		/*
-		 * this entry was deleted altogether. a_mode == 0 means
-		 * we had that path and want to actively remove it.
-		 */
-		remove_file(o, 1, path, !a_mode);
-	} else
-		die("Fatal merge failure, shouldn't happen.");
-
-	return clean_merge;
-}
-
-/*
- * Per entry merge function for D/F (and/or rename) conflicts.  In the
- * cases we can cleanly resolve D/F conflicts, process_entry() can
- * clean out all the files below the directory for us.  All D/F
- * conflict cases must be handled here at the end to make sure any
- * directories that can be cleaned out, are.
- *
- * Some rename conflicts may also be handled here that don't necessarily
- * involve D/F conflicts, since the code to handle them is generic enough
- * to handle those rename conflicts with or without D/F conflicts also
- * being involved.
- */
-static int process_df_entry(struct merge_options *o,
-			    const char *path, struct stage_data *entry)
-{
-	int clean_merge = 1;
-	unsigned o_mode = entry->stages[1].mode;
-	unsigned a_mode = entry->stages[2].mode;
-	unsigned b_mode = entry->stages[3].mode;
-	unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);
-	unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
-	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;
@@ -1543,27 +1452,41 @@ static int process_df_entry(struct merge_options *o,
 						    conflict_info->branch1,
 						    conflict_info->pair2,
 						    conflict_info->branch2);
-			conflict_info->dst_entry2->processed = 1;
 			break;
 		default:
 			entry->processed = 0;
 			break;
 		}
 	} else if (o_sha && (!a_sha || !b_sha)) {
-		/* Modify/delete; deleted side may have put a directory in the way */
-		char *new_path;
-		int free_me = 0;
-		if (dir_in_way(path, !o->call_depth)) {
-			new_path = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
-			free_me = 1;
+		/* Case A: Deleted in one */
+		if ((!a_sha && !b_sha) ||
+		    (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) ||
+		    (!a_sha && blob_unchanged(o_sha, b_sha, normalize, path))) {
+			/* Deleted in both or deleted in one and
+			 * unchanged in the other */
+			if (a_sha)
+				output(o, 2, "Removing %s", path);
+			/* do not touch working file if it did not exist */
+			remove_file(o, 1, path, !a_sha);
+		} else {
+			/* Modify/delete; deleted side may have put a directory in the way */
+			char *new_path = NULL;
+			int free_me = 0;
+			clean_merge = 0;
+			if (dir_in_way(path, !o->call_depth)) {
+				new_path = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
+				free_me = 1;
+			}
+			handle_delete_modify(o, path, new_path,
+					     a_sha, a_mode, b_sha, b_mode);
+			if (free_me)
+				free(new_path);
 		}
-		clean_merge = 0;
-		handle_delete_modify(o, path, free_me ? new_path : path,
-				     a_sha, a_mode, b_sha, b_mode);
-		if (free_me)
-			free(new_path);
-	} else if (!o_sha && !!a_sha != !!b_sha) {
-		/* directory -> (directory, file) or <nothing> -> (directory, file) */
+	} else if ((!o_sha && a_sha && !b_sha) ||
+		   (!o_sha && !a_sha && b_sha)) {
+		/* Case B: Added in one. */
+		/* [nothing|directory] -> ([nothing|directory], file) */
+
 		const char *add_branch;
 		const char *other_branch;
 		unsigned mode;
@@ -1599,10 +1522,20 @@ static int process_df_entry(struct merge_options *o,
 			output(o, 2, "Adding %s", path);
 			update_file(o, 1, sha, mode, path);
 		}
-	} else {
-		entry->processed = 0;
-		return 1; /* not handled; assume clean until processed */
-	}
+	} 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,
+					    o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
+					    NULL);
+	} else if (!o_sha && !a_sha && !b_sha) {
+		/*
+		 * this entry was deleted altogether. a_mode == 0 means
+		 * we had that path and want to actively remove it.
+		 */
+		remove_file(o, 1, path, !a_mode);
+	} else
+		die("Fatal merge failure, shouldn't happen.");
 
 	return clean_merge;
 }
@@ -1650,7 +1583,7 @@ int merge_trees(struct merge_options *o,
 		re_head  = get_renames(o, head, common, head, merge, entries);
 		re_merge = get_renames(o, merge, common, head, merge, entries);
 		clean = process_renames(o, re_head, re_merge);
-		for (i = 0; i < entries->nr; i++) {
+		for (i = entries->nr-1; i >=0; i--) {
 			const char *path = entries->items[i].string;
 			struct stage_data *e = entries->items[i].util;
 			if (!e->processed
@@ -1658,13 +1591,6 @@ int merge_trees(struct merge_options *o,
 				clean = 0;
 		}
 		for (i = 0; i < entries->nr; i++) {
-			const char *path = entries->items[i].string;
-			struct stage_data *e = entries->items[i].util;
-			if (!e->processed
-				&& !process_df_entry(o, path, e))
-				clean = 0;
-		}
-		for (i = 0; i < entries->nr; i++) {
 			struct stage_data *e = entries->items[i].util;
 			if (!e->processed)
 				die("Unprocessed path??? %s",

-- 
1.7.6.rc0.62.g2d69f
Previous: Elijah NewrenNext: Junio C Hamano
Message 66 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.