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, oinksocket@letterboxes.org,
	ken.schalk@intel.com, Elijah Newren <newren@gmail.com>
Subject: [PATCH 2/3] merge-recursive: Small code cleanup
Date: Mon,  6 Sep 2010 14:47:49 -0600	[thread overview]
Message-ID: <1283806070-22027-3-git-send-email-newren@gmail.com> (raw)
In-Reply-To: <AANLkTimz8qSwefp137-D+vEbsf6soG51u0im9EC911_O@mail.gmail.com>

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 <newren@gmail.com>
---
 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

  parent reply	other threads:[~2010-09-06 20:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-02 16:17 cherry-picking a commit clobbers a file which is a directory in the target commit Nick
2010-09-06  9:50 ` Nick
2010-09-06 15:20   ` Elijah Newren
2010-09-06 20:47     ` [PATCH 0/3] Fix resolvable rename + D/F conflict testcases Elijah Newren
2010-09-06 20:49       ` Elijah Newren
2010-09-06 20:47     ` [PATCH 1/3] t3509: Add rename + D/F conflict testcases that recursive strategy fails Elijah Newren
2010-09-06 20:47     ` Elijah Newren [this message]
2010-09-06 21:25       ` [PATCH 2/3] merge-recursive: Small code cleanup Elijah Newren
2010-09-06 23:49         ` Junio C Hamano
2010-09-07 16:23       ` Schalk, Ken
2010-09-08  6:24         ` Elijah Newren
2010-09-09 20:23           ` Schalk, Ken
2010-10-21 19:43             ` Camille Moncelier
2010-09-06 20:47     ` [PATCH 3/3] merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir 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=1283806070-22027-3-git-send-email-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ken.schalk@intel.com \
    --cc=oinksocket@letterboxes.org \
    /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).