[PATCHv2 37/56] merge-recursive: When we detect we can skip an update, actually skip it

[Elijah Newren <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.

Note that there was a similar change in b2c8c0a (merge-recursive: When we
detect we can skip an update, actually skip it 2011-02-28), but it was
reverted by 6db4105 (Revert "Merge branch 'en/merge-recursive'"
2011-05-19) since it did not fix both of the relevant types of unnecessary
update breakages and, worse, it made use of some band-aids that caused
other problems.  The reason this change works is due to the changes earlier
in this series to (a) record_df_conflict_files instead of just unlinking
them early, (b) allowing make_room_for_path() to remove D/F entries,
(c) the splitting of update_stages_and_entry() to have its functionality
called at different points, and (d) making the pathnames of the files
involved in the merge available to merge_content().

Signed-off-by: Elijah Newren <newren@gmail.com>
---
Changes since v1:
  (1) Moved to later in the series (used to be patch #29).
  (2) Changed the "was_tracked" logic to use information about what
      paths were involved in renames to determine whether the file
      really 'was tracked' before the merge started.  Thus, the code
      is now about checking "!path_renamed_outside_HEAD" rather than
      "was_tracked(path)" (The latter was problematic because it tells
      us whether path was tracked once unpack_trees finishes() rather
      than whether the path was tracked before the merge started.)
  (3) Added another minor tweak to get the expected "Skipped" messages
      that t6022.12 expects.  We need to report that we are skipping
      the update even if the content is only on the other side of the
      merge and we need to update the working directory contents.  At
      least, that's what the author of t6022.12 who put the message in
      explicitly expected.

 merge-recursive.c       |   19 ++++++++++++++++---
 t/t6022-merge-rename.sh |    4 ++--
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index bc99f1a..ccf71d3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1356,6 +1356,7 @@ static int merge_content(struct merge_options *o,
 {
 	const char *reason = "content";
 	char *side1 = NULL, *side2 = NULL;
+	const char *path1 = NULL, *path2 = NULL;
 	struct merge_file_info mfi;
 	struct diff_filespec one, a, b;
 	unsigned df_conflict_remains = 0;
@@ -1373,7 +1374,6 @@ static int merge_content(struct merge_options *o,
 	b.mode = b_mode;
 
 	if (rename_conflict_info) {
-		const char *path1, *path2;
 		struct diff_filepair *pair1 = rename_conflict_info->pair1;
 
 		path1 = (o->branch1 == rename_conflict_info->branch1) ?
@@ -1399,9 +1399,22 @@ static int merge_content(struct merge_options *o,
 	free(side2);
 
 	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) {
+		int path_renamed_outside_HEAD;
 		output(o, 3, "Skipped %s (merged same as existing)", path);
-	else
+		/*
+		 * The content merge resulted in the same file contents we
+		 * already had.  We can return early if those file contents
+		 * are recorded at the correct path (which may not be true
+		 * if the merge involves a rename).
+		 */
+		path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
+		if (!path_renamed_outside_HEAD) {
+			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 4695cbc..d96d3c5 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -631,7 +631,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 &&
 	test-chmtime =1000000000 rename &&
 	test-chmtime -v +0 rename >expect &&
@@ -664,7 +664,7 @@ test_expect_success 'setup to test avoiding unnecessary update, with D/F conflic
 	git commit -m "Only unrelated changes"
 '
 
-test_expect_failure 'avoid unnecessary update, with D/F conflict' '
+test_expect_success 'avoid unnecessary update, with D/F conflict' '
 	git checkout -q avoid-unnecessary-update-2^0 &&
 	test-chmtime =1000000000 df &&
 	test-chmtime -v +0 df >expect &&
-- 
1.7.6.100.gac5c1