{"thread":{"id":"24777","subject":"[PATCH 0/2] Fix multiple file rename across D/F conflict","startedAt":"2010-08-17T23:53:17Z","lastAt":"2010-08-20T12:47:37Z","messageCount":6,"participants":["Elijah Newren","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"148322","messageId":"1282089199-17253-1-git-send-email-newren@gmail.com","threadId":"24777","inReplyTo":null,"subject":"[PATCH 0/2] Fix multiple file rename across D/F conflict","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-17T23:53:17Z","receivedAt":"2010-08-17T23:53:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Another D/F conflict not fixed by the en/d-f-conflict-fix series in next\nwas found in the wild, involving multiple file renames across a D/F\nconflict. That series would have fixed this case too, were it not for\na stupid simple bug. Fix it, and add a testcase for good measure.\n\nThis series is on top of next; it needs both the en/d-f-conflict-fix\nseries and the testcase requires em/checkout-orphan.\n\nElijah Newren (2):\n t6031: Add a testcase covering multiple renames across a D/F conflict\n merge-recursive: Fix triple file rename across D/F conflict\n\n merge-recursive.c | 1 +\n t/t6031-merge-recursive.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 0 deletions(-)\n\n-- \n1.7.2.1.227.g086c8\n"},{"id":"148323","messageId":"1282089199-17253-2-git-send-email-newren@gmail.com","threadId":"24777","inReplyTo":"1282089199-17253-1-git-send-email-newren@gmail.com","subject":"[PATCH 1/2] t6031: Add a testcase covering multiple renames across a D/F conflict","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-17T23:53:18Z","receivedAt":"2010-08-17T23:53:18Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"\nSigned-off-by: Elijah Newren \n---\n t/t6031-merge-recursive.sh | 28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6031-merge-recursive.sh b/t/t6031-merge-recursive.sh\nindex 8a3304f..aa235b9 100755\n--- a/t/t6031-merge-recursive.sh\n+++ b/t/t6031-merge-recursive.sh\n@@ -57,4 +57,32 @@ test_expect_success FILEMODE 'verify executable bit on file' '\n \ttest -x file2\n '\n \n+test_expect_failure 'merging with triple rename across D/F conflict' '\n+\tgit reset --hard HEAD &&\n+\tgit checkout --orphan main &&\n+\tgit rm -rf . &&\n+\n+\techo \"just a file\" > sub1 &&\n+\tmkdir -p sub2 &&\n+\techo content > sub2/file1 &&\n+\techo content > sub2/file2 &&\n+\techo content > sub2/file3 &&\n+\tmkdir simple &&\n+\techo base > simple/bar &&\n+\tgit add -A &&\n+\tgit commit -m base &&\n+\n+\tgit checkout -b other &&\n+\techo more >> simple/bar &&\n+\tgit commit -a -m changesimplefile &&\n+\n+\tgit checkout main &&\n+\tgit rm sub1 &&\n+\tgit mv sub2 sub1 &&\n+\tgit commit -m changefiletodir &&\n+\n+\tgit merge other\n+'\n+\n+\n test_done\n-- \n1.7.2.1.227.g086c8\n"},{"id":"148324","messageId":"1282089199-17253-3-git-send-email-newren@gmail.com","threadId":"24777","inReplyTo":"1282089199-17253-1-git-send-email-newren@gmail.com","subject":"[PATCH 2/2] merge-recursive: Fix multiple file rename across D/F conflict","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-17T23:53:19Z","receivedAt":"2010-08-17T23:53:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"In 5a2580d (merge_recursive: Fix renames across paths below D/F conflicts\n2010-07-09), detection was added for renames across paths involved in a\ndirectory<->file conflict. However, the change accidentally involved\nreusing an outer loop index ('i') in an inner loop, changing its values\nand causing a slightly different type of breakage for cases where there are\nmultiple renames across the D/F conflict. Fix by creating a new temporary\nvariable 'i'.\n\nSigned-off-by: Elijah Newren \n---\nI'm really embarrased about this one... :-/\n\n merge-recursive.c | 1 +\n t/t6031-merge-recursive.sh | 2 +-\n 2 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex af53b2a..f413e9f 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1018,6 +1018,7 @@ static int process_renames(struct merge_options *o,\n \t\t\t\tif (mfi.clean &&\n \t\t\t\t sha_eq(mfi.sha, ren1->pair->two->sha1) &&\n \t\t\t\t mfi.mode == ren1->pair->two->mode) {\n+\t\t\t\t\tint i;\n \t\t\t\t\t/*\n \t\t\t\t\t * This messaged is part of\n \t\t\t\t\t * t6022 test. If you change\ndiff --git a/t/t6031-merge-recursive.sh b/t/t6031-merge-recursive.sh\nindex aa235b9..25ae637 100755\n--- a/t/t6031-merge-recursive.sh\n+++ b/t/t6031-merge-recursive.sh\n@@ -57,7 +57,7 @@ test_expect_success FILEMODE 'verify executable bit on file' '\n \ttest -x file2\n '\n \n-test_expect_failure 'merging with triple rename across D/F conflict' '\n+test_expect_success 'merging with triple rename across D/F conflict' '\n \tgit reset --hard HEAD &&\n \tgit checkout --orphan main &&\n \tgit rm -rf . &&\n-- \n1.7.2.1.227.g086c8\n"},{"id":"148404","messageId":"7vaaojagqq.fsf@alter.siamese.dyndns.org","threadId":"24777","inReplyTo":"1282089199-17253-3-git-send-email-newren@gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: Fix multiple file rename across D/F conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-18T19:00:13Z","receivedAt":"2010-08-18T19:00:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> In 5a2580d (merge_recursive: Fix renames across paths below D/F conflicts\n> 2010-07-09), detection was added for renames across paths involved in a\n> directory<->file conflict. However, the change accidentally involved\n> reusing an outer loop index ('i') in an inner loop, changing its values\n> and causing a slightly different type of breakage for cases where there are\n> multiple renames across the D/F conflict. Fix by creating a new temporary\n> variable 'i'.\n>\n> Signed-off-by: Elijah Newren \n> ---\n> I'm really embarrased about this one... :-/\n\nI am too ;-) Thanks for a fix.\n"},{"id":"148423","messageId":"7v8w438q9k.fsf@alter.siamese.dyndns.org","threadId":"24777","inReplyTo":"1282089199-17253-3-git-send-email-newren@gmail.com","subject":"Re: [PATCH 2/2] merge-recursive: Fix multiple file rename across D/F conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-18T23:17:27Z","receivedAt":"2010-08-18T23:17:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> I'm really embarrased about this one... :-/\n\nLet's embarrass you even more ;-)\n\nAfter 5a2580d, the merge has become very noisy, and I don't see a good\nreason for that change.\n\nWith a version of git built from 5a2580d^, for example, merging\nsp/fix-smart-http-deadlock-on-error into maint would give me this:\n\n----------------\n$ git merge sp/fix-smart-http-deadlock-on-error\nAuto-merging remote-curl.c\nMerge made by recursive.\n remote-curl.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n----------------\n\nWith 5a2580d, it has become this:\n\n----------------\n$ git merge sp/fix-smart-http-deadlock-on-error\nAdding builtin/add.c\nAuto-merging remote-curl.c\nMerge made by recursive.\n remote-curl.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n----------------\n\nAnd with this patch on top, it will make it a disaster:\n\n----------------\n$ git merge sp/fix-smart-http-deadlock-on-error\nAdding builtin/add.c\n... similar 97 Adding lines omitted ...\nAdding gitweb/static/gitweb.js\nAuto-merging remote-curl.c\nAdding t/lib-t6000.sh\nAdding t/t7810-grep.sh\nAdding t/t9350-fast-export.sh\nMerge made by recursive.\n remote-curl.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n----------------\n"},{"id":"148552","messageId":"AANLkTin+keov7xDnXZVWzWVkQRqdfPEKBnpkzn27fUC+@mail.gmail.com","threadId":"24777","inReplyTo":"7v8w438q9k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] merge-recursive: Fix multiple file rename across D/F conflict","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-20T12:47:37Z","receivedAt":"2010-08-20T12:47:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Aug 18, 2010 at 5:17 PM, Junio C Hamano wrote:\n> Elijah Newren writes:\n>\n>> I'm really embarrased about this one...  :-/\n>\n> Let's embarrass you even more ;-)\n\nMission accomplished. :-/\n\n> After 5a2580d, the merge has become very noisy, and I don't see a good\n> reason for that change.\n>\n> With a version of git built from 5a2580d^, for example, merging\n> sp/fix-smart-http-deadlock-on-error into maint would give me this:\n>\n> ----------------\n> $ git merge sp/fix-smart-http-deadlock-on-error\n> Auto-merging remote-curl.c\n> Merge made by recursive.\n>  remote-curl.c |    5 +++--\n>  1 files changed, 3 insertions(+), 2 deletions(-)\n> ----------------\n>\n> With 5a2580d, it has become this:\n>\n> ----------------\n> $ git merge sp/fix-smart-http-deadlock-on-error\n> Adding builtin/add.c\n> Auto-merging remote-curl.c\n> Merge made by recursive.\n>  remote-curl.c |    5 +++--\n>  1 files changed, 3 insertions(+), 2 deletions(-)\n> ----------------\n>\n> And with this patch on top, it will make it a disaster:\n>\n> ----------------\n> $ git merge sp/fix-smart-http-deadlock-on-error\n> Adding builtin/add.c\n> ... similar 97 Adding lines omitted ...\n> Adding gitweb/static/gitweb.js\n> Auto-merging remote-curl.c\n> Adding t/lib-t6000.sh\n> Adding t/t7810-grep.sh\n> Adding t/t9350-fast-export.sh\n> Merge made by recursive.\n>  remote-curl.c |    5 +++--\n>  1 files changed, 3 insertions(+), 2 deletions(-)\n> ----------------\n\nIck.\n\nInterestingly, the new paths I added for additional output are not\nbeing triggered here. Rather, \"normal\" renames are being considered\nunprocessed (I had a faulty assumption in the previous patch that a\nclean merge would have cleared out the higher stage entries in\ndst_entry as a side effect). Because of this, normal renames are\nneedlessly re-processed in process_entry(), and trigger extra output\nas a side-effect. One could work around this by the following patch,\nthough it'd be better to just avoid the extra reprocessing. I'll\nsubmit a better patch in a minute.\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 7ac0f57..5ec7f70 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -75,6 +75,7 @@ struct stage_data\n unsigned char sha[20];\n } stages[4];\n unsigned processed:1;\n+ unsigned silent:1;\n };\n\n static int show(struct merge_options *o, int v)\n@@ -1033,9 +1034,11 @@ static int process_renames(struct merge_options *o,\n * conflict) that need to be resolved.\n */\n for (i = 1; i <= 3; i++) {\n if\n(!ren1->dst_entry->stages[i].mode)\n continue;\n ren1->dst_entry->processed = 0;\n+ ren1->dst_entry->silent = 1;\n break;\n }\n } else {\n@@ -1188,7 +1198,8 @@ static int process_entry(struct merge_options *o,\n remove_file(o, 0, path, !a_sha);\n return 1; /* Assume clean till processed */\n } else {\n- output(o, 2, \"Adding %s\", path);\n+ if (!entry->silent)\n+ output(o, 2, \"Adding %s\", path);\n update_file(o, 1, sha, mode, path);\n }\n } else if (a_sha && b_sha) {\n"}]}