{"thread":{"id":"24225","subject":"[PATCH 0/5] D/F conflict fixes","startedAt":"2010-06-29T01:12:11Z","lastAt":"2010-06-30T06:53:19Z","messageCount":13,"participants":["newren@gmail.com","Miklos Vajna","Alexander Gladysh","Elijah Newren","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"144429","messageId":"1277773936-12412-1-git-send-email-newren@gmail.com","threadId":"24225","inReplyTo":null,"subject":"[PATCH 0/5] D/F conflict fixes","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T01:12:11Z","receivedAt":"2010-06-29T01:12:11Z","isPatch":true,"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\nthat I have (or have seen others) run across in cherry-pick, rebase,\nmerge, and fast-import, while fixing already known failures in both\nt6020-merge-df.sh and t6035-merge-dir-to-symlink.sh.\n\nIt also involves an extra testcase posted by Alexander Gladysh to the\nlist on March 8; I hope it's not bad form for me to put his testcase\ninto the testsuite and submit it.\n\nAlexander Gladysh (1):\n Add another rename + D/F conflict testcase\n\nElijah Newren (4):\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-import: Handle directories changing into symlinks\n\n fast-import.c | 5 ++\n merge-recursive.c | 106 ++++++++++++++++++++++++++++++++-------\n t/t6020-merge-df.sh | 34 ++++++++++++-\n t/t6035-merge-dir-to-symlink.sh | 37 +++++++++++++-\n t/t9350-fast-export.sh | 24 +++++++++\n 5 files changed, 185 insertions(+), 21 deletions(-)\n"},{"id":"144430","messageId":"1277773936-12412-2-git-send-email-newren@gmail.com","threadId":"24225","inReplyTo":"1277773936-12412-1-git-send-email-newren@gmail.com","subject":"[PATCH 1/5] Add additional testcases for D/F conflicts","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T01:12:12Z","receivedAt":"2010-06-29T01:12:12Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\n\nSigned-off-by: Elijah Newren \n---\n t/t6035-merge-dir-to-symlink.sh | 37 +++++++++++++++++++++++++++++++++++--\n t/t9350-fast-export.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 59 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\nindex cd3190c..4f04f41 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 '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,31 @@ 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_failure 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout master &&\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_success 'do not lose untracked in merge (recursive)' '\n+\tgit reset --hard &&\n+\tgit checkout baseline^0 &&\n+\ttouch 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 (recursive)' '\n+\tgit add --all &&\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@@ -82,7 +107,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 +115,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.2.rc0.212.g0c601\n"},{"id":"144434","messageId":"1277773936-12412-3-git-send-email-newren@gmail.com","threadId":"24225","inReplyTo":"1277773936-12412-1-git-send-email-newren@gmail.com","subject":"[PATCH 2/5] Add another rename + D/F conflict testcase","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T01:12:13Z","receivedAt":"2010-06-29T01:12:13Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Alexander Gladysh \n\nThis is a simple testcase where both sides of the rename are paths involved\nin (separate) D/F merge conflicts.\n---\nI hope it's not bad style to take someone else's testcase from the mailing\nlist and submit it on their behalf as a testsuite addition (nor do I know\nwhat to do about the signed-off-by line in this case). This is simply the\ntestcase Alexander Gladysh posted to the list on March 8. I really like\nhis example due to how it serves as a simple case where there are two D/F\nconflicts with a rename across paths involved in both of those D/F\nconflicts.\n\nI'm trying to submit this with Alexander listed as the author, but I'm not\nsure how to preserve that when using git-send-email.\n\n t/t6020-merge-df.sh | 32 ++++++++++++++++++++++++++++++++\n 1 files changed, 32 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex e71c687..99acb89 100755\n--- a/t/t6020-merge-df.sh\n+++ b/t/t6020-merge-df.sh\n@@ -45,4 +45,36 @@ test_expect_failure 'F/D conflict' '\n \tgit merge master\n '\n \n+test_expect_success 'Setup rename across paths each below D/F conflicts' '\n+\tgit symbolic-ref HEAD refs/heads/newmaster &&\n+\trm .git/index &&\n+\tgit clean -fdx &&\n+\n+\tmkdir a &&\n+\ttouch 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/ &&\n+\tln -s b/a a &&\n+\tgit add . &&\n+\tgit commit -m \"swap\" &&\n+\n+\ttouch f1 &&\n+\tgit add f1 &&\n+\tgit commit -m \"f1\"\n+'\n+\n+test_expect_failure 'Test rename across paths below D/F conflicts' '\n+\tgit checkout newmaster &&\n+\tgit cherry-pick branch\n+'\n+\n test_done\n-- \n1.7.2.rc0.212.g0c601\n"},{"id":"144432","messageId":"1277773936-12412-4-git-send-email-newren@gmail.com","threadId":"24225","inReplyTo":"1277773936-12412-1-git-send-email-newren@gmail.com","subject":"[PATCH 3/5] merge-recursive: Fix D/F conflicts","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T01:12:14Z","receivedAt":"2010-06-29T01:12:14Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe D/F conflicts we know how to resolve (file or directory unmodified on\none side of history), have the nice property that process_entry() can\ncorrectly handle all subpaths of the D/F conflict, and will in fact delete\nall non-conflicting files below the relevant directory and the directory\nitself in such cases. So if we handle D/F conflicts after all other\nconflicts, they become fairly simple to handle. We do this by adding an\nextra process_df_entry() step after process_renames() and process_entry().\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 93 ++++++++++++++++++++++++++++++++-------\n t/t6035-merge-dir-to-symlink.sh | 8 ++--\n 2 files changed, 81 insertions(+), 20 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 856e98c..8d70fc0 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+/* 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+\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+\tconst char *add_branch;\n+\tconst char *other_branch;\n+\tunsigned mode;\n+\tconst unsigned char *sha;\n+\tconst char *conf;\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+\tstruct stat st;\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 4f04f41..64387a0 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 '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 '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@@ -64,7 +64,7 @@ test_expect_failure 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recurs\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+test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' '\n \tgit reset --hard &&\n \tgit checkout master &&\n \tgit merge -s recursive baseline^0 &&\n@@ -107,7 +107,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 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@@ -115,7 +115,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.2.rc0.212.g0c601\n"},{"id":"144433","messageId":"1277773936-12412-5-git-send-email-newren@gmail.com","threadId":"24225","inReplyTo":"1277773936-12412-1-git-send-email-newren@gmail.com","subject":"[PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T01:12:15Z","receivedAt":"2010-06-29T01:12:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\n\nSigned-off-by: Elijah Newren \n---\nI'm a little uneasy with this change, mainly because I don't fully\nunderstand the rename processing logic (I was actually kind of surprised\nwhen I made these changes and it worked). Although I verified that\nthese changes (and my others in this patch series) introduce no new\nbreakages in the testsuite and even fix a known issue, I'm still not\nquite sure I follow the logic well enough to feel fully confident in\nthis change. I'm particularly worried I may have neglected some closely\nrelated cases that I should have fixed but which may still be broken.\n\n merge-recursive.c | 13 +++++++++++--\n t/t6020-merge-df.sh | 4 ++--\n 2 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 8d70fc0..ab0743f 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1019,14 +1019,23 @@ 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/* If this was a rename across a path involved\n+\t\t\t\t\t * in a D/F conflict, there may be more work to\n+\t\t\t\t\t * do.\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\tren1->dst_entry->processed = 0;\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/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex 99acb89..7278eee 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@@ -72,7 +72,7 @@ test_expect_success 'Setup rename across paths each below D/F conflicts' '\n \tgit commit -m \"f1\"\n '\n \n-test_expect_failure 'Test rename across paths below D/F conflicts' '\n+test_expect_success 'Test rename across paths below D/F conflicts' '\n \tgit checkout newmaster &&\n \tgit cherry-pick branch\n '\n-- \n1.7.2.rc0.212.g0c601\n"},{"id":"144431","messageId":"1277773936-12412-6-git-send-email-newren@gmail.com","threadId":"24225","inReplyTo":"1277773936-12412-1-git-send-email-newren@gmail.com","subject":"[PATCH 5/5] fast-import: Handle directories changing into symlinks","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T01:12:16Z","receivedAt":"2010-06-29T01:12:16Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\n\nSigned-off-by: Elijah Newren \n---\nThis is a resend of an earlier patch. Since the previous one wasn't\nreviewed and didn't make it to pu, I decided to resend it along with the\nmerge-recursive directory/symlink conflict fixes as part of a patch series.\n\n fast-import.c | 5 +++++\n t/t9350-fast-export.sh | 2 +-\n 2 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 1e5d66e..9a2ecc8 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1528,6 +1528,11 @@ 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_ISLNK(e->versions[1].mode))\n+\t\t\t\t/* p was already removed by an earlier change\n+\t\t\t\t * of a parent directory to a symlink.\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)\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.2.rc0.212.g0c601\n"},{"id":"144446","messageId":"20100629075442.GB31048@genesis.frugalware.org","threadId":"24225","inReplyTo":"1277773936-12412-5-git-send-email-newren@gmail.com","subject":"Re: [PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2010-06-29T07:54:42Z","receivedAt":"2010-06-29T07:54:42Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Mon, Jun 28, 2010 at 07:12:15PM -0600, newren@gmail.com wrote:\n> From: Elijah Newren \n> \n> \n> Signed-off-by: Elijah Newren \n> ---\n> I'm a little uneasy with this change, mainly because I don't fully\n> understand the rename processing logic (I was actually kind of surprised\n> when I made these changes and it worked). Although I verified that\n> these changes (and my others in this patch series) introduce no new\n> breakages in the testsuite and even fix a known issue, I'm still not\n> quite sure I follow the logic well enough to feel fully confident in\n> this change. I'm particularly worried I may have neglected some closely\n> related cases that I should have fixed but which may still be broken.\n\nSame here, I touched merge-recursive, but not this part of it, so others\nwill give you a better review, I'm sure. :)\n\nOther than that, I like it, thanks!\n"},{"id":"144452","messageId":"AANLkTik6Qt40dZA8Gv26j0SyhkvZt5D2LIm_QBEtmAsU@mail.gmail.com","threadId":"24225","inReplyTo":"1277773936-12412-3-git-send-email-newren@gmail.com","subject":"Re: [PATCH 2/5] Add another rename + D/F conflict testcase","fromName":"Alexander Gladysh","fromEmail":"agladysh@gmail.com","sentAt":"2010-06-29T08:49:58Z","receivedAt":"2010-06-29T08:49:58Z","isPatch":true,"sender":{"key":"agladysh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38239?v=4"},"body":"> I hope it's not bad style to take someone else's testcase from the mailing\n> list and submit it on their behalf as a testsuite addition (nor do I know\n> what to do about the signed-off-by line in this case).  This is simply the\n> testcase Alexander Gladysh posted to the list on March 8.  I really like\n> his example due to how it serves as a simple case where there are two D/F\n> conflicts with a rename across paths involved in both of those D/F\n> conflicts.\n\nI have no problem if you use my test case for the Git testsuite. The\nmore tests — the merrier. :-)\n\nConsider this testcase as signed-off by me.\n\nIf you feel that you need to mention my name somewhere and proper Git\nheaders are a problem — commit comments would do.\n\nIf, to honor all formalities, you would need my actual commit — please\ntell me, I'll try to submit it.\n\nAlexander.\n"},{"id":"144458","messageId":"AANLkTimFBlWiK76quLW1TiUfueGISsW7ZIHgFUcFg4j8@mail.gmail.com","threadId":"24225","inReplyTo":"20100629075442.GB31048@genesis.frugalware.org","subject":"Re: [PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T12:52:07Z","receivedAt":"2010-06-29T12:52:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Jun 29, 2010 at 1:54 AM, Miklos Vajna wrote:\n> On Mon, Jun 28, 2010 at 07:12:15PM -0600, newren@gmail.com wrote:\n>> I'm a little uneasy with this change, mainly because I don't fully\n>> understand the rename processing logic (I was actually kind of surprised\n>> when I made these changes and it worked).  Although I verified that\n>> these changes (and my others in this patch series) introduce no new\n>> breakages in the testsuite and even fix a known issue, I'm still not\n>> quite sure I follow the logic well enough to feel fully confident in\n>> this change.  I'm particularly worried I may have neglected some closely\n>> related cases that I should have fixed but which may still be broken.\n>\n> Same here, I touched merge-recursive, but not this part of it, so others\n> will give you a better review, I'm sure. :)\n>\n> Other than that, I like it, thanks!\n\nOh, it looks like I was off by a couple lines when trying to read the\nauthorship out of git blame -C -C. You touched lines that were pretty\nclose, but it looks like this if block was actually due to Alex. So\nI'll add him to the cc.\n\nAlex: I think the basic idea is just that the rename logic isn't aware\nthat there may be higher stage entries in the index 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\nthose higher stages. But I'm not sure if that's the right way to\nhandle it, or if just having process_renames() should take care of\nclearing out the higher stage entries, or if something else entirely\nshould be done.\n\nThanks,\nElijah\n"},{"id":"144461","messageId":"AANLkTil7CdCoP3wLVKX0MEiwp8KaKWFLvRtUWzt2a3Nh@mail.gmail.com","threadId":"24225","inReplyTo":"AANLkTimFBlWiK76quLW1TiUfueGISsW7ZIHgFUcFg4j8@mail.gmail.com","subject":"Re: [PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-06-29T13:36:58Z","receivedAt":"2010-06-29T13:36:58Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Jun 29, 2010 at 14:52, Elijah Newren wrote:\n> On Tue, Jun 29, 2010 at 1:54 AM, Miklos Vajna wrote:\n>> On Mon, Jun 28, 2010 at 07:12:15PM -0600, newren@gmail.com wrote:\n>>> I'm a little uneasy with this change, mainly because I don't fully\n>>> understand the rename processing logic (I was actually kind of surprised\n>>> when I made these changes and it worked).  Although I verified that\n>>> these changes (and my others in this patch series) introduce no new\n>>> breakages in the testsuite and even fix a known issue, I'm still not\n>>> quite sure I follow the logic well enough to feel fully confident in\n>>> this change.  I'm particularly worried I may have neglected some closely\n>>> related cases that I should have fixed but which may still be broken.\n>\n> Alex: I think the basic idea is just that the rename logic isn't aware\n> that there may be higher stage entries in the index due to D/F\n> conflicts; by checking for such cases and marking the entry as not\n> processed, it allows process_entry() later to look at it and handle\n> those higher stages.  But I'm not sure if that's the right way to\n> handle it, or if just having process_renames() should take care of\n> clearing out the higher stage entries, or if something else entirely\n> should be done.\n\nNor am I. You may be still off by some commits in detecting the authorship :)\nThis code was seldom touched since it was written (by Johannes). It has\nsurvived in this sorry state all (at least my) attempts to fix it. OTOH I never\ntried really hard. Maybe because the D/F conflicts are rare and are relatively\nsimple to work around.\n\nI cannot say much about your change... Are you sure about D/F conflict\ndetection, though? You just test if target mode not 0.\n"},{"id":"144467","messageId":"AANLkTilggM9-vBabNvJiYMiQZyZtJMLhfWleYKvuJNMv@mail.gmail.com","threadId":"24225","inReplyTo":"AANLkTil7CdCoP3wLVKX0MEiwp8KaKWFLvRtUWzt2a3Nh@mail.gmail.com","subject":"Re: [PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-06-29T15:55:38Z","receivedAt":"2010-06-29T15:55:38Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Jun 29, 2010 at 7:36 AM, Alex Riesen wrote:\n> On Tue, Jun 29, 2010 at 14:52, Elijah Newren wrote:\n>> Alex: I think the basic idea is just that the rename logic isn't aware\n>> that there may be higher stage entries in the index due to D/F\n>> conflicts; by checking for such cases and marking the entry as not\n>> processed, it allows process_entry() later to look at it and handle\n>> those higher stages.  But I'm not sure if that's the right way to\n>> handle it, or if just having process_renames() should take care of\n>> clearing out the higher stage entries, or if something else entirely\n>> should be done.\n>\n> Nor am I. You may be still off by some commits in detecting the authorship :)\n> This code was seldom touched since it was written (by Johannes). It has\n> survived in this sorry state all (at least my) attempts to fix it. OTOH I never\n> tried really hard. Maybe because the D/F conflicts are rare and are relatively\n> simple to work around.\n>\n> I cannot say much about your change... Are you sure about D/F conflict\n> detection, though? You just test if target mode not 0.\n\nWell, as far as this particular if-block is concerned, blame suggests\nthat you and Miklos were responsible (I apologize if gmail screws up\nand inserts line wrapping)::\n\n$ git blame -C -C -L 1020,1038 merge-recursive.c\n9047ebbc (Miklos Vajna 2008-08-12 18:45:14 +0200 1020)\n if (mfi.clean &&\n9047ebbc (Miklos Vajna 2008-08-12 18:45:14 +0200 1021)\n sha_eq(mfi.sha, ren1->pair->two->sha1) &&\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1022)\n mfi.mode == ren1->pair->two->mode) {\n8a359819 (Alex Riesen 2007-04-25 22:07:45 +0200 1023)\n /*\n8a359819 (Alex Riesen 2007-04-25 22:07:45 +0200 1024)\n * This messaged is part of\n8a359819 (Alex Riesen 2007-04-25 22:07:45 +0200 1025)\n * t6022 test. If you change\n8a359819 (Alex Riesen 2007-04-25 22:07:45 +0200 1026)\n * it update the test too.\n8a359819 (Alex Riesen 2007-04-25 22:07:45 +0200 1027)\n */\n8a2fce18 (Miklos Vajna 2008-08-25 16:25:57 +0200 1028)\n output(o, 3, \"Skipped %s (merged same as\nexisting)\", ren1_\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1029)\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1030)\n /* If this was a rename across a path\ninvolved\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1031)\n * in a D/F conflict, there may be more work\nto\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1032)\n * do.\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1033)\n */\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1034)\n for (i=1; i<=3; ++i) {\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1035)\n if (ren1->dst_entry->stages[i].mode)\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1036)\n ren1->dst_entry->processed =\n0;\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1037)\n }\nde4d7dc3 (Elijah Newren 2010-06-28 09:38:58 -0600 1038)\n } else {\n\nWith D/F conflicts, the files would be loaded into higher stages in\nthe index (before it gets to process_renames()), which I detected via\na non-zero mode. If there's a different way I should be checking for\nhigher stage entries that still need to be resolved, I'd be happy to\nuse it.\n"},{"id":"144485","messageId":"20100629223319.GC31048@genesis.frugalware.org","threadId":"24225","inReplyTo":"AANLkTilggM9-vBabNvJiYMiQZyZtJMLhfWleYKvuJNMv@mail.gmail.com","subject":"Re: [PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2010-06-29T22:33:19Z","receivedAt":"2010-06-29T22:33:19Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Tue, Jun 29, 2010 at 09:55:38AM -0600, Elijah Newren wrote:\n> Well, as far as this particular if-block is concerned, blame suggests\n> that you and Miklos were responsible (I apologize if gmail screws up\n> and inserts line wrapping)::\n> \n> $ git blame -C -C -L 1020,1038 merge-recursive.c\n> 9047ebbc (Miklos Vajna 2008-08-12 18:45:14 +0200 1020)\n> if (mfi.clean &&\n> 9047ebbc (Miklos Vajna 2008-08-12 18:45:14 +0200 1021)\n> sha_eq(mfi.sha, ren1->pair->two->sha1) &&\n\nAnd if you have a look at what commit 9047ebbc does, that's really just\nabout changing it to be part of libgit, so I could call it without\nfork() from builtin-merge.\n\nTo sum up, I sadly have to say I don't know too much about the\nmerge-recursive internal sematics.\n"},{"id":"144504","messageId":"AANLkTilJIh9V3kIhBnfm5Bunzbp7XdoYOOoVbku_u-8y@mail.gmail.com","threadId":"24225","inReplyTo":"AANLkTilggM9-vBabNvJiYMiQZyZtJMLhfWleYKvuJNMv@mail.gmail.com","subject":"Re: [PATCH 4/5] merge_recursive: Fix renames across paths below D/F conflicts","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-06-30T06:53:19Z","receivedAt":"2010-06-30T06:53:19Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Jun 29, 2010 at 17:55, Elijah Newren wrote:\n> On Tue, Jun 29, 2010 at 7:36 AM, Alex Riesen wrote:\n>> I cannot say much about your change... Are you sure about D/F conflict\n>> detection, though? You just test if target mode not 0.\n>\n> Well, as far as this particular if-block is concerned, blame suggests\n> that you and Miklos were responsible (I apologize if gmail screws up\n> and inserts line wrapping)::\n\nDon't just look at the blame output, look at what the commits actually changed.\nIt's either a reformatting or a trivial change.\n\n> With D/F conflicts, the files would be loaded into higher stages in\n> the index (before it gets to process_renames()), which I detected via\n> a non-zero mode.\n\nThis just detects if there was any conflict. Not specifically D/F or F/D.\n\n> If there's a different way I should be checking for higher stage entries\n> that still need to be resolved, I'd be happy to use it.\n\nI'd expect a check for a file-to-directory (or back) mode change.\n"}]}