Thread (13 messages) flat view 13 messages, 4 authors, 2016-06-15
STALE3750d

[PATCH 2/3] merge-recursive: Small code cleanup

From: Elijah Newren <hidden>
Date: 2016-06-15 22:49:30
Subsystem: the rest · Maintainer: Linus Torvalds

process_renames() had a variable named "stage" and derived variables
src_other and dst_other whose purpose was not entirely clear to me.  Make
the name of stage slightly more descriptive and add a brief comment
explaining what is occurring.

Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are
identical 2010-09-01), a separate if-block was added to provide a special
case for the rename/add conflict case that can be resolved (namely when
the contents on the destination side are identical).  However, as a
separate if block, it's not immediately obvious that its code is related to
the subsequent code checking for a rename/add conflict.  We can combine and
simplify the check slightly.

Signed-off-by: Elijah Newren <redacted>
---
 merge-recursive.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c574698..5e2886a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -923,15 +923,26 @@ static int process_renames(struct merge_options *o,
 			/* Renamed in 1, maybe changed in 2 */
 			struct string_list_item *item;
 			/* we only use sha1 and mode of these */
-			struct diff_filespec src_other, dst_other;
-			int try_merge, stage = a_renames == renames1 ? 3: 2;
+			struct diff_filespec src_other, dst_other, dst_renamed;
+			int try_merge;
 
-			remove_file(o, 1, ren1_src, o->call_depth || stage == 3);
+			/*
+			 * unpack_trees loads entries from common-commit
+			 * into stage 1, from head-commit into stage 2, and
+			 * from merge-commit into stage 3.  We keep track
+			 * of which side corresponds to the rename.
+			 */
+			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);
 
-			hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
-			src_other.mode = ren1->src_entry->stages[stage].mode;
-			hashcpy(dst_other.sha1, ren1->dst_entry->stages[stage].sha);
-			dst_other.mode = ren1->dst_entry->stages[stage].mode;
+			hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);
+			src_other.mode = ren1->src_entry->stages[other_stage].mode;
+			hashcpy(dst_other.sha1, ren1->dst_entry->stages[other_stage].sha);
+			dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
+			hashcpy(dst_renamed.sha1, ren1->dst_entry->stages[renamed_stage].sha);
+			dst_renamed.mode = ren1->dst_entry->stages[renamed_stage].mode;
 
 			try_merge = 0;
 
@@ -955,13 +966,8 @@ static int process_renames(struct merge_options *o,
 							ren1->pair->two : NULL,
 							branch1 == o->branch1 ?
 							NULL : ren1->pair->two, 1);
-			} else if ((dst_other.mode == ren1->pair->two->mode) &&
-				   sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {
-				/* Added file on the other side
-				   identical to the file being
-				   renamed: clean merge */
-				update_file(o, 1, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
-			} else if (!sha_eq(dst_other.sha1, null_sha1)) {
+			} else if (!sha_eq(dst_other.sha1, null_sha1) &&
+				   !sha_eq(dst_other.sha1, dst_renamed.sha1)) {
 				const char *new_path;
 				clean_merge = 0;
 				try_merge = 1;
-- 
1.7.3.rc0.170.g5cfb0.dirty
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help