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

[PATCH v4 25/34] merge-recursive: add computation of collisions due to dir rename & merging

From
Elijah Newren <newren@gmail.com>
Date
Nov 29, 2017, 01:42 UTC
Message-ID
<20171129014237.32570-26-newren@gmail.com>
In-Reply-To
<20171129014237.32570-1-newren@gmail.com>

directory renaming and merging can cause one or more files to be moved to where an existing file is, or to cause several files to all be moved to the same (otherwise vacant) location. Add checking and reporting for such cases, falling back to no-directory-rename handling for such paths.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 120 insertions(+), 3 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 6bd4f34d55..9e31baaf33 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1416,6 +1416,31 @@ static int tree_has_path(struct tree *tree, const char *path)
 			       hashy, &mode_o);
 }
 
+/*
+ * Return a new string that replaces the beginning portion (which matches
+ * entry->dir), with entry->new_dir.  In perl-speak:
+ *   new_path_name = (old_path =~ s/entry->dir/entry->new_dir/);
+ * NOTE:
+ *   Caller must ensure that old_path starts with entry->dir + '/'.
+ */
+static char *apply_dir_rename(struct dir_rename_entry *entry,
+			      const char *old_path)
+{
+	struct strbuf new_path = STRBUF_INIT;
+	int oldlen, newlen;
+
+	if (entry->non_unique_new_dir)
+		return NULL;
+
+	oldlen = strlen(entry->dir);
+	newlen = entry->new_dir.len + (strlen(old_path) - oldlen) + 1;
+	strbuf_grow(&new_path, newlen);
+	strbuf_addbuf(&new_path, &entry->new_dir);
+	strbuf_addstr(&new_path, &old_path[oldlen]);
+
+	return strbuf_detach(&new_path, NULL);
+}
+
 static void get_renamed_dir_portion(const char *old_path, const char *new_path,
 				    char **old_dir, char **new_dir)
 {
@@ -1654,6 +1679,84 @@ static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs,
 	return dir_renames;
 }
 
