git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: sbeller@google.com, gitster@pobox.com, Elijah Newren <newren@gmail.com>
Subject: [PATCH v4 25/34] merge-recursive: add computation of collisions due to dir rename & merging
Date: Tue, 28 Nov 2017 17:42:28 -0800	[thread overview]
Message-ID: <20171129014237.32570-26-newren@gmail.com> (raw)
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


  parent reply	other threads:[~2017-11-29  1:44 UTC|newest]

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

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20171129014237.32570-26-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).