git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, Elijah Newren <newren@gmail.com>
Subject: [PATCH v3 24/33] merge-recursive: add computation of collisions due to dir rename & merging
Date: Tue, 21 Nov 2017 00:00:50 -0800	[thread overview]
Message-ID: <20171121080059.32304-25-newren@gmail.com> (raw)
In-Reply-To: <20171121080059.32304-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 | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 121 insertions(+), 3 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 1fa3eb6fb5..b8c7d6dce3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1416,6 +1416,32 @@ 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)
+{
+	char *new_path;
+	int entrylen, oldlen, newlen;
+
+	if (entry->non_unique_new_dir)
+		return NULL;
+
+	entrylen = strlen(entry->new_dir);
+	oldlen = strlen(entry->dir);
+	newlen = entrylen + (strlen(old_path) - oldlen) + 1;
+	new_path = malloc(newlen);
+	strcpy(new_path, entry->new_dir);
+	strcpy(&new_path[entrylen], &old_path[oldlen]);
+
+	return new_path;
+}
+
 static void get_renamed_dir_portion(const char *old_path, const char *new_path,
 				    char **old_dir, char **new_dir)
 {
@@ -1641,6 +1667,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.
@@ -1650,6 +1754,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,
@@ -1657,8 +1762,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) {
@@ -1689,6 +1798,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;
 }
 
@@ -1981,9 +2097,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.309.g62ce55426d


  parent reply	other threads:[~2017-11-21  8:01 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-21  8:00 [PATCH v3 00/33] Add directory rename detection to git Elijah Newren
2017-11-21  8:00 ` [PATCH v3 01/33] Tighten and correct a few testcases for merging and cherry-picking Elijah Newren
2017-11-21  8:00 ` [PATCH v3 02/33] merge-recursive: fix logic ordering issue Elijah Newren
2017-11-21  8:00 ` [PATCH v3 03/33] merge-recursive: add explanation for src_entry and dst_entry Elijah Newren
2017-11-21  8:00 ` [PATCH v3 04/33] directory rename detection: basic testcases Elijah Newren
2017-11-21  8:00 ` [PATCH v3 05/33] directory rename detection: directory splitting testcases Elijah Newren
2017-11-21  8:00 ` [PATCH v3 06/33] directory rename detection: testcases to avoid taking detection too far Elijah Newren
2017-11-21  8:00 ` [PATCH v3 07/33] directory rename detection: partially renamed directory testcase/discussion Elijah Newren
2017-11-21  8:00 ` [PATCH v3 08/33] directory rename detection: files/directories in the way of some renames Elijah Newren
2017-11-21  8:00 ` [PATCH v3 09/33] directory rename detection: testcases checking which side did the rename Elijah Newren
2017-11-21  8:00 ` [PATCH v3 10/33] directory rename detection: more involved edge/corner testcases Elijah Newren
2017-11-21  8:00 ` [PATCH v3 11/33] directory rename detection: testcases exploring possibly suboptimal merges Elijah Newren
2017-11-22 18:29   ` Stefan Beller
2017-11-21  8:00 ` [PATCH v3 12/33] directory rename detection: miscellaneous testcases to complete coverage Elijah Newren
2017-11-21  8:00 ` [PATCH v3 13/33] directory rename detection: tests for handling overwriting untracked files Elijah Newren
2017-11-21  8:00 ` [PATCH v3 14/33] directory rename detection: tests for handling overwriting dirty files Elijah Newren
2017-11-21  8:00 ` [PATCH v3 15/33] merge-recursive: move the get_renames() function Elijah Newren
2017-11-21  8:00 ` [PATCH v3 16/33] merge-recursive: introduce new functions to handle rename logic Elijah Newren
2017-11-21  8:00 ` [PATCH v3 17/33] merge-recursive: fix leaks of allocated renames and diff_filepairs Elijah Newren
2017-11-21  8:00 ` [PATCH v3 18/33] merge-recursive: make !o->detect_rename codepath more obvious Elijah Newren
2017-11-21  8:00 ` [PATCH v3 19/33] merge-recursive: split out code for determining diff_filepairs Elijah Newren
2017-11-21  8:00 ` [PATCH v3 20/33] merge-recursive: add a new hashmap for storing directory renames Elijah Newren
2017-11-21  8:00 ` [PATCH v3 21/33] merge-recursive: add get_directory_renames() Elijah Newren
2017-11-26  0:52   ` Johannes Schindelin
2017-11-26  1:45     ` Elijah Newren
2017-11-21  8:00 ` [PATCH v3 22/33] merge-recursive: check for directory level conflicts Elijah Newren
2017-11-22 18:36   ` Stefan Beller
2017-11-21  8:00 ` [PATCH v3 23/33] merge-recursive: add a new hashmap for storing file collisions Elijah Newren
2017-11-21  8:00 ` Elijah Newren [this message]
2017-11-21  8:00 ` [PATCH v3 25/33] merge-recursive: check for file level conflicts then get new name Elijah Newren
2017-11-22 18:56   ` Stefan Beller
2017-11-21  8:00 ` [PATCH v3 26/33] merge-recursive: when comparing files, don't include trees Elijah Newren
2017-11-21  8:00 ` [PATCH v3 27/33] merge-recursive: apply necessary modifications for directory renames Elijah Newren
2017-11-21  8:00 ` [PATCH v3 28/33] merge-recursive: avoid clobbering untracked files with " Elijah Newren
2017-11-21  8:00 ` [PATCH v3 29/33] merge-recursive: fix overwriting dirty files involved in renames Elijah Newren
2017-11-21  8:00 ` [PATCH v3 30/33] merge-recursive: fix remaining directory rename + dirty overwrite cases Elijah Newren
2017-11-21  8:00 ` [PATCH v3 31/33] directory rename detection: new testcases showcasing a pair of bugs Elijah Newren
2017-11-21  8:00 ` [PATCH v3 32/33] merge-recursive: avoid spurious rename/rename conflict from dir renames Elijah Newren
2017-11-21  8:00 ` [PATCH v3 33/33] merge-recursive: ensure we write updates for directory-renamed file Elijah Newren
2017-11-22  0:42 ` [PATCH v3 00/33] Add directory rename detection to git Stefan Beller
2017-11-22  1:12   ` Elijah Newren
2017-11-22  2:44     ` Junio C Hamano
2017-11-22 19:24     ` Stefan Beller
2017-11-23  6:22       ` Elijah Newren
2017-11-23 11:52 ` Adam Dinwoodie
2017-11-23 22:28   ` Elijah Newren
2017-11-24  5:25     ` Elijah Newren
2017-11-24 20:07       ` Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171121080059.32304-25-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).