{"thread":{"id":"26642","subject":"[PATCHv3 0/3] Fix unnecessary (mtime) updates of files during merge","startedAt":"2011-03-03T06:13:09Z","lastAt":"2011-03-03T17:52:21Z","messageCount":5,"participants":["Elijah Newren","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"162703","messageId":"1299132792-17497-1-git-send-email-newren@gmail.com","threadId":"26642","inReplyTo":null,"subject":"[PATCHv3 0/3] Fix unnecessary (mtime) updates of files during merge","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2011-03-03T06:13:09Z","receivedAt":"2011-03-03T06:13:09Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series fixes a bug reported by Stephen Rothwell -- that\nduring merges git would unnecessarily update modification times of\nfiles.  As noted by both Jeff and Junio, it's a bit of a band-aid\nsince it doesn't dive into fixing make_room_for_path() and\nmake_room_for_directories_of_df_conflicts(), but that would be a\nmuch bigger and more invasive change.\n\nChanges since last version:\n  * Portability fixes for the tests suggested by Hannes (thanks!)\n  * Additional test cleanups suggested by Jeff\n  * Jeff's acks have been added\n\nElijah Newren (3):\n  t6022: New test checking for unnecessary updates of renamed+modified files\n  t6022: New test checking for unnecessary updates of files in D/F conflicts\n  merge-recursive: When we detect we can skip an update, actually skip it\n\n merge-recursive.c       |   17 ++++++++----\n t/t6022-merge-rename.sh |   65 +++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 76 insertions(+), 6 deletions(-)\n\n-- \n1.7.4\n"},{"id":"162705","messageId":"1299132792-17497-2-git-send-email-newren@gmail.com","threadId":"26642","inReplyTo":"1299132792-17497-1-git-send-email-newren@gmail.com","subject":"[PATCHv3 1/3] t6022: New test checking for unnecessary updates of renamed+modified files","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2011-03-03T06:13:10Z","receivedAt":"2011-03-03T06:13:10Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"\nSigned-off-by: Elijah Newren <newren@gmail.com>\nAcked-by: Jeff King <peff@peff.net>\n---\n t/t6022-merge-rename.sh |   31 +++++++++++++++++++++++++++++++\n 1 files changed, 31 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 1ed259d..16330a6 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -609,4 +609,35 @@ test_expect_success 'check handling of differently renamed file with D/F conflic\n \t! test -f original\n '\n \n+test_expect_success 'setup avoid unnecessary update, normal rename' '\n+\tgit reset --hard &&\n+\tgit checkout --orphan avoid-unnecessary-update-1 &&\n+\tgit rm -rf . &&\n+\tgit clean -fdqx &&\n+\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\n10\\n\" >original &&\n+\tgit add -A &&\n+\tgit commit -m \"Common commmit\" &&\n+\n+\tgit mv original rename &&\n+\techo 11 >>rename &&\n+\tgit add -u &&\n+\tgit commit -m \"Renamed and modified\" &&\n+\n+\tgit checkout -b merge-branch-1 HEAD~1 &&\n+\techo \"random content\" >random-file &&\n+\tgit add -A &&\n+\tgit commit -m \"Random, unrelated changes\"\n+'\n+\n+test_expect_failure 'avoid unnecessary update, normal rename' '\n+\tgit checkout -q avoid-unnecessary-update-1^0 &&\n+\ttest-chmtime =1 rename &&\n+\ttest-chmtime -v +0 rename >expect &&\n+\tgit merge merge-branch-1 &&\n+\tgit diff-files --exit-code &&   # Is \"rename\" clean, or stat dirty?\n+\ttest-chmtime -v +0 rename >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.7.4\n"},{"id":"162704","messageId":"1299132792-17497-3-git-send-email-newren@gmail.com","threadId":"26642","inReplyTo":"1299132792-17497-1-git-send-email-newren@gmail.com","subject":"[PATCHv3 2/3] t6022: New test checking for unnecessary updates of files in D/F conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2011-03-03T06:13:11Z","receivedAt":"2011-03-03T06:13:11Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"\nSigned-off-by: Elijah Newren <newren@gmail.com>\nAcked-by: Jeff King <peff@peff.net>\n---\n t/t6022-merge-rename.sh |   34 ++++++++++++++++++++++++++++++++++\n 1 files changed, 34 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 16330a6..88cc410 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -640,4 +640,38 @@ test_expect_failure 'avoid unnecessary update, normal rename' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup avoid unnecessary update, with D/F conflict' '\n+\tgit reset --hard &&\n+\tgit checkout --orphan avoid-unnecessary-update-2 &&\n+\tgit rm -rf . &&\n+\tgit clean -fdqx &&\n+\n+\tmkdir df &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\n10\\n\" >df/file &&\n+\tgit add -A &&\n+\tgit commit -m \"Common commmit\" &&\n+\n+\tgit mv df/file temp &&\n+\trm -rf df &&\n+\tgit mv temp df &&\n+\techo 11 >>df &&\n+\tgit add -u &&\n+\tgit commit -m \"Renamed and modified\" &&\n+\n+\tgit checkout -b merge-branch-2 HEAD~1 &&\n+\t>unrelated-change &&\n+\tgit add unrelated-change &&\n+\tgit commit -m \"Only unrelated changes\"\n+'\n+\n+test_expect_failure 'avoid unnecessary update, with D/F conflict' '\n+\tgit checkout -q avoid-unnecessary-update-2^0 &&\n+\ttest-chmtime =1 rename &&\n+\ttest-chmtime -v +0 rename >expect &&\n+\tgit merge merge-branch-2 &&\n+\tgit diff-files --exit-code &&   # Is \"rename\" clean, or stat dirty?\n+\ttest-chmtime -v +0 rename >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.7.4\n"},{"id":"162706","messageId":"1299132792-17497-4-git-send-email-newren@gmail.com","threadId":"26642","inReplyTo":"1299132792-17497-1-git-send-email-newren@gmail.com","subject":"[PATCHv3 3/3] merge-recursive: When we detect we can skip an update, actually skip it","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2011-03-03T06:13:12Z","receivedAt":"2011-03-03T06:13:12Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"In 882fd11 (merge-recursive: Delay content merging for renames 2010-09-20),\nthere was code that checked for whether we could skip updating a file in\nthe working directory, based on whether the merged version matched the\ncurrent working copy.  Due to the desire to handle directory/file conflicts\nthat were resolvable, that commit deferred content merging by first\nupdating the index with the unmerged entries and then moving the actual\nmerging (along with the skip-the-content-update check) to another function\nthat ran later in the merge process.  As part moving the content merging\ncode, a bug was introduced such that although the message about skipping\nthe update would be printed (whenever GIT_MERGE_VERBOSITY was sufficiently\nhigh), the file would be unconditionally updated in the working copy\nanyway.\n\nWhen we detect that the file does not need to be updated in the working\ncopy, update the index appropriately and then return early before updating\nthe working copy.\n\nReported-by: Stephen Rothwell <sfr@canb.auug.org.au>\nSigned-off-by: Elijah Newren <newren@gmail.com>\nAcked-by: Jeff King <peff@peff.net>\n---\n merge-recursive.c       |   17 +++++++++++------\n t/t6022-merge-rename.sh |    2 +-\n 2 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 16c2dbe..ec07a30 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -354,10 +354,11 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,\n \t * make room for the corresponding directory.  Such paths will\n \t * later be processed in process_df_entry() at the end.  If\n \t * the corresponding directory ends up being removed by the\n-\t * merge, then the file will be reinstated at that time;\n-\t * otherwise, if the file is not supposed to be removed by the\n-\t * merge, the contents of the file will be placed in another\n-\t * unique filename.\n+\t * merge, then the file will be reinstated at that time\n+\t * (albeit with a different timestamp!); otherwise, if the\n+\t * file is not supposed to be removed by the merge, the\n+\t * contents of the file will be placed in another unique\n+\t * filename.\n \t *\n \t * NOTE: This function relies on the fact that entries for a\n \t * D/F conflict will appear adjacent in the index, with the\n@@ -1274,9 +1275,13 @@ static int merge_content(struct merge_options *o,\n \t}\n \n \tif (mfi.clean && !df_conflict_remains &&\n-\t    sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode)\n+\t    sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode &&\n+\t    lstat(path, &st) == 0) {\n \t\toutput(o, 3, \"Skipped %s (merged same as existing)\", path);\n-\telse\n+\t\tadd_cacheinfo(mfi.mode, mfi.sha, path,\n+\t\t\t      0 /*stage*/, 1 /*refresh*/, 0 /*options*/);\n+\t\treturn mfi.clean;\n+\t} else\n \t\toutput(o, 2, \"Auto-merging %s\", path);\n \n \tif (!mfi.clean) {\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 88cc410..6f9df68 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -630,7 +630,7 @@ test_expect_success 'setup avoid unnecessary update, normal rename' '\n \tgit commit -m \"Random, unrelated changes\"\n '\n \n-test_expect_failure 'avoid unnecessary update, normal rename' '\n+test_expect_success 'avoid unnecessary update, normal rename' '\n \tgit checkout -q avoid-unnecessary-update-1^0 &&\n \ttest-chmtime =1 rename &&\n \ttest-chmtime -v +0 rename >expect &&\n-- \n1.7.4\n"},{"id":"162730","messageId":"7vpqq8852i.fsf@alter.siamese.dyndns.org","threadId":"26642","inReplyTo":"1299132792-17497-1-git-send-email-newren@gmail.com","subject":"Re: [PATCHv3 0/3] Fix unnecessary (mtime) updates of files during merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-03T17:52:21Z","receivedAt":"2011-03-03T17:52:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This seems to more-or-less match what I queued in 'pu' yesterday (v2 with\nfix-ups from the list).\n\nThanks.\n"}]}