git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: sbeller@google.com, szeder.dev@gmail.com, j6t@kdbg.org,
	jrnieder@gmail.com, peff@peff.net, git@matthieu-moy.fr,
	Elijah Newren <newren@gmail.com>
Subject: [PATCHv6 20/31] merge-recursive: check for directory level conflicts
Date: Fri,  5 Jan 2018 12:27:00 -0800	[thread overview]
Message-ID: <20180105202711.24311-21-newren@gmail.com> (raw)
In-Reply-To: <20180105202711.24311-1-newren@gmail.com>

Before trying to apply directory renames to paths within the given
directories, we want to make sure that there aren't conflicts at the
directory level.  There will be additional checks at the individual
file level too, which will be added later.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/merge-recursive.c b/merge-recursive.c
index 6aef357e7..d92fba277 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1384,6 +1384,15 @@ static struct diff_queue_struct *get_diffpairs(struct merge_options *o,
 	return ret;
 }
 
+static int tree_has_path(struct tree *tree, const char *path)
+{
+	unsigned char hashy[20];
+	unsigned int mode_o;
+
+	return !get_tree_entry(tree->object.oid.hash, path,
+			       hashy, &mode_o);
+}
+
 static void get_renamed_dir_portion(const char *old_path, const char *new_path,
 				    char **old_dir, char **new_dir)
 {
@@ -1438,6 +1447,112 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,
 	}
 }
 
