{"thread":{"id":"24950","subject":"cherry-picking a commit clobbers a file which is a directory in the target commit","startedAt":"2010-09-02T16:17:43Z","lastAt":"2010-10-21T19:43:15Z","messageCount":14,"participants":["Nick","Elijah Newren","Junio C Hamano","Schalk, Ken","Camille Moncelier"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"149660","messageId":"4C7FCE27.7000500@letterboxes.org","threadId":"24950","inReplyTo":null,"subject":"cherry-picking a commit clobbers a file which is a directory in the target commit","fromName":"Nick","fromEmail":"oinksocket@letterboxes.org","sentAt":"2010-09-02T16:17:43Z","receivedAt":"2010-09-02T16:17:43Z","isPatch":false,"sender":{"key":"oinksocket@letterboxes.org","avatar":null},"body":"This was seen on git version 1.7.0.4 on Ubuntu Lucid.\n\nBasically, cherry-picking a commit from a branch where a file in the current\nbranch has been replaced by a directory, git clobbers the file and the\ncherry-pick fails reporting conflicts.\n\nTo replicate:\n\n $ mkdir clobber\n $ cd clobber/\n $ git init\n $ touch sausage\n $ git add sausage\n $ git commit -m \"added sausage\"\n $ git checkout -b branch1\n $ mv sausage sausage1\n $ mkdir sausage\n $ mv sausage1 sausage/roll\n $ git add sausage/roll\n $ git commit -m \"renamed sausage as sausage/roll\"\n $ touch falafel\n $ git add falafel\n $ git commit -m \"added falafel\"\n $ git checkout master\n\nNow if you try to cherry pick the commit just added to branch1 onto master:\n\n $ git cherry-pick branch1\n Automatic cherry-pick failed. After resolving the conflicts,\n mark the corrected paths with 'git add ' or 'git rm '\n and commit the result with:\n\n git commit -c branch1\n\n $ git status\n # On branch master\n # Changes to be committed:\n # (use \"git reset HEAD ...\" to unstage)\n #\n # renamed: sausage -> falafel\n #\n # Untracked files:\n # (use \"git add ...\" to include in what will be committed)\n #\n # sausage~HEAD\n\nNot what I expected at all. I'd not expect the file 'sausage' to be modified,\njust the new file 'falafel' added, as I did in the original commit.\n\nI know I can recover from this by moving sausage~HEAD back to sausage, or delete\nit and check out sausage again, but I suspect it just shouldn't happen at all.\n\n\nIs this a bug?\n\nN\n"},{"id":"150060","messageId":"4C84B948.9040907@letterboxes.org","threadId":"24950","inReplyTo":"4C7FCE27.7000500@letterboxes.org","subject":"Re: cherry-picking a commit clobbers a file which is a directory in the target commit","fromName":"Nick","fromEmail":"oinksocket@letterboxes.org","sentAt":"2010-09-06T09:50:00Z","receivedAt":"2010-09-06T09:50:00Z","isPatch":false,"sender":{"key":"oinksocket@letterboxes.org","avatar":null},"body":"I've been Warnocked. Can anyone point me in the right direction?\n"},{"id":"150085","messageId":"AANLkTimz8qSwefp137-D+vEbsf6soG51u0im9EC911_O@mail.gmail.com","threadId":"24950","inReplyTo":"4C84B948.9040907@letterboxes.org","subject":"Re: cherry-picking a commit clobbers a file which is a directory in the target commit","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-06T15:20:51Z","receivedAt":"2010-09-06T15:20:51Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Sep 6, 2010 at 3:50 AM, Nick wrote:\n> I've been Warnocked.  Can anyone point me in the right direction?\n\nIt's a bug in the recursive merge strategy (i.e. the default one), and\naffects current master as well as 1.7.0.4. The resolve strategy\n(which can be used with cherry-pick since 1.7.2) handles this\ncorrectly:\n\n$ git cherry-pick --strategy=resolve branch1\nTrying simple merge.\nSimple merge failed, trying Automatic merge.\n[master e95e377] added falafel\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 falafel\n\nI'll investigate.\n\n\nElijah\n"},{"id":"150126","messageId":"1283806070-22027-1-git-send-email-newren@gmail.com","threadId":"24950","inReplyTo":"AANLkTimz8qSwefp137-D+vEbsf6soG51u0im9EC911_O@mail.gmail.com","subject":"[PATCH 0/3] Fix resolvable rename + D/F conflict testcases","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-06T20:47:47Z","receivedAt":"2010-09-06T20:47:47Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This fixes an issue reported by Nick, as well as a closely related\nissue in the handling of rename + directory/file conflicts,\nparticularly where a file on one side of the rename is a directory\nname on the other side of the merge.\n\nElijah Newren (3):\n t3509: Add rename + D/F conflict testcases that recursive strategy\n fails\n merge-recursive: Small code cleanup\n merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir\n\n merge-recursive.c | 50 ++++++++++++++++-------------\n t/t3509-cherry-pick-merge-df.sh | 66 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 94 insertions(+), 22 deletions(-)\n\n-- \n1.7.3.rc0.170.g5cfb0.dirty\n"},{"id":"150127","messageId":"1283806070-22027-2-git-send-email-newren@gmail.com","threadId":"24950","inReplyTo":"AANLkTimz8qSwefp137-D+vEbsf6soG51u0im9EC911_O@mail.gmail.com","subject":"[PATCH 1/3] t3509: Add rename + D/F conflict testcases that recursive strategy fails","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-06T20:47:48Z","receivedAt":"2010-09-06T20:47:48Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"When one side of a file rename matches a directory name on the other side,\nthe recursive merge strategy will fail. This is true even if the merge is\ntrivially resolvable.\n\nSigned-off-by: Elijah Newren \n---\n t/t3509-cherry-pick-merge-df.sh | 66 +++++++++++++++++++++++++++++++++++++++\n 1 files changed, 66 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh\nindex a5ccdbf..eb5826f 100755\n--- a/t/t3509-cherry-pick-merge-df.sh\n+++ b/t/t3509-cherry-pick-merge-df.sh\n@@ -32,4 +32,70 @@ test_expect_success SYMLINKS 'Cherry-pick succeeds with rename across D/F confli\n \tgit cherry-pick branch\n '\n \n+test_expect_success 'Setup rename with file on one side matching directory name on other' '\n+\tgit checkout --orphan nick-testcase &&\n+\tgit rm -rf . &&\n+\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"Empty file\" &&\n+\n+\tgit checkout -b simple &&\n+\tmv empty file &&\n+\tmkdir empty &&\n+\tmv file empty &&\n+\tgit add empty/file &&\n+\tgit commit -m \"Empty file under empty dir\" &&\n+\n+\techo content >newfile &&\n+\tgit add newfile &&\n+\tgit commit -m \"New file\"\n+'\n+\n+test_expect_success 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (resolve)' '\n+\tgit reset --hard &&\n+\tgit checkout -q nick-testcase^0 &&\n+\tgit cherry-pick --strategy=resolve simple\n+'\n+\n+test_expect_failure 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout -q nick-testcase^0 &&\n+\tgit cherry-pick --strategy=recursive simple\n+'\n+\n+test_expect_success 'Setup rename with file on one side matching different dirname on other' '\n+\tgit reset --hard &&\n+\tgit checkout --orphan mergeme &&\n+\tgit rm -rf . &&\n+\n+\tmkdir sub &&\n+\tmkdir othersub &&\n+\techo content > sub/file &&\n+\techo foo > othersub/whatever &&\n+\tgit add -A &&\n+\tgit commit -m \"Common commmit\" &&\n+\n+\tgit rm -rf othersub &&\n+\tgit mv sub/file othersub &&\n+\tgit commit -m \"Commit to merge\" &&\n+\n+\tgit checkout -b newhead mergeme~1 &&\n+\t>independent-change &&\n+\tgit add independent-change &&\n+\tgit commit -m \"Completely unrelated change\"\n+'\n+\n+test_expect_success 'Cherry-pick with rename to different D/F conflict succeeds (resolve)' '\n+\tgit reset --hard &&\n+\tgit checkout -q newhead^0 &&\n+\tgit cherry-pick --strategy=resolve mergeme\n+'\n+\n+test_expect_failure 'Cherry-pick with rename to different D/F conflict succeeds (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout -q newhead^0 &&\n+\tgit cherry-pick --strategy=recursive mergeme\n+'\n+\n test_done\n-- \n1.7.3.rc0.170.g5cfb0.dirty\n"},{"id":"150128","messageId":"1283806070-22027-3-git-send-email-newren@gmail.com","threadId":"24950","inReplyTo":"AANLkTimz8qSwefp137-D+vEbsf6soG51u0im9EC911_O@mail.gmail.com","subject":"[PATCH 2/3] merge-recursive: Small code cleanup","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-06T20:47:49Z","receivedAt":"2010-09-06T20:47:49Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"process_renames() had a variable named \"stage\" and derived variables\nsrc_other and dst_other whose purpose was not entirely clear to me. Make\nthe name of stage slightly more descriptive and add a brief comment\nexplaining what is occurring.\n\nAlso, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are\nidentical 2010-09-01), a separate if-block was added to provide a special\ncase for the rename/add conflict case that can be resolved (namely when\nthe contents on the destination side are identical). However, as a\nseparate if block, it's not immediately obvious that its code is related to\nthe subsequent code checking for a rename/add conflict. We can combine and\nsimplify the check slightly.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 34 ++++++++++++++++++++--------------\n 1 files changed, 20 insertions(+), 14 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c574698..5e2886a 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -923,15 +923,26 @@ static int process_renames(struct merge_options *o,\n \t\t\t/* Renamed in 1, maybe changed in 2 */\n \t\t\tstruct string_list_item *item;\n \t\t\t/* we only use sha1 and mode of these */\n-\t\t\tstruct diff_filespec src_other, dst_other;\n-\t\t\tint try_merge, stage = a_renames == renames1 ? 3: 2;\n+\t\t\tstruct diff_filespec src_other, dst_other, dst_renamed;\n+\t\t\tint try_merge;\n \n-\t\t\tremove_file(o, 1, ren1_src, o->call_depth || stage == 3);\n+\t\t\t/*\n+\t\t\t * unpack_trees loads entries from common-commit\n+\t\t\t * into stage 1, from head-commit into stage 2, and\n+\t\t\t * from merge-commit into stage 3. We keep track\n+\t\t\t * of which side corresponds to the rename.\n+\t\t\t */\n+\t\t\tint renamed_stage = a_renames == renames1 ? 2 : 3;\n+\t\t\tint other_stage = a_renames == renames1 ? 3 : 2;\n+\n+\t\t\tremove_file(o, 1, ren1_src, o->call_depth || renamed_stage == 2);\n \n-\t\t\thashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);\n-\t\t\tsrc_other.mode = ren1->src_entry->stages[stage].mode;\n-\t\t\thashcpy(dst_other.sha1, ren1->dst_entry->stages[stage].sha);\n-\t\t\tdst_other.mode = ren1->dst_entry->stages[stage].mode;\n+\t\t\thashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);\n+\t\t\tsrc_other.mode = ren1->src_entry->stages[other_stage].mode;\n+\t\t\thashcpy(dst_other.sha1, ren1->dst_entry->stages[other_stage].sha);\n+\t\t\tdst_other.mode = ren1->dst_entry->stages[other_stage].mode;\n+\t\t\thashcpy(dst_renamed.sha1, ren1->dst_entry->stages[renamed_stage].sha);\n+\t\t\tdst_renamed.mode = ren1->dst_entry->stages[renamed_stage].mode;\n \n \t\t\ttry_merge = 0;\n \n@@ -955,13 +966,8 @@ static int process_renames(struct merge_options *o,\n \t\t\t\t\t\t\tren1->pair->two : NULL,\n \t\t\t\t\t\t\tbranch1 == o->branch1 ?\n \t\t\t\t\t\t\tNULL : ren1->pair->two, 1);\n-\t\t\t} else if ((dst_other.mode == ren1->pair->two->mode) &&\n-\t\t\t\t sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {\n-\t\t\t\t/* Added file on the other side\n-\t\t\t\t identical to the file being\n-\t\t\t\t renamed: clean merge */\n-\t\t\t\tupdate_file(o, 1, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);\n-\t\t\t} else if (!sha_eq(dst_other.sha1, null_sha1)) {\n+\t\t\t} else if (!sha_eq(dst_other.sha1, null_sha1) &&\n+\t\t\t\t !sha_eq(dst_other.sha1, dst_renamed.sha1)) {\n \t\t\t\tconst char *new_path;\n \t\t\t\tclean_merge = 0;\n \t\t\t\ttry_merge = 1;\n-- \n1.7.3.rc0.170.g5cfb0.dirty\n"},{"id":"150129","messageId":"1283806070-22027-4-git-send-email-newren@gmail.com","threadId":"24950","inReplyTo":"AANLkTimz8qSwefp137-D+vEbsf6soG51u0im9EC911_O@mail.gmail.com","subject":"[PATCH 3/3] merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-06T20:47:50Z","receivedAt":"2010-09-06T20:47:50Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"In merge-recursive.c, whenever there was a rename where a file name on one\nside of the rename matches a directory name on the other side of the merge,\nthen the very first check that\n string_list_has_string(&o->current_directory_set, ren1_dst)\nwould trigger forcing it into marking it as a rename/directory conflict.\n\nHowever, if the path is only renamed on one side and a simple three-way\nmerge between the separate files resolves cleanly, then we don't need to\nmark it as a rename/directory conflict. So, we can simply move the check\nfor rename/directory conflicts after we've verified that there isn't a\nrename/rename conflict and that a threeway content merge doesn't work.\n\nThis changes the particular error message one gets in the case where the\ndirectory name that a file on one side of the rename matches is not also\npart of the rename pair. For example, with commits containing the files:\n COMMON -> (HEAD, MERGE )\n --------- --------------- -------\n sub/file1 -> (sub/file1, newsub)\n -> (newsub/newfile, )\nthen previously when one tried to merge MERGE into HEAD, one would get\n CONFLICT (rename/directory): Rename sub/file1->newsub in HEAD directory newsub added in merge\n Renaming sub/file1 to newsub~HEAD instead\n Adding newsub/newfile\n Automatic merge failed; fix conflicts and then commit the result.\nAfter this patch, the error message will instead become:\n Removing newsub\n Adding newsub/newfile\n CONFLICT (file/directory): There is a directory with name newsub in merge. Adding newsub as newsub~HEAD\n Automatic merge failed; fix conflicts and then commit the result.\nThat makes more sense to me, because git can't know that there's a conflict\nuntil after it's tried resolving paths involving newsub/newfile to see if\nthey are still in the way at the end (and if newsub/newfile is not in the\nway at the end, there should be no conflict at all, which did not hold with\ngit previously).\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 16 ++++++++--------\n t/t3509-cherry-pick-merge-df.sh | 4 ++--\n 2 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 5e2886a..645a79d 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -946,14 +946,7 @@ static int process_renames(struct merge_options *o,\n \n \t\t\ttry_merge = 0;\n \n-\t\t\tif (string_list_has_string(&o->current_directory_set, ren1_dst)) {\n-\t\t\t\tclean_merge = 0;\n-\t\t\t\toutput(o, 1, \"CONFLICT (rename/directory): Rename %s->%s in %s \"\n-\t\t\t\t \" directory %s added in %s\",\n-\t\t\t\t ren1_src, ren1_dst, branch1,\n-\t\t\t\t ren1_dst, branch2);\n-\t\t\t\tconflict_rename_dir(o, ren1, branch1);\n-\t\t\t} else if (sha_eq(src_other.sha1, null_sha1)) {\n+\t\t\tif (sha_eq(src_other.sha1, null_sha1)) {\n \t\t\t\tclean_merge = 0;\n \t\t\t\toutput(o, 1, \"CONFLICT (rename/delete): Rename %s->%s in %s \"\n \t\t\t\t \"and deleted in %s\",\n@@ -1046,6 +1039,13 @@ static int process_renames(struct merge_options *o,\n \t\t\t\t\tif (!ren1->dst_entry->stages[2].mode !=\n \t\t\t\t\t !ren1->dst_entry->stages[3].mode)\n \t\t\t\t\t\tren1->dst_entry->processed = 0;\n+\t\t\t\t} else if (string_list_has_string(&o->current_directory_set, ren1_dst)) {\n+\t\t\t\t\tclean_merge = 0;\n+\t\t\t\t\toutput(o, 1, \"CONFLICT (rename/directory): Rename %s->%s in %s \"\n+\t\t\t\t\t \" directory %s added in %s\",\n+\t\t\t\t\t ren1_src, ren1_dst, branch1,\n+\t\t\t\t\t ren1_dst, branch2);\n+\t\t\t\t\tconflict_rename_dir(o, ren1, branch1);\n \t\t\t\t} else {\n \t\t\t\t\tif (mfi.merge || !mfi.clean)\n \t\t\t\t\t\toutput(o, 1, \"Renaming %s => %s\", ren1_src, ren1_dst);\ndiff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh\nindex eb5826f..948ca1b 100755\n--- a/t/t3509-cherry-pick-merge-df.sh\n+++ b/t/t3509-cherry-pick-merge-df.sh\n@@ -58,7 +58,7 @@ test_expect_success 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (reso\n \tgit cherry-pick --strategy=resolve simple\n '\n \n-test_expect_failure 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (recursive)' '\n+test_expect_success 'Cherry-pick succeeds with was_a_dir/file -> was_a_dir (recursive)' '\n \tgit reset --hard &&\n \tgit checkout -q nick-testcase^0 &&\n \tgit cherry-pick --strategy=recursive simple\n@@ -92,7 +92,7 @@ test_expect_success 'Cherry-pick with rename to different D/F conflict succeeds\n \tgit cherry-pick --strategy=resolve mergeme\n '\n \n-test_expect_failure 'Cherry-pick with rename to different D/F conflict succeeds (recursive)' '\n+test_expect_success 'Cherry-pick with rename to different D/F conflict succeeds (recursive)' '\n \tgit reset --hard &&\n \tgit checkout -q newhead^0 &&\n \tgit cherry-pick --strategy=recursive mergeme\n-- \n1.7.3.rc0.170.g5cfb0.dirty\n"},{"id":"150130","messageId":"AANLkTimLHitZFPRMSdXTSfq6tF=jcwu0Lp9ET0smg3-U@mail.gmail.com","threadId":"24950","inReplyTo":"1283806070-22027-1-git-send-email-newren@gmail.com","subject":"Re: [PATCH 0/3] Fix resolvable rename + D/F conflict testcases","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-06T20:49:45Z","receivedAt":"2010-09-06T20:49:45Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Sep 6, 2010 at 2:47 PM, Elijah Newren wrote:\n> This fixes an issue reported by Nick, as well as a closely related\n> issue in the handling of rename + directory/file conflicts,\n> particularly where a file on one side of the rename is a directory\n> name on the other side of the merge.\n\nI forgot to mention; this patch series is based on next, since it\ntouches some of the code from ks/recursive-rename-add-identical.\n\n> Elijah Newren (3):\n>  t3509: Add rename + D/F conflict testcases that recursive strategy\n>    fails\n>  merge-recursive: Small code cleanup\n>  merge-recursive: D/F conflicts where was_a_dir/file -> was_a_dir\n>\n>  merge-recursive.c               |   50 ++++++++++++++++-------------\n>  t/t3509-cherry-pick-merge-df.sh |   66 +++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 94 insertions(+), 22 deletions(-)\n"},{"id":"150136","messageId":"AANLkTiky3JL6rpo2x79dqQKKndUMa58Se_4CLpSFdj4+@mail.gmail.com","threadId":"24950","inReplyTo":"1283806070-22027-3-git-send-email-newren@gmail.com","subject":"Re: [PATCH 2/3] merge-recursive: Small code cleanup","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-06T21:25:17Z","receivedAt":"2010-09-06T21:25:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Sep 6, 2010 at 2:47 PM, Elijah Newren wrote:\n> process_renames() had a variable named \"stage\" and derived variables\n> src_other and dst_other whose purpose was not entirely clear to me.  Make\n> the name of stage slightly more descriptive and add a brief comment\n> explaining what is occurring.\n>\n> Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are\n> identical 2010-09-01), a separate if-block was added to provide a special\n> case for the rename/add conflict case that can be resolved (namely when\n> the contents on the destination side are identical).  However, as a\n> separate if block, it's not immediately obvious that its code is related to\n> the subsequent code checking for a rename/add conflict.  We can combine and\n> simplify the check slightly.\n>\n> Signed-off-by: Elijah Newren \n> ---\n\nHmmm...should I have split this off from the rest of the series (its\nonly relation is that it cleans up code that made it harder for me to\nfind the real fix)? If I did that, I could rebase the rest of the\nseries on maint...\n"},{"id":"150155","messageId":"7vy6besa9e.fsf@alter.siamese.dyndns.org","threadId":"24950","inReplyTo":"AANLkTiky3JL6rpo2x79dqQKKndUMa58Se_4CLpSFdj4+@mail.gmail.com","subject":"Re: [PATCH 2/3] merge-recursive: Small code cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-06T23:49:33Z","receivedAt":"2010-09-06T23:49:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> Hmmm...should I have split this off from the rest of the series (its\n> only relation is that it cleans up code that made it harder for me to\n> find the real fix)? If I did that, I could rebase the rest of the\n> series on maint...\n\nGood thinking.\n\nThe real polishing of this series will happen after 1.7.3 anyway, so for\nnow a series forking from ks/recursive-rename-add-identical (which I\nexpect to be in 1.7.3) is fine.\n\nThanks.\n"},{"id":"150216","messageId":"EF9FEAB3A4B7D245B0801936B6EF4A254B6BBD@azsmsx503.amr.corp.intel.com","threadId":"24950","inReplyTo":"1283806070-22027-3-git-send-email-newren@gmail.com","subject":"RE: [PATCH 2/3] merge-recursive: Small code cleanup","fromName":"Schalk, Ken","fromEmail":"ken.schalk@intel.com","sentAt":"2010-09-07T16:23:46Z","receivedAt":"2010-09-07T16:23:46Z","isPatch":true,"sender":{"key":"ken.schalk@intel.com","avatar":null},"body":">Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are\n>identical 2010-09-01), a separate if-block was added to provide a special\n>case for the rename/add conflict case that can be resolved (namely when\n>the contents on the destination side are identical). However, as a\n>separate if block, it's not immediately obvious that its code is related to\n>the subsequent code checking for a rename/add conflict. We can combine and\n>simplify the check slightly.\n\nOriginally I tried the fix the way you've re-structured it, just adding a test to the if around the rename/add conflict handling. Unfortunately that didn't completely solve the problem in the case that originally motivated the fix (rename vs. rename+symlink, as in my initial post and my first attempt at adding a test to t/t3030-merge-recursive.sh). That's why I changed it to a separate if block.\n\nThe problem comes down in the code inside the \"if(try_merge)\" block below. It merges the source of the rename on the other side with the renamed file, rather than the destination. In the case with the symlink on the other side, this code merged a symlink with a regular file which resulted in a conflict. I was trying to eliminate both conflicts in this case by avoiding the final else that sets try_merge=1.\n\nYour re-structuring will therefore only solve half the problem I was trying to solve.\n\nI suppose an alternative solution would have been to change the \"if(try_merge)\" code to merge with the destination of the rename on the other side, if it exists and is the same type. However that clearly would have had a much more significant impact on other merge cases, so it didn't seem like a good choice to me.\n\n--Ken\n"},{"id":"150278","messageId":"AANLkTim5AA7mnAhkbqJaFcUv9vniTVG7siOMxE+y=ehf@mail.gmail.com","threadId":"24950","inReplyTo":"EF9FEAB3A4B7D245B0801936B6EF4A254B6BBD@azsmsx503.amr.corp.intel.com","subject":"Re: [PATCH 2/3] merge-recursive: Small code cleanup","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-08T06:24:03Z","receivedAt":"2010-09-08T06:24:03Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Sep 7, 2010 at 10:23 AM, Schalk, Ken wrote:\n>>Also, in d5af510 (RE: [PATCH] Avoid rename/add conflict when contents are\n>>identical 2010-09-01), a separate if-block was added to provide a special\n>>case for the rename/add conflict case that can be resolved (namely when\n>>the contents on the destination side are identical).  However, as a\n>>separate if block, it's not immediately obvious that its code is related to\n>>the subsequent code checking for a rename/add conflict.  We can combine and\n>>simplify the check slightly.\n>\n> Originally I tried the fix the way you've re-structured it, just adding a test to the if around the rename/add conflict handling.  Unfortunately that didn't completely solve the problem in the case that originally motivated the fix (rename vs. rename+symlink, as in my initial post and my first attempt at adding a test to t/t3030-merge-recursive.sh).  That's why I changed it to a separate if block.\n>\n> The problem comes down in the code inside the \"if(try_merge)\" block below.  It merges the source of the rename on the other side with the renamed file, rather than the destination.  In the case with the symlink on the other side, this code merged a symlink with a regular file which resulted in a conflict.  I was trying to eliminate both conflicts in this case by avoiding the final else that sets try_merge=1.\n>\n> Your re-structuring will therefore only solve half the problem I was trying to solve.\n>\n> I suppose an alternative solution would have been to change the \"if(try_merge)\" code to merge with the destination of the rename on the other side, if it exists and is the same type.  However that clearly would have had a much more significant impact on other merge cases, so it didn't seem like a good choice to me.\n\nInteresting...that means we probably should have stuck with the\noriginal testcase you suggested (though marking it with the SYMLINK\ndependence), since the new one doesn't fail with my modifications but\nthe old one would. The typechange is critical. So I'll drop that\nportion of my patch.\n\nPerhaps you could submit another patch changing your testcase back to\nusing a symlink to make sure someone like me doesn't break your\noriginal testcase in the future?\n\n\nThanks,\nElijah\n"},{"id":"150418","messageId":"EF9FEAB3A4B7D245B0801936B6EF4A25593A57@azsmsx503.amr.corp.intel.com","threadId":"24950","inReplyTo":"AANLkTim5AA7mnAhkbqJaFcUv9vniTVG7siOMxE+y=ehf@mail.gmail.com","subject":"RE: [PATCH 2/3] merge-recursive: Small code cleanup","fromName":"Schalk, Ken","fromEmail":"ken.schalk@intel.com","sentAt":"2010-09-09T20:23:26Z","receivedAt":"2010-09-09T20:23:26Z","isPatch":true,"sender":{"key":"ken.schalk@intel.com","avatar":null},"body":">Perhaps you could submit another patch changing your testcase back to\n>using a symlink to make sure someone like me doesn't break your\n>original testcase in the future?\n\nHere's a patch relative to my last one. Rather than restoring the previous test, I added it so that platforms with no symlink support can still test copy vs. rename and platforms with symlink support can also test rename vs. rename/symlink.\n\nSigned-off-by: Ken Schalk \n---\n t/t3030-merge-recursive.sh | 36 +++++++++++++++++++++++++++++++++++-\n 1 files changed, 35 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex b23bd9f..9514ae2 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -25,6 +25,10 @@ test_expect_success 'setup 1' '\n git branch submod &&\n git branch copy &&\n git branch rename &&\n+ if test_have_prereq SYMLINKS\n+ then\n+ git branch rename-ln\n+ fi &&\n\n echo hello >>a &&\n cp a d/e &&\n@@ -256,7 +260,17 @@ test_expect_success 'setup 8' '\n git mv a e &&\n git add e &&\n test_tick &&\n- git commit -m \"rename a->e\"\n+ git commit -m \"rename a->e\" &&\n+ if test_have_prereq SYMLINKS\n+ then\n+ git checkout rename-ln &&\n+ git mv a e &&\n+ ln -s e a &&\n+ git add a e &&\n+ test_tick &&\n+ git commit -m \"rename a->e, symlink a->e\"\n+ fi\n+\n '\n\n test_expect_success 'setup 9' '\n@@ -618,5 +632,25 @@ test_expect_success 'merge-recursive copy vs. rename' '\n test_cmp expected actual\n '\n\n+if test_have_prereq SYMLINKS\n+then\n+ test_expect_success 'merge-recursive rename vs. rename/symlink' '\n+\n+ git checkout -f rename &&\n+ git merge rename-ln &&\n+ ( git ls-tree -r HEAD ; git ls-files -s ) >actual &&\n+ (\n+ echo \"100644 blob $o0 b\"\n+ echo \"100644 blob $o0 c\"\n+ echo \"100644 blob $o0 d/e\"\n+ echo \"100644 blob $o0 e\"\n+ echo \"100644 $o0 0 b\"\n+ echo \"100644 $o0 0 c\"\n+ echo \"100644 $o0 0 d/e\"\n+ echo \"100644 $o0 0 e\"\n+ ) >expected &&\n+ test_cmp expected actual\n+ '\n+fi\n\n test_done\n--\n1.7.0\n"},{"id":"154012","messageId":"20101021214315.17c535e1@cortex","threadId":"24950","inReplyTo":"EF9FEAB3A4B7D245B0801936B6EF4A25593A57@azsmsx503.amr.corp.intel.com","subject":"Re: [PATCH 2/3] merge-recursive: Small code cleanup","fromName":"Camille Moncelier","fromEmail":"moncelier@devlife.org","sentAt":"2010-10-21T19:43:15Z","receivedAt":"2010-10-21T19:43:15Z","isPatch":true,"sender":{"key":"moncelier@devlife.org","avatar":"https://gravatar.com/avatar/4f9e2cf967b39917f6102c3c828bf5adbbf77b2662046a2a8a8c35bbbb58cbf1?d=mp&s=160"},"body":"On Thu, 9 Sep 2010 13:23:26 -0700\n\"Schalk, Ken\" wrote:\n\n> >Perhaps you could submit another patch changing your testcase back to\n> >using a symlink to make sure someone like me doesn't break your\n> >original testcase in the future?\n> \n> Here's a patch relative to my last one. Rather than restoring the\n> previous test, I added it so that platforms with no symlink support\n> can still test copy vs. rename and platforms with symlink support can\n> also test rename vs. rename/symlink.\nHello, I think I have a test case that seems to be related to this\nissue.\n\nmkdir -p repo1\npushd repo1\ngit init .\n\nmkdir dir1\necho file1 > dir1/file1\nln -s dir1 dir2\ngit add dir1 dir2\ngit commit -m \"Initial status: dir2 -> dir1\"\n\ngit checkout -b test1\ngit checkout -b test2\n\ngit co test1\ngit rm dir2\nmkdir dir2\ntouch file2 > dir2/file1\ngit add dir2/file1\ngit commit -m \"Removing link: dir1/ and dir2/\"\n\nmessage=\"New file in test1\"\necho $message > new_file_test1\ngit add new_file_test1\ngit commit -m \"$message\"\n\ngit co test2\nmessage=\"New file in test2\"\necho $message > new_file_test2\ngit add new_file_test2\ngit commit -m \"$message\"\n\n# Tries to get the last commit (which adds new_file_test1)\n# into test2 fails.\ngit cherry-pick test1\n\n# Would work with: git cherry-pick --strategy=resolve test1 \n# (using 1.7.3.1) \n"}]}