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, Stephen Rothwell <sfr@canb.auug.org.au>,
	Jeff King <peff@peff.net>, Elijah Newren <newren@gmail.com>
Subject: [PATCHv2 3/3] merge-recursive: When we detect we can skip an update, actually skip it
Date: Mon, 28 Feb 2011 18:08:52 -0700	[thread overview]
Message-ID: <1298941732-19763-4-git-send-email-newren@gmail.com> (raw)
In-Reply-To: <1298941732-19763-1-git-send-email-newren@gmail.com>

In 882fd11 (merge-recursive: Delay content merging for renames 2010-09-20),
there was code that checked for whether we could skip updating a file in
the working directory, based on whether the merged version matched the
current working copy.  Due to the desire to handle directory/file conflicts
that were resolvable, that commit deferred content merging by first
updating the index with the unmerged entries and then moving the actual
merging (along with the skip-the-content-update check) to another function
that ran later in the merge process.  As part moving the content merging
code, a bug was introduced such that although the message about skipping
the update would be printed (whenever GIT_MERGE_VERBOSITY was sufficiently
high), the file would be unconditionally updated in the working copy
anyway.

When we detect that the file does not need to be updated in the working
copy, update the index appropriately and then return early before updating
the working copy.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c       |   17 +++++++++++------
 t/t6022-merge-rename.sh |    2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 16c2dbe..ec07a30 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -354,10 +354,11 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,
 	 * make room for the corresponding directory.  Such paths will
 	 * later be processed in process_df_entry() at the end.  If
 	 * the corresponding directory ends up being removed by the
-	 * merge, then the file will be reinstated at that time;
-	 * otherwise, if the file is not supposed to be removed by the
-	 * merge, the contents of the file will be placed in another
-	 * unique filename.
+	 * merge, then the file will be reinstated at that time
+	 * (albeit with a different timestamp!); otherwise, if the
+	 * file is not supposed to be removed by the merge, the
+	 * contents of the file will be placed in another unique
+	 * filename.
 	 *
 	 * NOTE: This function relies on the fact that entries for a
 	 * D/F conflict will appear adjacent in the index, with the
@@ -1274,9 +1275,13 @@ static int merge_content(struct merge_options *o,
 	}
 
 	if (mfi.clean && !df_conflict_remains &&
-	    sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode)
+	    sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode &&
+	    lstat(path, &st) == 0) {
 		output(o, 3, "Skipped %s (merged same as existing)", path);
-	else
+		add_cacheinfo(mfi.mode, mfi.sha, path,
+			      0 /*stage*/, 1 /*refresh*/, 0 /*options*/);
+		return mfi.clean;
+	} else
 		output(o, 2, "Auto-merging %s", path);
 
 	if (!mfi.clean) {
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index a5d252b..a44117a 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -630,7 +630,7 @@ test_expect_success 'setup avoid unnecessary update, normal rename' '
 	git commit -m "Random, unrelated changes"
 '
 
-test_expect_failure 'avoid unnecessary update, normal rename' '
+test_expect_success 'avoid unnecessary update, normal rename' '
 	git checkout -q avoid-unnecessary-update-1^0 &&
 	touch -t 197001010000.01 rename &&
 	orig=$(stat --format="%Y" rename) &&
-- 
1.7.4

  parent reply	other threads:[~2011-03-01  1:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-01  1:08 [PATCHv2 0/3] Fix unnecessary (mtime) updates of files during merge Elijah Newren
2011-03-01  1:08 ` [PATCHv2 1/3] t6022: New test checking for unnecessary updates of renamed+modified files Elijah Newren
2011-03-01  7:33   ` Johannes Sixt
2011-03-01 19:38     ` Jeff King
2011-03-02 22:26     ` Elijah Newren
2011-03-01  1:08 ` [PATCHv2 2/3] t6022: New test checking for unnecessary updates of files in D/F conflicts Elijah Newren
2011-03-01  1:08 ` Elijah Newren [this message]
2011-03-02 20:19   ` [PATCHv2 3/3] merge-recursive: When we detect we can skip an update, actually skip it Junio C Hamano
2011-03-02 22:22     ` Elijah Newren
2011-03-02 23:23       ` Junio C Hamano
2011-03-01 19:31 ` [PATCHv2 0/3] Fix unnecessary (mtime) updates of files during merge Jeff King
2011-03-01 19:36   ` Jeff King
2011-03-02 23:11   ` Elijah Newren
2011-03-02 23:22 ` 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

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

  git send-email \
    --in-reply-to=1298941732-19763-4-git-send-email-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sfr@canb.auug.org.au \
    /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).