From: Elijah Newren <newren@gmail.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org, Jim Foucar <jgfouca@sandia.gov>,
Elijah Newren <newren@gmail.com>
Subject: [PATCHv2 54/56] merge-recursive: Avoid unnecessary file rewrites
Date: Thu, 11 Aug 2011 23:20:27 -0600 [thread overview]
Message-ID: <1313126429-17368-55-git-send-email-newren@gmail.com> (raw)
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Often times, a potential conflict at a path is resolved by merge-recursive
by using the content that was already present at that location. In such
cases, we do not want to overwrite the content that is already present, as
that could trigger unnecessary recompilations. One of the patches earlier
in this series ("merge-recursive: When we detect we can skip an update,
actually skip it") fixed the cases that involved content merges, but there
were a few other cases as well.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Completely new patch (about a problem that existed previous to v1, but which
I hadn't noticed).
merge-recursive.c | 30 ++++++++++++++++++++++++------
t/t6022-merge-rename.sh | 6 +++---
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index b2deb53..f088132 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1038,7 +1038,14 @@ static void handle_change_delete(struct merge_options *o,
change_past, o->branch1, o->branch1, path,
NULL == renamed ? "" : " at ",
NULL == renamed ? "" : renamed);
- update_file(o, 0, a_sha, a_mode, renamed ? renamed : path);
+ if (renamed)
+ update_file(o, 0, a_sha, a_mode, renamed);
+ /*
+ * No need to call update_file() on path when !renamed, since
+ * that would needlessly touch path. We could call
+ * update_file_flags() with update_cache=0 and update_wd=0,
+ * but that's a no-op.
+ */
}
free(renamed);
}
@@ -1398,10 +1405,20 @@ static int process_renames(struct merge_options *o,
NULL);
} 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);
+ /*
+ * Added file on the other side identical to
+ * the file being renamed: clean merge.
+ * Also, there is no need to overwrite the
+ * file already in the working copy, so call
+ * update_file_flags() instead of
+ * update_file().
+ */
+ update_file_flags(o,
+ ren1->pair->two->sha1,
+ ren1->pair->two->mode,
+ ren1_dst,
+ 1, /* update_cache */
+ 0 /* update_wd */);
} else if (!sha_eq(dst_other.sha1, null_sha1)) {
clean_merge = 0;
try_merge = 1;
@@ -1729,7 +1746,8 @@ static int process_entry(struct merge_options *o,
free(new_path);
} else {
output(o, 2, "Adding %s", path);
- update_file(o, 1, sha, mode, path);
+ /* do not overwrite file if already present */
+ update_file_flags(o, sha, mode, path, 1, !a_sha);
}
} else if (a_sha && b_sha) {
/* Case C: Added in both (check for same permissions) and */
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index c2993fc..9d8584e 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -696,7 +696,7 @@ test_expect_success 'setup avoid unnecessary update, dir->(file,nothing)' '
git commit -m "Add a newfile"
'
-test_expect_failure 'avoid unnecessary update, dir->(file,nothing)' '
+test_expect_success 'avoid unnecessary update, dir->(file,nothing)' '
git checkout -q master^0 &&
test-chmtime =1000000000 df &&
test-chmtime -v +0 df >expect &&
@@ -726,7 +726,7 @@ test_expect_success 'setup avoid unnecessary update, modify/delete' '
git commit -m "Modify file"
'
-test_expect_failure 'avoid unnecessary update, modify/delete' '
+test_expect_success 'avoid unnecessary update, modify/delete' '
git checkout -q master^0 &&
test-chmtime =1000000000 file &&
test-chmtime -v +0 file >expect &&
@@ -755,7 +755,7 @@ test_expect_success 'setup avoid unnecessary update, rename/add-dest' '
git commit -m "Rename file"
'
-test_expect_failure 'avoid unnecessary update, rename/add-dest' '
+test_expect_success 'avoid unnecessary update, rename/add-dest' '
git checkout -q master^0 &&
test-chmtime =1000000000 newfile &&
test-chmtime -v +0 newfile >expect &&
--
1.7.6.100.gac5c1
next prev parent reply other threads:[~2011-08-12 5:23 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-12 5:19 [PATCHv2 00/57] Re-roll of en/merge-recursive from pu Elijah Newren
2011-08-12 5:19 ` [PATCHv2 01/56] t6042: Add a testcase where git deletes an untracked file Elijah Newren
2011-08-12 5:19 ` [PATCHv2 02/56] t6042: Add failing testcase for rename/modify/add-source conflict Elijah Newren
2011-08-12 5:19 ` [PATCHv2 03/56] t6042: Add a pair of cases where undetected renames cause issues Elijah Newren
2011-08-12 5:19 ` [PATCHv2 04/56] t6042: Add a testcase where undetected rename causes silent file deletion Elijah Newren
2011-08-12 5:19 ` [PATCHv2 05/56] t6042: Add tests for content issues with modify/rename/directory conflicts Elijah Newren
2011-08-12 5:19 ` [PATCHv2 06/56] t6042: Ensure rename/rename conflicts leave index and workdir in sane state Elijah Newren
2011-08-12 5:19 ` [PATCHv2 07/56] t6042: Add failing testcases for rename/rename/add-{source,dest} conflicts Elijah Newren
2011-08-12 5:19 ` [PATCHv2 08/56] t6036: Add differently resolved modify/delete conflict in criss-cross test Elijah Newren
2011-08-12 5:19 ` [PATCHv2 09/56] t6036: criss-cross with weird content can fool git into clean merge Elijah Newren
2011-08-12 5:19 ` [PATCHv2 10/56] t6036: tests for criss-cross merges with various directory/file conflicts Elijah Newren
2011-08-12 5:19 ` [PATCHv2 11/56] t6036: criss-cross w/ rename/rename(1to2)/modify+rename/rename(2to1)/modify Elijah Newren
2011-08-12 5:19 ` [PATCHv2 12/56] t6036: criss-cross + rename/rename(1to2)/add-source + modify/modify Elijah Newren
2011-08-12 5:19 ` [PATCHv2 13/56] t6022: Remove unnecessary untracked files to make test cleaner Elijah Newren
2011-08-12 5:19 ` [PATCHv2 14/56] t6022: New tests checking for unnecessary updates of files Elijah Newren
2011-08-12 5:19 ` [PATCHv2 15/56] t6022: Add testcase for merging a renamed file with a simple change Elijah Newren
2011-08-12 5:19 ` [PATCHv2 16/56] merge-recursive: Make BUG message more legible by adding a newline Elijah Newren
2011-08-12 5:19 ` [PATCHv2 17/56] merge-recursive: Correct a comment Elijah Newren
2011-08-12 5:19 ` [PATCHv2 18/56] merge-recursive: Mark some diff_filespec struct arguments const Elijah Newren
2011-08-12 5:19 ` [PATCHv2 19/56] merge-recursive: Consolidate different update_stages functions Elijah Newren
2011-08-12 5:19 ` [PATCHv2 20/56] merge-recursive: Remember to free generated unique path names Elijah Newren
2011-08-12 5:19 ` [PATCHv2 21/56] merge-recursive: Avoid working directory changes during recursive case Elijah Newren
2011-08-12 5:19 ` [PATCHv2 22/56] merge-recursive: Fix recursive case with D/F conflict via add/add conflict Elijah Newren
2011-08-12 5:19 ` [PATCHv2 23/56] merge-recursive: Fix sorting order and directory change assumptions Elijah Newren
2011-08-12 5:19 ` [PATCHv2 24/56] merge-recursive: Fix code checking for D/F conflicts still being present Elijah Newren
2011-08-12 5:19 ` [PATCHv2 25/56] merge-recursive: Save D/F conflict filenames instead of unlinking them Elijah Newren
2011-08-12 5:19 ` [PATCHv2 26/56] merge-recursive: Split was_tracked() out of would_lose_untracked() Elijah Newren
2011-08-12 5:20 ` [PATCHv2 27/56] string-list: Add API to remove an item from an unsorted list Elijah Newren
2011-08-12 7:00 ` Johannes Sixt
2011-08-12 9:27 ` Alex Riesen
2011-08-12 22:14 ` Elijah Newren
2011-08-13 9:08 ` Johannes Sixt
2011-08-12 5:20 ` [PATCHv2 28/56] merge-recursive: Allow make_room_for_path() to remove D/F entries Elijah Newren
2011-08-12 5:20 ` [PATCHv2 29/56] merge-recursive: Split update_stages_and_entry; only update stages at end Elijah Newren
2011-08-12 5:20 ` [PATCHv2 30/56] merge-recursive: Fix deletion of untracked file in rename/delete conflicts Elijah Newren
2011-08-12 5:20 ` [PATCHv2 31/56] merge-recursive: Make dead code for rename/rename(2to1) conflicts undead Elijah Newren
2011-08-12 5:20 ` [PATCHv2 32/56] merge-recursive: Add comments about handling rename/add-source cases Elijah Newren
2011-08-12 5:20 ` [PATCHv2 33/56] merge-recursive: Improve handling of rename target vs. directory addition Elijah Newren
2011-08-12 5:20 ` [PATCHv2 34/56] merge-recursive: Consolidate process_entry() and process_df_entry() Elijah Newren
2011-08-12 5:20 ` [PATCHv2 35/56] merge-recursive: Cleanup and consolidation of rename_conflict_info Elijah Newren
2011-08-12 5:20 ` [PATCHv2 36/56] merge-recursive: Provide more info in conflict markers with file renames Elijah Newren
2011-08-12 5:20 ` [PATCHv2 37/56] merge-recursive: When we detect we can skip an update, actually skip it Elijah Newren
2011-08-12 5:20 ` [PATCHv2 38/56] merge-recursive: Fix modify/delete resolution in the recursive case Elijah Newren
2011-08-12 5:20 ` [PATCHv2 39/56] merge-recursive: Introduce a merge_file convenience function Elijah Newren
2011-08-12 5:20 ` [PATCHv2 40/56] merge-recursive: Fix rename/rename(1to2) resolution for virtual merge base Elijah Newren
2011-08-12 5:20 ` [PATCHv2 41/56] merge-recursive: Small cleanups for conflict_rename_rename_1to2 Elijah Newren
2011-08-12 5:20 ` [PATCHv2 42/56] merge-recursive: Defer rename/rename(2to1) handling until process_entry Elijah Newren
2011-08-12 5:20 ` [PATCHv2 43/56] merge-recursive: Record more data needed for merging with dual renames Elijah Newren
2011-08-12 5:20 ` [PATCHv2 44/56] merge-recursive: Create function for merging with branchname:file markers Elijah Newren
2011-08-12 5:20 ` [PATCHv2 45/56] merge-recursive: Consider modifications in rename/rename(2to1) conflicts Elijah Newren
2011-08-12 5:20 ` [PATCHv2 46/56] merge-recursive: Make modify/delete handling code reusable Elijah Newren
2011-08-12 5:20 ` [PATCHv2 47/56] merge-recursive: Have conflict_rename_delete reuse modify/delete code Elijah Newren
2011-08-12 5:20 ` [PATCHv2 48/56] merge-recursive: add handling for rename/rename/add-dest/add-dest Elijah Newren
2011-08-12 5:20 ` [PATCHv2 49/56] merge-recursive: Fix working copy handling for rename/rename/add/add Elijah Newren
2011-08-12 5:20 ` [PATCHv2 50/56] t3030: fix accidental success in symlink rename Elijah Newren
2011-08-12 5:20 ` [PATCHv2 51/56] t6022: Add testcase for spurious "refusing to lose untracked" messages Elijah Newren
2011-08-12 5:20 ` [PATCHv2 52/56] merge-recursive: Fix spurious 'refusing to lose untracked file...' messages Elijah Newren
2011-08-12 5:20 ` [PATCHv2 53/56] t6022: Additional tests checking for unnecessary updates of files Elijah Newren
2011-08-12 5:20 ` Elijah Newren [this message]
2011-08-12 5:20 ` [PATCHv2 55/56] t6036: criss-cross + rename/rename(1to2)/add-dest + simple modify Elijah Newren
2011-08-12 5:20 ` [PATCHv2 56/56] merge-recursive: Fix virtual merge base for rename/rename(1to2)/add-dest Elijah Newren
2011-08-12 5:48 ` [PATCHv2 00/57] Re-roll of en/merge-recursive from pu Junio C Hamano
2011-08-12 21:59 ` Elijah Newren
2011-08-13 2:37 ` Elijah Newren
2011-08-13 2:23 ` [PATCHv2 57/57] merge-recursive: Don't re-sort a list whose order we depend upon 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=1313126429-17368-55-git-send-email-newren@gmail.com \
--to=newren@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jgfouca@sandia.gov \
/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).