{"thread":{"id":"24348","subject":"[PATCHv4 2/6] Add a rename + D/F conflict testcase","startedAt":"2010-07-09T13:10:50Z","lastAt":"2010-07-09T13:10:56Z","messageCount":7,"participants":["Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"145216","messageId":"1278681056-31460-1-git-send-email-newren@gmail.com","threadId":"24348","inReplyTo":null,"subject":"[PATCHv4 0/6] D/F conflict fixes","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-09T13:10:50Z","receivedAt":"2010-07-09T13:10:50Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series fixes a number of spurious directory/file conflicts\nand associated bugs appearing in cherry-pick, rebase, merge, and\nfast-export.  It also has a minor robustness improvement for\nfast-import.  This series includes testsuite fixes for currently known\nfailures in both t6020-merge-df.sh and t6035-merge-dir-to-symlink.sh.\n\nChanges since the previous submission:\n  * Addressed comments from Junio in the previous round.\n\nAlexander Gladysh (1):\n      Add a rename + D/F conflict testcase\n\nElijah Newren (5):\n      Add additional testcases for D/F conflicts\n      merge-recursive: Fix D/F conflicts\n      merge_recursive: Fix renames across paths below D/F conflicts\n      fast-export: Fix output order of D/F changes\n      fast-import: Improve robustness when D->F changes provided in wrong order\n\n builtin/fast-export.c           |   29 ++++++++++\n fast-import.c                   |    8 +++\n merge-recursive.c               |  108 ++++++++++++++++++++++++++++++++-------\n t/t3509-cherry-pick-merge-df.sh |   35 +++++++++++++\n t/t6020-merge-df.sh             |    2 +-\n t/t6035-merge-dir-to-symlink.sh |   64 +++++++++++++++++++++--\n t/t9350-fast-export.sh          |   24 +++++++++\n 7 files changed, 247 insertions(+), 23 deletions(-)\n"},{"id":"145214","messageId":"1278681056-31460-2-git-send-email-newren@gmail.com","threadId":"24348","inReplyTo":"1278681056-31460-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 1/6] Add additional testcases for D/F conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-09T13:10:51Z","receivedAt":"2010-07-09T13:10:51Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Signed-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t6035-merge-dir-to-symlink.sh |   64 ++++++++++++++++++++++++++++++++++++--\n t/t9350-fast-export.sh          |   24 ++++++++++++++\n 2 files changed, 84 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\nindex 3202e1d..761ad9d 100755\n--- a/t/t6035-merge-dir-to-symlink.sh\n+++ b/t/t6035-merge-dir-to-symlink.sh\n@@ -48,7 +48,7 @@ test_expect_success 'setup for merge test' '\n \tgit tag baseline\n '\n \n-test_expect_success 'do not lose a/b-2/c/d in merge (resolve)' '\n+test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolve)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s resolve master &&\n@@ -56,7 +56,7 @@ test_expect_success 'do not lose a/b-2/c/d in merge (resolve)' '\n \ttest -f a/b-2/c/d\n '\n \n-test_expect_failure 'do not lose a/b-2/c/d in merge (recursive)' '\n+test_expect_failure 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s recursive master &&\n@@ -64,6 +64,54 @@ test_expect_failure 'do not lose a/b-2/c/d in merge (recursive)' '\n \ttest -f a/b-2/c/d\n '\n \n+test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (resolve)' '\n+\tgit reset --hard &&\n+\tgit checkout master^0 &&\n+\tgit merge -s resolve baseline^0 &&\n+\ttest -h a/b &&\n+\ttest -f a/b-2/c/d\n+'\n+\n+test_expect_failure 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout master^0 &&\n+\tgit merge -s recursive baseline^0 &&\n+\ttest -h a/b &&\n+\ttest -f a/b-2/c/d\n+'\n+\n+test_expect_failure 'do not lose untracked in merge (resolve)' '\n+\tgit reset --hard &&\n+\tgit checkout baseline^0 &&\n+\t>a/b/c/e &&\n+\ttest_must_fail git merge -s resolve master &&\n+\ttest -f a/b/c/e &&\n+\ttest -f a/b-2/c/d\n+'\n+\n+test_expect_success 'do not lose untracked in merge (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout baseline^0 &&\n+\t>a/b/c/e &&\n+\ttest_must_fail git merge -s recursive master &&\n+\ttest -f a/b/c/e &&\n+\ttest -f a/b-2/c/d\n+'\n+\n+test_expect_success 'do not lose modifications in merge (resolve)' '\n+\tgit reset --hard &&\n+\tgit checkout baseline^0 &&\n+\techo more content >>a/b/c/d &&\n+\ttest_must_fail git merge -s resolve master\n+'\n+\n+test_expect_success 'do not lose modifications in merge (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout baseline^0 &&\n+\techo more content >>a/b/c/d &&\n+\ttest_must_fail git merge -s recursive master\n+'\n+\n test_expect_success 'setup a merge where dir a/b-2 changed to symlink' '\n \tgit reset --hard &&\n \tgit checkout start^0 &&\n@@ -74,7 +122,7 @@ test_expect_success 'setup a merge where dir a/b-2 changed to symlink' '\n \tgit tag test2\n '\n \n-test_expect_success 'merge should not have conflicts (resolve)' '\n+test_expect_success 'merge should not have D/F conflicts (resolve)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s resolve test2 &&\n@@ -82,7 +130,7 @@ test_expect_success 'merge should not have conflicts (resolve)' '\n \ttest -f a/b/c/d\n '\n \n-test_expect_failure 'merge should not have conflicts (recursive)' '\n+test_expect_failure 'merge should not have D/F conflicts (recursive)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s recursive test2 &&\n@@ -90,4 +138,12 @@ test_expect_failure 'merge should not have conflicts (recursive)' '\n \ttest -f a/b/c/d\n '\n \n+test_expect_failure 'merge should not have F/D conflicts (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout -b foo test2 &&\n+\tgit merge -s recursive baseline^0 &&\n+\ttest -h a/b-2 &&\n+\ttest -f a/b/c/d\n+'\n+\n test_done\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex d43f37c..69179c6 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -376,4 +376,28 @@ test_expect_success 'tree_tag-obj'    'git fast-export tree_tag-obj'\n test_expect_success 'tag-obj_tag'     'git fast-export tag-obj_tag'\n test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'\n \n+test_expect_failure 'directory becomes symlink'        '\n+\tgit init dirtosymlink &&\n+\tgit init result &&\n+\t(\n+\t\tcd dirtosymlink &&\n+\t\tmkdir foo &&\n+\t\tmkdir bar &&\n+\t\techo hello > foo/world &&\n+\t\techo hello > bar/world &&\n+\t\tgit add foo/world bar/world &&\n+\t\tgit commit -q -mone &&\n+\t\tgit rm -r foo &&\n+\t\tln -s bar foo &&\n+\t\tgit add foo &&\n+\t\tgit commit -q -mtwo\n+\t) &&\n+\t(\n+\t\tcd dirtosymlink &&\n+\t\tgit fast-export master -- foo |\n+\t\t(cd ../result && git fast-import --quiet)\n+\t) &&\n+\t(cd result && git show master:foo)\n+'\n+\n test_done\n-- \n1.7.1.1.23.gafea6\n"},{"id":"145210","messageId":"1278681056-31460-3-git-send-email-newren@gmail.com","threadId":"24348","inReplyTo":"1278681056-31460-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 2/6] Add a rename + D/F conflict testcase","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-09T13:10:52Z","receivedAt":"2010-07-09T13:10:52Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Alexander Gladysh <agladysh@gmail.com>\n\nThis is a simple testcase where both sides of the rename are paths involved\nin (separate) D/F merge conflicts\n\nSigned-off-by: Alexander Gladysh <agladysh@gmail.com>\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\nJunio was also curious in the last round how well the resolve strategy\nhandled this testcase.  It passes.  I could add it as a testcase, but use\nof the --strategy option for cherry-pick would make the series no longer\napply to maint.  However, if that is wanted, I can add it.\n\n t/t3509-cherry-pick-merge-df.sh |   35 +++++++++++++++++++++++++++++++++++\n 1 files changed, 35 insertions(+), 0 deletions(-)\n create mode 100755 t/t3509-cherry-pick-merge-df.sh\n\ndiff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh\nnew file mode 100755\nindex 0000000..7c05e16\n--- /dev/null\n+++ b/t/t3509-cherry-pick-merge-df.sh\n@@ -0,0 +1,35 @@\n+#!/bin/sh\n+\n+test_description='Test cherry-pick with directory/file conflicts'\n+. ./test-lib.sh\n+\n+test_expect_success 'Setup rename across paths each below D/F conflicts' '\n+\tmkdir a &&\n+\t>a/f &&\n+\tgit add a &&\n+\tgit commit -m a &&\n+\n+\tmkdir b &&\n+\tln -s ../a b/a &&\n+\tgit add b &&\n+\tgit commit -m b &&\n+\n+\tgit checkout -b branch &&\n+\trm b/a &&\n+\tmv a b/a &&\n+\tln -s b/a a &&\n+\tgit add . &&\n+\tgit commit -m swap &&\n+\n+\t>f1 &&\n+\tgit add f1 &&\n+\tgit commit -m f1\n+'\n+\n+test_expect_failure 'Cherry-pick succeeds with rename across D/F conflicts' '\n+\tgit reset --hard &&\n+\tgit checkout master^0 &&\n+\tgit cherry-pick branch\n+'\n+\n+test_done\n-- \n1.7.1.1.23.gafea6\n"},{"id":"145212","messageId":"1278681056-31460-4-git-send-email-newren@gmail.com","threadId":"24348","inReplyTo":"1278681056-31460-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 3/6] merge-recursive: Fix D/F conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-09T13:10:53Z","receivedAt":"2010-07-09T13:10:53Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The D/F conflicts that can be automatically resolved (file or directory\nunmodified on one side of history), have the nice property that\nprocess_entry() can correctly handle all subpaths of the D/F conflict.  In\nthe case of D->F conversions, it will correctly delete all non-conflicting\nfiles below the relevant directory and the directory itself (note that both\nuntracked and conflicting files below the directory will prevent its\nremoval).  So if we handle D/F conflicts after all other conflicts, they\nbecome fairly simple to handle -- we just need to check for whether or not\na path (file/directory) is in the way of creating the new content.  We do\nthis by having process_entry() defer handling such entries to a subsequent\nprocess_df_entry() step.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\nJunio pointed out that if there's a D->F conflict with unmerged or\nuntracked files below the directory, and the user wants to resolve the\nconflict by choosing to keep the directory, then it is quite difficult\nsince all the non-conflicting files under that directory will have been\ndeleted.  I intended to add another patch to this series to address\nthat, however, current git already has the same problem and I wanted to\nget a cleaned up series resubmitted soon.\n\n merge-recursive.c               |   93 ++++++++++++++++++++++++++++++++-------\n t/t6035-merge-dir-to-symlink.sh |    6 +-\n 2 files changed, 80 insertions(+), 19 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 206c103..7d0c27d 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1072,6 +1072,7 @@ static int process_entry(struct merge_options *o,\n \tunsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);\n \tunsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);\n \n+\tentry->processed = 1;\n \tif (o_sha && (!a_sha || !b_sha)) {\n \t\t/* Case A: Deleted in one */\n \t\tif ((!a_sha && !b_sha) ||\n@@ -1104,33 +1105,28 @@ static int process_entry(struct merge_options *o,\n \t} else if ((!o_sha && a_sha && !b_sha) ||\n \t\t   (!o_sha && !a_sha && b_sha)) {\n \t\t/* Case B: Added in one. */\n-\t\tconst char *add_branch;\n-\t\tconst char *other_branch;\n \t\tunsigned mode;\n \t\tconst unsigned char *sha;\n-\t\tconst char *conf;\n \n \t\tif (a_sha) {\n-\t\t\tadd_branch = o->branch1;\n-\t\t\tother_branch = o->branch2;\n \t\t\tmode = a_mode;\n \t\t\tsha = a_sha;\n-\t\t\tconf = \"file/directory\";\n \t\t} else {\n-\t\t\tadd_branch = o->branch2;\n-\t\t\tother_branch = o->branch1;\n \t\t\tmode = b_mode;\n \t\t\tsha = b_sha;\n-\t\t\tconf = \"directory/file\";\n \t\t}\n \t\tif (string_list_has_string(&o->current_directory_set, path)) {\n-\t\t\tconst char *new_path = unique_path(o, path, add_branch);\n-\t\t\tclean_merge = 0;\n-\t\t\toutput(o, 1, \"CONFLICT (%s): There is a directory with name %s in %s. \"\n-\t\t\t       \"Adding %s as %s\",\n-\t\t\t       conf, path, other_branch, path, new_path);\n-\t\t\tremove_file(o, 0, path, 0);\n-\t\t\tupdate_file(o, 0, sha, mode, new_path);\n+\t\t\t/* Handle D->F conflicts after all subfiles */\n+\t\t\tentry->processed = 0;\n+\t\t\t/* But get any file out of the way now, so conflicted\n+\t\t\t * entries below the directory of the same name can\n+\t\t\t * be put in the working directory.\n+\t\t\t */\n+\t\t\tif (a_sha)\n+\t\t\t\toutput(o, 2, \"Removing %s\", path);\n+\t\t\t/* do not touch working file if it did not exist */\n+\t\t\tremove_file(o, 0, path, !a_sha);\n+\t\t\treturn 1; /* Assume clean till processed */\n \t\t} else {\n \t\t\toutput(o, 2, \"Adding %s\", path);\n \t\t\tupdate_file(o, 1, sha, mode, path);\n@@ -1178,6 +1174,64 @@ static int process_entry(struct merge_options *o,\n \treturn clean_merge;\n }\n \n+/*\n+ * Per entry merge function for D/F conflicts, to be called only after\n+ * all files below dir have been processed.  We do this because in the\n+ * cases we can cleanly resolve D/F conflicts, process_entry() can clean\n+ * out all the files below the directory for us.\n+ */\n+static int process_df_entry(struct merge_options *o,\n+\t\t\t const char *path, struct stage_data *entry)\n+{\n+\tint clean_merge = 1;\n+\tunsigned o_mode = entry->stages[1].mode;\n+\tunsigned a_mode = entry->stages[2].mode;\n+\tunsigned b_mode = entry->stages[3].mode;\n+\tunsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);\n+\tunsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);\n+\tunsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);\n+\tconst char *add_branch;\n+\tconst char *other_branch;\n+\tunsigned mode;\n+\tconst unsigned char *sha;\n+\tconst char *conf;\n+\tstruct stat st;\n+\n+\t/* We currently only handle D->F cases */\n+\tassert((!o_sha && a_sha && !b_sha) ||\n+\t       (!o_sha && !a_sha && b_sha));\n+\n+\tentry->processed = 1;\n+\n+\tif (a_sha) {\n+\t\tadd_branch = o->branch1;\n+\t\tother_branch = o->branch2;\n+\t\tmode = a_mode;\n+\t\tsha = a_sha;\n+\t\tconf = \"file/directory\";\n+\t} else {\n+\t\tadd_branch = o->branch2;\n+\t\tother_branch = o->branch1;\n+\t\tmode = b_mode;\n+\t\tsha = b_sha;\n+\t\tconf = \"directory/file\";\n+\t}\n+\tif (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {\n+\t\tconst char *new_path = unique_path(o, path, add_branch);\n+\t\tclean_merge = 0;\n+\t\toutput(o, 1, \"CONFLICT (%s): There is a directory with name %s in %s. \"\n+\t\t       \"Adding %s as %s\",\n+\t\t       conf, path, other_branch, path, new_path);\n+\t\tremove_file(o, 0, path, 0);\n+\t\tupdate_file(o, 0, sha, mode, new_path);\n+\t} else {\n+\t\toutput(o, 2, \"Adding %s\", path);\n+\t\tupdate_file(o, 1, sha, mode, path);\n+\t}\n+\n+\treturn clean_merge;\n+}\n+\n struct unpack_trees_error_msgs get_porcelain_error_msgs(void)\n {\n \tstruct unpack_trees_error_msgs msgs = {\n@@ -1249,6 +1303,13 @@ int merge_trees(struct merge_options *o,\n \t\t\t\t&& !process_entry(o, path, e))\n \t\t\t\tclean = 0;\n \t\t}\n+\t\tfor (i = 0; i < entries->nr; i++) {\n+\t\t\tconst char *path = entries->items[i].string;\n+\t\t\tstruct stage_data *e = entries->items[i].util;\n+\t\t\tif (!e->processed\n+\t\t\t\t&& !process_df_entry(o, path, e))\n+\t\t\t\tclean = 0;\n+\t\t}\n \n \t\tstring_list_clear(re_merge, 0);\n \t\tstring_list_clear(re_head, 0);\ndiff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\nindex 761ad9d..f6972be 100755\n--- a/t/t6035-merge-dir-to-symlink.sh\n+++ b/t/t6035-merge-dir-to-symlink.sh\n@@ -56,7 +56,7 @@ test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolv\n \ttest -f a/b-2/c/d\n '\n \n-test_expect_failure 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '\n+test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s recursive master &&\n@@ -130,7 +130,7 @@ test_expect_success 'merge should not have D/F conflicts (resolve)' '\n \ttest -f a/b/c/d\n '\n \n-test_expect_failure 'merge should not have D/F conflicts (recursive)' '\n+test_expect_success 'merge should not have D/F conflicts (recursive)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s recursive test2 &&\n@@ -138,7 +138,7 @@ test_expect_failure 'merge should not have D/F conflicts (recursive)' '\n \ttest -f a/b/c/d\n '\n \n-test_expect_failure 'merge should not have F/D conflicts (recursive)' '\n+test_expect_success 'merge should not have F/D conflicts (recursive)' '\n \tgit reset --hard &&\n \tgit checkout -b foo test2 &&\n \tgit merge -s recursive baseline^0 &&\n-- \n1.7.1.1.23.gafea6\n"},{"id":"145215","messageId":"1278681056-31460-5-git-send-email-newren@gmail.com","threadId":"24348","inReplyTo":"1278681056-31460-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 4/6] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-09T13:10:54Z","receivedAt":"2010-07-09T13:10:54Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The rename logic in process_renames() handles renames and merging of file\ncontents and then marks files as processed.  However, there may be higher\nstage entries left in the index for other reasons (e.g., due to D/F\nconflicts).  By checking for such cases and marking the entry as not\nprocessed, it allows process_entry() later to look at it and handle those\nhigher stages.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n merge-recursive.c               |   15 +++++++++++++--\n t/t3509-cherry-pick-merge-df.sh |    2 +-\n t/t6020-merge-df.sh             |    2 +-\n 3 files changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 7d0c27d..8f4b7d4 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1019,14 +1019,25 @@ static int process_renames(struct merge_options *o,\n \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    mfi.mode == ren1->pair->two->mode) {\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\n \t\t\t\t\t * it update the test too.\n \t\t\t\t\t */\n \t\t\t\t\toutput(o, 3, \"Skipped %s (merged same as existing)\", ren1_dst);\n-\t\t\t\telse {\n+\n+\t\t\t\t\t/* There may be higher stage entries left\n+\t\t\t\t\t * in the index (e.g. due to a D/F\n+\t\t\t\t\t * conflict) that need to be resolved.\n+\t\t\t\t\t */\n+\t\t\t\t\tfor (i = 1; i <= 3; i++) {\n+\t\t\t\t\t\tif (!ren1->dst_entry->stages[i].mode)\n+\t\t\t\t\t\t\tcontinue;\n+\t\t\t\t\t\tren1->dst_entry->processed = 0;\n+\t\t\t\t\t\tbreak;\n+\t\t\t\t\t}\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);\n \t\t\t\t\tif (mfi.merge)\ndiff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh\nindex 7c05e16..6e7ef84 100755\n--- a/t/t3509-cherry-pick-merge-df.sh\n+++ b/t/t3509-cherry-pick-merge-df.sh\n@@ -26,7 +26,7 @@ test_expect_success 'Setup rename across paths each below D/F conflicts' '\n \tgit commit -m f1\n '\n \n-test_expect_failure 'Cherry-pick succeeds with rename across D/F conflicts' '\n+test_expect_success 'Cherry-pick succeeds with rename across D/F conflicts' '\n \tgit reset --hard &&\n \tgit checkout master^0 &&\n \tgit cherry-pick branch\ndiff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex e71c687..490d397 100755\n--- a/t/t6020-merge-df.sh\n+++ b/t/t6020-merge-df.sh\n@@ -22,7 +22,7 @@ git commit -m \"File: dir\"'\n \n test_expect_code 1 'Merge with d/f conflicts' 'git merge \"merge msg\" B master'\n \n-test_expect_failure 'F/D conflict' '\n+test_expect_success 'F/D conflict' '\n \tgit reset --hard &&\n \tgit checkout master &&\n \trm .git/index &&\n-- \n1.7.1.1.23.gafea6\n"},{"id":"145213","messageId":"1278681056-31460-6-git-send-email-newren@gmail.com","threadId":"24348","inReplyTo":"1278681056-31460-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 5/6] fast-export: Fix output order of D/F changes","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-09T13:10:55Z","receivedAt":"2010-07-09T13:10:55Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The fast-import stream format requires incremental changes which take place\nimmediately, meaning that for D->F conversions all files below the relevant\ndirectory must be deleted before the resulting file of the same name is\ncreated.  Reversing the order can result in fast-import silently deleting\nthe file right after creating it, resulting in the file missing from the\nresulting repository.\n\nWe correct this by first sorting the diff_queue_struct in depth-first\norder.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\nEmail from Junio:\n> If all you want is to force a particular order of paths in the output\n> (e.g. depth first) in this one single application, the cleanest way might\n> be to let the diffcore do its work and at the very end sort the elements\n> in the diff_queued_diff to your liking (c.f. diffcore_fix_diff_index()\n> that uses diffnamecmp() to sort the list).\nThis patch is my attempt to do precisely that.\n\n builtin/fast-export.c  |   29 +++++++++++++++++++++++++++++\n t/t9350-fast-export.sh |    2 +-\n 2 files changed, 30 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex c6dd71a..965e90e 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -147,10 +147,39 @@ static void handle_object(const unsigned char *sha1)\n \tfree(buf);\n }\n \n+static int depth_first(const void *a_, const void *b_)\n+{\n+\tconst struct diff_filepair *a = *((const struct diff_filepair **)a_);\n+\tconst struct diff_filepair *b = *((const struct diff_filepair **)b_);\n+\tconst char *name_a, *name_b;\n+\tint len_a, len_b, len;\n+\tint cmp;\n+\n+\tname_a = a->one ? a->one->path : a->two->path;\n+\tname_b = b->one ? b->one->path : b->two->path;\n+\n+\tlen_a = strlen(name_a);\n+\tlen_b = strlen(name_b);\n+\tlen = (len_a < len_b) ? len_a : len_b;\n+\n+\t/* strcmp will sort 'd' before 'd/e', we want 'd/e' before 'd' */\n+\tcmp = memcmp(name_a, name_b, len);\n+\tif (cmp)\n+\t\treturn cmp;\n+\treturn (len_b - len_a);\n+}\n+\n static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t    struct diff_options *options, void *data)\n {\n \tint i;\n+\n+\t/*\n+\t * Handle files below a directory first, in case they are all deleted\n+\t * and the directory changes to a file or symlink.\n+\t */\n+\tqsort(q->queue, q->nr, sizeof(q->queue[0]), depth_first);\n+\n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filespec *ospec = q->queue[i]->one;\n \t\tstruct diff_filespec *spec = q->queue[i]->two;\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 69179c6..1ee1461 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -376,7 +376,7 @@ test_expect_success 'tree_tag-obj'    'git fast-export tree_tag-obj'\n test_expect_success 'tag-obj_tag'     'git fast-export tag-obj_tag'\n test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'\n \n-test_expect_failure 'directory becomes symlink'        '\n+test_expect_success 'directory becomes symlink'        '\n \tgit init dirtosymlink &&\n \tgit init result &&\n \t(\n-- \n1.7.1.1.23.gafea6\n"},{"id":"145211","messageId":"1278681056-31460-7-git-send-email-newren@gmail.com","threadId":"24348","inReplyTo":"1278681056-31460-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 6/6] fast-import: Improve robustness when D->F changes provided in wrong order","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-09T13:10:56Z","receivedAt":"2010-07-09T13:10:56Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"When older versions of fast-export came across a directory changing to a\nsymlink (or regular file), it would output the changes in the form\n  M 120000 :239821 dir-changing-to-symlink\n  D dir-changing-to-symlink/filename1\nWhen fast-import sees the first line, it deletes the directory named\ndir-changing-to-symlink (and any files below it) and creates a symlink in\nits place.  When fast-import came across the second line, it was previously\ntrying to remove the file and relevant leading directories in\ntree_content_remove(), and as a side effect it would delete the symlink\nthat was just created.  This resulted in the symlink silently missing from\nthe resulting repository.\n\nTo improve robustness, we ignore file deletions underneath directory names\nthat correspond to non-directories.  This can also be viewed as a minor\noptimization: since there cannot be a file and a directory with the same\nname in the same directory, the file clearly can't exist so nothing needs\nto be done to delete it.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n fast-import.c |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 309f2c5..75ed738 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1528,6 +1528,14 @@ static int tree_content_remove(\n \tfor (i = 0; i < t->entry_count; i++) {\n \t\te = t->entries[i];\n \t\tif (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) {\n+\t\t\tif (slash1 && !S_ISDIR(e->versions[1].mode))\n+\t\t\t\t/*\n+\t\t\t\t * If p names a file in some subdirectory, and a\n+\t\t\t\t * file or symlink matching the name of the\n+\t\t\t\t * parent directory of p exists, then p cannot\n+\t\t\t\t * exist and need not be deleted.\n+\t\t\t\t */\n+\t\t\t\treturn 1;\n \t\t\tif (!slash1 || !S_ISDIR(e->versions[1].mode))\n \t\t\t\tgoto del_entry;\n \t\t\tif (!e->tree)\n-- \n1.7.1.1.23.gafea6\n"}]}