merge-recursive: When we detect we can skip an update, actually skip it

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Aug 11, 2011 at 23:20 UTC 5b448b8530308b1f5a7a721cb1bf0ba557b5c78d
2 files changed +18 -5
merge-recursive.c
+16 -3
@@ -1354,6 +1354,7 @@ static int merge_content(struct merge_options *o,
1354 {
1355 const char *reason = "content";
1356 char *side1 = NULL, *side2 = NULL;
1357 + const char *path1 = NULL, *path2 = NULL;
1358 struct merge_file_info mfi;
1359 struct diff_filespec one, a, b;
1360 unsigned df_conflict_remains = 0;
@@ -1371,7 +1372,6 @@ static int merge_content(struct merge_options *o,
1372 b.mode = b_mode;
1373
1374 if (rename_conflict_info) {
1374 - const char *path1, *path2;
1375 struct diff_filepair *pair1 = rename_conflict_info->pair1;
1376
1377 path1 = (o->branch1 == rename_conflict_info->branch1) ?
@@ -1397,9 +1397,22 @@ static int merge_content(struct merge_options *o,
1397 free(side2);
1398
1399 if (mfi.clean && !df_conflict_remains &&
1400 - sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode)
1400 + sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) {
1401 + int path_renamed_outside_HEAD;
1402 output(o, 3, "Skipped %s (merged same as existing)", path);
1402 - else
1403 + /*
1404 + * The content merge resulted in the same file contents we
1405 + * already had. We can return early if those file contents
1406 + * are recorded at the correct path (which may not be true
1407 + * if the merge involves a rename).
1408 + */
1409 + path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
1410 + if (!path_renamed_outside_HEAD) {
1411 + add_cacheinfo(mfi.mode, mfi.sha, path,
1412 + 0 /*stage*/, 1 /*refresh*/, 0 /*options*/);
1413 + return mfi.clean;
1414 + }
1415 + } else
1416 output(o, 2, "Auto-merging %s", path);
1417
1418 if (!mfi.clean) {
t/t6022-merge-rename.sh
+2 -2
@@ -631,7 +631,7 @@ test_expect_success 'setup avoid unnecessary update, normal rename' '
631 git commit -m "Random, unrelated changes"
632 '
633
634 -test_expect_failure 'avoid unnecessary update, normal rename' '
634 +test_expect_success 'avoid unnecessary update, normal rename' '
635 git checkout -q avoid-unnecessary-update-1^0 &&
636 test-chmtime =1000000000 rename &&
637 test-chmtime -v +0 rename >expect &&
@@ -664,7 +664,7 @@ test_expect_success 'setup to test avoiding unnecessary update, with D/F conflic
664 git commit -m "Only unrelated changes"
665 '
666
667 -test_expect_failure 'avoid unnecessary update, with D/F conflict' '
667 +test_expect_success 'avoid unnecessary update, with D/F conflict' '
668 git checkout -q avoid-unnecessary-update-2^0 &&
669 test-chmtime =1000000000 df &&
670 test-chmtime -v +0 df >expect &&