+static void remove_hashmap_entries(struct hashmap *dir_renames,
+				   struct string_list *items_to_remove)
+{
+	int i;
+	struct dir_rename_entry *entry;
+
+	for (i = 0; i < items_to_remove->nr; i++) {
+		entry = items_to_remove->items[i].util;
+		hashmap_remove(dir_renames, entry, NULL);
+	}
+	string_list_clear(items_to_remove, 0);
+}
+
+/*
+ * There are a couple things we want to do at the directory level:
+ *   1. Check for both sides renaming to the same thing, in order to avoid
+ *      implicit renaming of files that should be left in place.  (See
+ *      testcase 6b in t6043 for details.)
+ *   2. Prune directory renames if there are still files left in the
+ *      the original directory.  These represent a partial directory rename,
+ *      i.e. a rename where only some of the files within the directory
+ *      were renamed elsewhere.  (Technically, this could be done earlier
+ *      in get_directory_renames(), except that would prevent us from
+ *      doing the previous check and thus failing testcase 6b.)
+ *   3. Check for rename/rename(1to2) conflicts (at the directory level).
+ *      In the future, we could potentially record this info as well and
+ *      omit reporting rename/rename(1to2) conflicts for each path within
+ *      the affected directories, thus cleaning up the merge output.
+ *   NOTE: We do NOT check for rename/rename(2to1) conflicts at the
+ *         directory level, because merging directories is fine.  If it
+ *         causes conflicts for files within those merged directories, then
+ *         that should be detected at the individual path level.
+ */
+static void handle_directory_level_conflicts(struct merge_options *o,
+					     struct hashmap *dir_re_head,
+					     struct tree *head,
+					     struct hashmap *dir_re_merge,
+					     struct tree *merge)
+{
+	struct hashmap_iter iter;
+	struct dir_rename_entry *head_ent;
+	struct dir_rename_entry *merge_ent;
+
+	struct string_list remove_from_head = STRING_LIST_INIT_NODUP;
+	struct string_list remove_from_merge = STRING_LIST_INIT_NODUP;
+
+	hashmap_iter_init(dir_re_head, &iter);
+	while ((head_ent = hashmap_iter_next(&iter))) {
+		merge_ent = dir_rename_find_entry(dir_re_merge, head_ent->dir);
+		if (merge_ent &&
+		    !head_ent->non_unique_new_dir &&
+		    !merge_ent->non_unique_new_dir &&
+		    !strbuf_cmp(&head_ent->new_dir, &merge_ent->new_dir)) {
+			/* 1. Renamed identically; remove it from both sides */
+			string_list_append(&remove_from_head,
+					   head_ent->dir)->util = head_ent;
+			strbuf_release(&head_ent->new_dir);
+			string_list_append(&remove_from_merge,
+					   merge_ent->dir)->util = merge_ent;
+			strbuf_release(&merge_ent->new_dir);
+		} else if (tree_has_path(head, head_ent->dir)) {
+			/* 2. This wasn't a directory rename after all */
+			string_list_append(&remove_from_head,
+					   head_ent->dir)->util = head_ent;
+			strbuf_release(&head_ent->new_dir);
+		}
+	}
+
+	remove_hashmap_entries(dir_re_head, &remove_from_head);
+	remove_hashmap_entries(dir_re_merge, &remove_from_merge);
+
+	hashmap_iter_init(dir_re_merge, &iter);
+	while ((merge_ent = hashmap_iter_next(&iter))) {
+		head_ent = dir_rename_find_entry(dir_re_head, merge_ent->dir);
+		if (tree_has_path(merge, merge_ent->dir)) {
+			/* 2. This wasn't a directory rename after all */
+			string_list_append(&remove_from_merge,
+					   merge_ent->dir)->util = merge_ent;
+		} else if (head_ent &&
+			   !head_ent->non_unique_new_dir &&
+			   !merge_ent->non_unique_new_dir) {
+			/* 3. rename/rename(1to2) */
+			/*
+			 * We can assume it's not rename/rename(1to1) because
+			 * that was case (1), already checked above.  So we
+			 * know that head_ent->new_dir and merge_ent->new_dir
+			 * are different strings.
+			 */
+			output(o, 1, _("CONFLICT (rename/rename): "
+				       "Rename directory %s->%s in %s. "
+				       "Rename directory %s->%s in %s"),
+			       head_ent->dir, head_ent->new_dir.buf, o->branch1,
+			       head_ent->dir, merge_ent->new_dir.buf, o->branch2);
+			string_list_append(&remove_from_head,
+					   head_ent->dir)->util = head_ent;
+			strbuf_release(&head_ent->new_dir);
+			string_list_append(&remove_from_merge,
+					   merge_ent->dir)->util = merge_ent;
+			strbuf_release(&merge_ent->new_dir);
+		}
+	}
+
+	remove_hashmap_entries(dir_re_head, &remove_from_head);
+	remove_hashmap_entries(dir_re_merge, &remove_from_merge);
+}
+
 static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs,
 					     struct tree *tree)
 {
@@ -1869,6 +1984,10 @@ static int handle_renames(struct merge_options *o,
 	dir_re_head = get_directory_renames(head_pairs, head);
 	dir_re_merge = get_directory_renames(merge_pairs, merge);
 
+	handle_directory_level_conflicts(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,
-- 
2.14.2


  parent reply	other threads:[~2018-01-05 20:29 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-05 20:26 [PATCHv6 00/31] Add directory rename detection to git Elijah Newren
2018-01-05 20:26 ` [PATCHv6 01/31] directory rename detection: basic testcases Elijah Newren
2018-01-05 20:26 ` [PATCHv6 02/31] directory rename detection: directory splitting testcases Elijah Newren
2018-01-05 20:26 ` [PATCHv6 03/31] directory rename detection: testcases to avoid taking detection too far Elijah Newren
2018-01-26 11:37   ` SZEDER Gábor
2018-01-26 16:55     ` Elijah Newren
2018-01-05 20:26 ` [PATCHv6 04/31] directory rename detection: partially renamed directory testcase/discussion Elijah Newren
2018-01-05 20:26 ` [PATCHv6 05/31] directory rename detection: files/directories in the way of some renames Elijah Newren
2018-01-05 20:26 ` [PATCHv6 06/31] directory rename detection: testcases checking which side did the rename Elijah Newren
2018-01-05 20:26 ` [PATCHv6 07/31] directory rename detection: more involved edge/corner testcases Elijah Newren
2018-01-05 20:26 ` [PATCHv6 08/31] directory rename detection: testcases exploring possibly suboptimal merges Elijah Newren
2018-01-05 20:26 ` [PATCHv6 09/31] directory rename detection: miscellaneous testcases to complete coverage Elijah Newren
2018-01-05 20:26 ` [PATCHv6 10/31] directory rename detection: tests for handling overwriting untracked files Elijah Newren
2018-01-05 20:26 ` [PATCHv6 11/31] directory rename detection: tests for handling overwriting dirty files Elijah Newren
2018-01-05 20:26 ` [PATCHv6 12/31] merge-recursive: move the get_renames() function Elijah Newren
2018-01-05 20:26 ` [PATCHv6 13/31] merge-recursive: introduce new functions to handle rename logic Elijah Newren
2018-01-05 20:26 ` [PATCHv6 14/31] merge-recursive: fix leaks of allocated renames and diff_filepairs Elijah Newren
2018-01-05 20:26 ` [PATCHv6 15/31] merge-recursive: make !o->detect_rename codepath more obvious Elijah Newren
2018-01-05 20:26 ` [PATCHv6 16/31] merge-recursive: split out code for determining diff_filepairs Elijah Newren
2018-01-05 20:26 ` [PATCHv6 17/31] merge-recursive: add a new hashmap for storing directory renames Elijah Newren
2018-01-05 20:26 ` [PATCHv6 18/31] merge-recursive: make a helper function for cleanup for handle_renames Elijah Newren
2018-01-05 20:26 ` [PATCHv6 19/31] merge-recursive: add get_directory_renames() Elijah Newren
2018-01-05 20:27 ` Elijah Newren [this message]
2018-01-05 20:27 ` [PATCHv6 21/31] merge-recursive: add a new hashmap for storing file collisions Elijah Newren
2018-01-05 20:27 ` [PATCHv6 22/31] merge-recursive: add computation of collisions due to dir rename & merging Elijah Newren
2018-01-05 20:27 ` [PATCHv6 23/31] merge-recursive: check for file level conflicts then get new name Elijah Newren
2018-01-05 20:27 ` [PATCHv6 24/31] merge-recursive: when comparing files, don't include trees Elijah Newren
2018-01-05 20:27 ` [PATCHv6 25/31] merge-recursive: apply necessary modifications for directory renames Elijah Newren
2018-01-05 20:27 ` [PATCHv6 26/31] merge-recursive: avoid clobbering untracked files with " Elijah Newren
2018-01-05 20:27 ` [PATCHv6 27/31] merge-recursive: fix overwriting dirty files involved in renames Elijah Newren
2018-01-05 20:27 ` [PATCHv6 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases Elijah Newren
2018-01-05 20:27 ` [PATCHv6 29/31] directory rename detection: new testcases showcasing a pair of bugs Elijah Newren
2018-01-05 20:27 ` [PATCHv6 30/31] merge-recursive: avoid spurious rename/rename conflict from dir renames Elijah Newren
2018-01-05 20:27 ` [PATCHv6 31/31] merge-recursive: ensure we write updates for directory-renamed file 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=20180105202711.24311-21-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@matthieu-moy.fr \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    --cc=szeder.dev@gmail.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).