[PATCH 2/3] merge: handle renames with replacement content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



We generally think of a rename as removing one path entirely
and placing similar content at a new path. In other words,
after a rename, the original path is now empty. But that is
not necessarily the case with rewrite detection (which is
not currently possible to do for merge-recursive).

The current merge code blindly removes paths that are used
as rename sources; however, we should check to see if there
is useful content at that path.  There are basically two
interesting cases:

  1. One side renames a path, but also puts new content
     (or a symlink) at the same path. We want to detect the
     rename, and have changes from the other side applied to
     the rename destination. The new content at the original
     path should be left untouched.

     The current code just calls remove_file, but that
     ignores the concept that the renaming side may put
     something else useful there. We should detect this case
     and either remove (if no new content), or put the new
     content in place at the original path.

  2. Both sides renamed and installed new content at the
     original path. If they didn't rename to the same
     destination, it is a conflict, and we already mark it
     as such. But if it's the same destination, then it's
     not a conflict; the renamed content will be merged at
     the new destination.

     For the new content at the original path, we have to do
     a 3-way merge. The base must be the null sha1, because
     this "slot" for content didn't exist before (it was
     taken up by the content which got renamed away). So if
     only one side installed new content, that content
     automatically wins. If both sides did, and it is the
     same content, then that content is OK. But if the
     content is different, then we have a conflict and
     should do the usual conflict-markers thing.

This patch implements the semantics described above, which
lays the groundwork for turning on rewrite detection.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
I split this one out for easier review. The semantics I am
proposing are a superset of the current "remove" behavior (it's just
that without break detection on, you can't trigger the other cases). But
by putting this in before enabling break detection, you can test easily
that the new code isn't breaking any existing behavior.

 merge-recursive.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 8e82a8b..16263b0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -706,6 +706,64 @@ static void update_file(struct merge_options *o,
 	update_file_flags(o, sha, mode, path, o->call_depth || clean, !o->call_depth);
 }
 
+static int update_or_remove(struct merge_options *o,
+			    const unsigned char *sha1, unsigned mode,
+			    const char *path, int update_wd)
+{
+	if (is_null_sha1(sha1))
+		return remove_file(o, 1, path, !update_wd);
+
+	update_file_flags(o, sha1, mode, path, 1, update_wd);
+	return 0;
+}
+
+static void merge_rename_source(struct merge_options *o,
+				       const char *path,
+				       struct stage_data *d)
+{
+	if (is_null_sha1(d->stages[2].sha)) {
+		/*
+		 * If both were real renames (not from a broken pair), we can
+		 * stop caring about the path. We don't touch the working
+		 * directory, though. The path must be gone in HEAD, so there
+		 * is no point (and anything we did delete would be an
+		 * untracked file).
+		 */
+		if (is_null_sha1(d->stages[3].sha)) {
+			remove_file(o, 1, path, 1);
+			return;
+		}
+
+		/*
+		 * If "ours" was a real rename, but the other side came
+		 * from a broken pair, then their version is the right
+		 * resolution (because we have no content, ours having been
+		 * renamed away, and they have new content).
+		 */
+		update_file_flags(o, d->stages[3].sha, d->stages[3].mode,
+				  path, 1, 1);
+		return;
+	}
+
+	/*
+	 * Now we have the opposite. "theirs" is a real rename, but ours
+	 * is from a broken pair. We resolve in favor of us, but we don't
+	 * need to touch the working directory.
+	 */
+	if (is_null_sha1(d->stages[3].sha)) {
+		update_file_flags(o, d->stages[2].sha, d->stages[2].mode,
+				  path, 1, 0);
+		return;
+	}
+
+	/*
+	 * Otherwise, both came from broken pairs. We need to do an actual
+	 * merge on the entries. We can just mark it as unprocessed and
+	 * the regular code will handle it.
+	 */
+	d->processed = 0;
+}
+
 /* Low level file merging, update and removal */
 
 struct merge_file_info {
@@ -1025,7 +1083,7 @@ static int process_renames(struct merge_options *o,
 							      ren1->dst_entry,
 							      ren2->dst_entry);
 			} else {
-				remove_file(o, 1, ren1_src, 1);
+				merge_rename_source(o, ren1_src, ren1->src_entry);
 				update_stages_and_entry(ren1_dst,
 							ren1->dst_entry,
 							ren1->pair->one,
@@ -1049,7 +1107,10 @@ static int process_renames(struct merge_options *o,
 			int renamed_stage = a_renames == renames1 ? 2 : 3;
 			int other_stage =   a_renames == renames1 ? 3 : 2;
 
-			remove_file(o, 1, ren1_src, o->call_depth || renamed_stage == 2);
+			update_or_remove(o,
+				ren1->src_entry->stages[renamed_stage].sha,
+				ren1->src_entry->stages[renamed_stage].mode,
+				ren1_src, renamed_stage == 3);
 
 			hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);
 			src_other.mode = ren1->src_entry->stages[other_stage].mode;
-- 
1.7.4.41.g423da.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]