+static struct dir_rename_entry *check_dir_renamed(const char *path,
+						  struct hashmap *dir_renames)
+{
+	char temp[PATH_MAX];
+	char *end;
+	struct dir_rename_entry *entry;
+
+	strcpy(temp, path);
+	while ((end = strrchr(temp, '/'))) {
+		*end = '\0';
+		entry = dir_rename_find_entry(dir_renames, temp);
+		if (entry)
+			return entry;
+	}
+	return NULL;
+}
+
+static void compute_collisions(struct hashmap *collisions,
+			       struct hashmap *dir_renames,
+			       struct diff_queue_struct *pairs)
+{
+	int i;
+
+	/*
+	 * Multiple files can be mapped to the same path due to directory
+	 * renames done by the other side of history.  Since that other
+	 * side of history could have merged multiple directories into one,
+	 * if our side of history added the same file basename to each of
+	 * those directories, then all N of them would get implicitly
+	 * renamed by the directory rename detection into the same path,
+	 * and we'd get an add/add/.../add conflict, and all those adds
+	 * from *this* side of history.  This is not representable in the
+	 * index, and users aren't going to easily be able to make sense of
+	 * it.  So we need to provide a good warning about what's
+	 * happening, and fall back to no-directory-rename detection
+	 * behavior for those paths.
+	 *
+	 * See testcases 9e and all of section 5 from t6043 for examples.
+	 */
+	collision_init(collisions);
+
+	for (i = 0; i < pairs->nr; ++i) {
+		struct dir_rename_entry *dir_rename_ent;
+		struct collision_entry *collision_ent;
+		char *new_path;
+		struct diff_filepair *pair = pairs->queue[i];
+
+		if (pair->status == 'D')
+			continue;
+		dir_rename_ent = check_dir_renamed(pair->two->path,
+						   dir_renames);
+		if (!dir_rename_ent)
+			continue;
+
+		new_path = apply_dir_rename(dir_rename_ent, pair->two->path);
+		if (!new_path)
+			/*
+			 * dir_rename_ent->non_unique_new_path is true, which
+			 * means there is no directory rename for us to use,
+			 * which means it won't cause us any additional
+			 * collisions.
+			 */
+			continue;
+		collision_ent = collision_find_entry(collisions, new_path);
+		if (!collision_ent) {
+			collision_ent = xcalloc(1,
+						sizeof(struct collision_entry));
+			hashmap_entry_init(collision_ent, strhash(new_path));
+			hashmap_put(collisions, collision_ent);
+			collision_ent->target_file = new_path;
+		} else {
+			free(new_path);
+		}
+		string_list_insert(&collision_ent->source_files,
+				   pair->two->path);
+	}
+}
+
 /*
  * Get information of all renames which occurred in 'pairs', making use of
  * any implicit directory renames inferred from the other side of history.
@@ -1663,6 +1766,7 @@ static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs,
  */
 static struct string_list *get_renames(struct merge_options *o,
 				       struct diff_queue_struct *pairs,
+				       struct hashmap *dir_renames,
 				       struct tree *tree,
 				       struct tree *o_tree,
 				       struct tree *a_tree,
@@ -1670,8 +1774,12 @@ static struct string_list *get_renames(struct merge_options *o,
 				       struct string_list *entries)
 {
 	int i;
+	struct hashmap collisions;
+	struct hashmap_iter iter;
+	struct collision_entry *e;
 	struct string_list *renames;
 
+	compute_collisions(&collisions, dir_renames, pairs);
 	renames = xcalloc(1, sizeof(struct string_list));
 
 	for (i = 0; i < pairs->nr; ++i) {
@@ -1702,6 +1810,13 @@ static struct string_list *get_renames(struct merge_options *o,
 		item = string_list_insert(renames, pair->one->path);
 		item->util = re;
 	}
+
+	hashmap_iter_init(&collisions, &iter);
+	while ((e = hashmap_iter_next(&iter))) {
+		free(e->target_file);
+		string_list_clear(&e->source_files, 0);
+	}
+	hashmap_free(&collisions, 1);
 	return renames;
 }
 
@@ -2011,9 +2126,11 @@ static int handle_renames(struct merge_options *o,
 					 dir_re_head, head,
 					 dir_re_merge, merge);
 
-	ri->head_renames  = get_renames(o, head_pairs, head,
-					 common, head, merge, entries);
-	ri->merge_renames = get_renames(o, merge_pairs, merge,
+	ri->head_renames  = get_renames(o, head_pairs,
+					dir_re_merge, head,
+					common, head, merge, entries);
+	ri->merge_renames = get_renames(o, merge_pairs,
+					dir_re_head, merge,
 					 common, head, merge, entries);
 	clean = process_renames(o, ri->head_renames, ri->merge_renames);
 
-- 
2.15.0.408.g850bc54b15
Previous: Elijah NewrenNext: Elijah Newren
Message 22 of 40 in “Add directory rename detection to git”
  1. 00/34 Add directory rename detection to gitElijah Newren, Nov 29, 2017
  2. 15/34 merge-recursive: move the get_renames() functionElijah Newren, Nov 29, 2017
  3. 24/34 merge-recursive: add a new hashmap for storing file collisionsElijah Newren, Nov 29, 2017
  4. 13/34 directory rename detection: tests for handling overwriting untracked filesElijah Newren, Nov 29, 2017
  5. 05/34 directory rename detection: directory splitting testcasesElijah Newren, Nov 29, 2017
  6. 12/34 directory rename detection: miscellaneous testcases to complete coverageElijah Newren, Nov 29, 2017
  7. 19/34 merge-recursive: split out code for determining diff_filepairsElijah Newren, Nov 29, 2017
  8. 22/34 merge-recursive: add get_directory_renames()Elijah Newren, Nov 29, 2017
  9. 02/34 merge-recursive: fix logic ordering issueElijah Newren, Nov 29, 2017
  10. 18/34 merge-recursive: make !o->detect_rename codepath more obviousElijah Newren, Nov 29, 2017
  11. 17/34 merge-recursive: fix leaks of allocated renames and diff_filepairsElijah Newren, Nov 29, 2017
  12. 31/34 merge-recursive: fix remaining directory rename + dirty overwrite casesElijah Newren, Nov 29, 2017
  13. 04/34 directory rename detection: basic testcasesElijah Newren, Nov 29, 2017
  14. 26/34 merge-recursive: check for file level conflicts then get new nameElijah Newren, Nov 29, 2017
  15. 16/34 merge-recursive: introduce new functions to handle rename logicElijah Newren, Nov 29, 2017
  16. 28/34 merge-recursive: apply necessary modifications for directory renamesElijah Newren, Nov 29, 2017
  17. 23/34 merge-recursive: check for directory level conflictsElijah Newren, Nov 29, 2017
  18. 06/34 directory rename detection: testcases to avoid taking detection too farElijah Newren, Nov 29, 2017
  19. 11/34 directory rename detection: testcases exploring possibly suboptimal mergesElijah Newren, Nov 29, 2017
  20. 03/34 merge-recursive: add explanation for src_entry and dst_entryElijah Newren, Nov 29, 2017
  21. 14/34 directory rename detection: tests for handling overwriting dirty filesElijah Newren, Nov 29, 2017
  22. 25/34 merge-recursive: add computation of collisions due to dir rename & mergingElijah Newren, Nov 29, 2017
  23. 33/34 merge-recursive: avoid spurious rename/rename conflict from dir renamesElijah Newren, Nov 29, 2017
  24. 27/34 merge-recursive: when comparing files, don't include treesElijah Newren, Nov 29, 2017
  25. 32/34 directory rename detection: new testcases showcasing a pair of bugsElijah Newren, Nov 29, 2017
  26. 21/34 merge-recursive: make a helper function for cleanup for handle_renamesElijah Newren, Nov 29, 2017
  27. 10/34 directory rename detection: more involved edge/corner testcasesElijah Newren, Nov 29, 2017
  28. 20/34 merge-recursive: add a new hashmap for storing directory renamesElijah Newren, Nov 29, 2017
  29. 09/34 directory rename detection: testcases checking which side did the renameElijah Newren, Nov 29, 2017
  30. 30/34 merge-recursive: fix overwriting dirty files involved in renamesElijah Newren, Nov 29, 2017
  31. 29/34 merge-recursive: avoid clobbering untracked files with directory renamesElijah Newren, Nov 29, 2017
  32. 08/34 directory rename detection: files/directories in the way of some renamesElijah Newren, Nov 29, 2017
  33. 34/34 merge-recursive: ensure we write updates for directory-renamed fileElijah Newren, Nov 29, 2017
  34. 07/34 directory rename detection: partially renamed directory testcase/discussionElijah Newren, Nov 29, 2017
  35. 01/34 Tighten and correct a few testcases for merging and cherry-pickingElijah Newren, Nov 29, 2017
  36. Junio C HamanoDec 13, 2017
  37. Junio C HamanoDec 13, 2017
  38. Elijah NewrenDec 13, 2017
  39. Ramsay JonesDec 13, 2017
  40. Junio C HamanoDec 13, 2017

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.