{"thread":{"id":"48858","subject":"[PATCH 0/2] Address recovery failures with directory/file conflicts","startedAt":"2018-07-11T05:18:42Z","lastAt":"2018-07-31T17:12:18Z","messageCount":14,"participants":["Elijah Newren","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"352198","messageId":"20180711051834.28181-1-newren@gmail.com","threadId":"48858","inReplyTo":null,"subject":"[PATCH 0/2] Address recovery failures with directory/file conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-11T05:18:32Z","receivedAt":"2018-07-11T05:18:42Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Several \"recovery\" commands outright fail or do not fully recover\nwhen directory-file conflicts are present.  This includes:\n   * git read-tree --reset HEAD\n   * git am --skip\n   * git am --abort\n   * git merge --abort (or git reset --merge)\n   * git reset --hard\n\nIt turns out that multiple invocations of 'git reset --hard' can serve as\na functional workaround, but I've been annoyed with that workaround for\nyears.  This time, I found the problem, and this patch series provides a\nfix for all of these.\n\n\nElijah Newren (2):\n  t1015: demonstrate directory/file conflict recovery failures\n  read-cache: fix directory/file conflict handling in read_index_unmerged()\n\n read-cache.c                         |  13 +--\n t/t1015-read-index-unmerged.sh       | 123 +++++++++++++++++++++++++++\n t/t6020-merge-df.sh                  |   3 -\n t/t6042-merge-rename-corner-cases.sh |   1 -\n 4 files changed, 131 insertions(+), 9 deletions(-)\n create mode 100755 t/t1015-read-index-unmerged.sh\n\n-- \n2.18.0.138.g557c5d94c.dirty\n\n"},{"id":"352199","messageId":"20180711051834.28181-2-newren@gmail.com","threadId":"48858","inReplyTo":"20180711051834.28181-1-newren@gmail.com","subject":"[PATCH 1/2] t1015: demonstrate directory/file conflict recovery failures","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-11T05:18:33Z","receivedAt":"2018-07-11T05:18:43Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Several \"recovery\" commands outright fail or do not fully recover\nwhen directory-file conflicts are present.  This includes:\n  * git read-tree --reset HEAD\n  * git am --skip\n  * git am --abort\n  * git merge --abort\n  * git reset --hard\n\nAdd testcases documenting these shortcomings.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n t/t1015-read-index-unmerged.sh | 123 +++++++++++++++++++++++++++++++++\n 1 file changed, 123 insertions(+)\n create mode 100755 t/t1015-read-index-unmerged.sh\n\ndiff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\nnew file mode 100755\nindex 000000000..bbd64587c\n--- /dev/null\n+++ b/t/t1015-read-index-unmerged.sh\n@@ -0,0 +1,123 @@\n+#!/bin/sh\n+\n+test_description='Test various callers of read_index_unmerged'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup modify/delete + directory/file conflict' '\n+\ttest_create_repo df_plus_modify_delete &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\tprintf \"a\\nb\\nc\\nd\\ne\\nf\\ng\\nh\\n\" >letters &&\n+\t\tgit add letters &&\n+\t\tgit commit -m initial &&\n+\n+\t\tgit checkout -b modify &&\n+\t\t# Throw in letters.txt for sorting order fun\n+\t\t# (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n+\t\techo i >>letters &&\n+\t\techo \"version 2\" >letters.txt &&\n+\t\tgit add letters letters.txt &&\n+\t\tgit commit -m modified &&\n+\n+\t\tgit checkout -b delete HEAD^ &&\n+\t\tgit rm letters &&\n+\t\tmkdir letters &&\n+\t\t>letters/file &&\n+\t\techo \"version 1\" >letters.txt &&\n+\t\tgit add letters letters.txt &&\n+\t\tgit commit -m deleted\n+\t)\n+'\n+\n+test_expect_failure 'read-tree --reset cleans unmerged entries' '\n+\ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\tgit checkout delete^0 &&\n+\t\ttest_must_fail git merge modify &&\n+\n+\t\tgit read-tree --reset HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_failure 'One reset --hard cleans unmerged entries' '\n+\ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\tgit checkout delete^0 &&\n+\t\ttest_must_fail git merge modify &&\n+\n+\t\tgit reset --hard &&\n+\t\ttest_path_is_missing .git/MERGE_HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_success 'setup directory/file conflict + simple edit/edit' '\n+\ttest_create_repo df_plus_edit_edit &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\ttest_seq 1 10 >numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m initial &&\n+\n+\t\tgit checkout -b d-edit &&\n+\t\tmkdir foo &&\n+\t\techo content >foo/bar &&\n+\t\tgit add foo &&\n+\t\techo 11 >>numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m \"directory and edit\" &&\n+\n+\t\tgit checkout -b f-edit d-edit^1 &&\n+\t\techo content >foo &&\n+\t\tgit add foo &&\n+\t\techo eleven >>numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m \"file and edit\"\n+\t)\n+'\n+\n+test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n+\ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\tgit checkout f-edit^0 &&\n+\t\ttest_must_fail git merge d-edit^0 &&\n+\n+\t\tgit merge --abort &&\n+\t\ttest_path_is_missing .git/MERGE_HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n+\ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\tgit checkout f-edit^0 &&\n+\t\tgit format-patch -1 d-edit &&\n+\t\ttest_must_fail git am -3 0001*.patch &&\n+\n+\t\tgit am --skip &&\n+\t\ttest_path_is_missing .git/rebase-apply &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_done\n-- \n2.18.0.138.g557c5d94c.dirty\n\n"},{"id":"352200","messageId":"20180711051834.28181-3-newren@gmail.com","threadId":"48858","inReplyTo":"20180711051834.28181-1-newren@gmail.com","subject":"[PATCH 2/2] read-cache: fix directory/file conflict handling in read_index_unmerged()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-11T05:18:34Z","receivedAt":"2018-07-11T05:18:45Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"read_index_unmerged() has two intended purposes:\n  * return 1 if there are any unmerged entries, 0 otherwise\n  * drops any higher-stage entries down to stage #0\n\nThere are several callers of read_index_unmerged() that check the return\nvalue to see if it is non-zero, all of which then die() if that condition\nis met.  For these callers, dropping higher-stage entries down to stage #0\nis a waste of resources, and returning immediately on first unmerged entry\nwould be better.  But it's probably only a very minor difference and isn't\nthe focus of this series.\n\nThe remaining callers ignore the return value and call this function for\nthe side effect of dropping higher-stage entries down to stage #0.  As\nmentioned in commit e11d7b596970 (\"'reset --merge': fix unmerged case\",\n2009-12-31),\n\n    The _only_ reason we want to keep a previously unmerged entry in the\n    index at stage #0 is so that we don't forget the fact that we have\n    corresponding file in the work tree in order to be able to remove it\n    when the tree we are resetting to does not have the path.\n\nIn fact, prior to commit d1a43f2aa4bf (\"reset --hard/read-tree --reset -u:\nremove unmerged new paths\", 2008-10-15), read_index_unmerged() did just\nremove unmerged entries from the cache immediately but that had the\nunwanted effect of leaving around new untracked files in the tree from\naborted merges.\n\nSo, that's the intended purpose of this function.  The problem is that\nwhen directory/files conflicts are present, trying to add the file to the\nindex at stage 0 fails (because there is still a directory in the way),\nand the function returns early with a -1 return code to signify the error.\nAs noted above, none of the callers who want the drop-to-stage-0 behavior\ncheck the return status, though, so this means all remaining unmerged\nentries remain in the index and the callers proceed assuming otherwise.\nUsers then see errors of the form:\n\n    error: 'DIR-OR-FILE' appears as both a file and as a directory\n    error: DIR-OR-FILE: cannot drop to stage #0\n\nand potentially also messages about other unmerged entries which came\nlexicographically later than whatever pathname was both a file and a\ndirectory.  Google finds a few hits searching for those messages,\nsuggesting there were probably a couple people who hit this besides me.\nLuckily, calling `git reset --hard` multiple times would workaround\nthis bug.\n\nSince the whole purpose here is to just put the entry *temporarily* into\nthe index so that any associated file in the working copy can be removed,\nwe can just skip the DFCHECK and allow both the file and directory to\nappear in the index.  The temporary simultaneous appearance of the\ndirectory and file entries in the index will be removed by the callers\nbefore they attempt to write the index anywhere.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n read-cache.c                         | 13 ++++++++-----\n t/t1015-read-index-unmerged.sh       |  8 ++++----\n t/t6020-merge-df.sh                  |  3 ---\n t/t6042-merge-rename-corner-cases.sh |  1 -\n 4 files changed, 12 insertions(+), 13 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 372588260..666d295a5 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2632,10 +2632,13 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \n /*\n  * Read the index file that is potentially unmerged into given\n- * index_state, dropping any unmerged entries.  Returns true if\n- * the index is unmerged.  Callers who want to refuse to work\n- * from an unmerged state can call this and check its return value,\n- * instead of calling read_cache().\n+ * index_state, dropping any unmerged entries to stage #0 (potentially\n+ * resulting in a path appearing as both a file and a directory in the\n+ * index; the caller is responsible to clear out the extra entries\n+ * before writing the index to a tree).  Returns true if the index is\n+ * unmerged.  Callers who want to refuse to work from an unmerged\n+ * state can call this and check its return value, instead of calling\n+ * read_cache().\n  */\n int read_index_unmerged(struct index_state *istate)\n {\n@@ -2658,7 +2661,7 @@ int read_index_unmerged(struct index_state *istate)\n \t\tnew_ce->ce_flags = create_ce_flags(0) | CE_CONFLICTED;\n \t\tnew_ce->ce_namelen = len;\n \t\tnew_ce->ce_mode = ce->ce_mode;\n-\t\tif (add_index_entry(istate, new_ce, 0))\n+\t\tif (add_index_entry(istate, new_ce, ADD_CACHE_SKIP_DFCHECK))\n \t\t\treturn error(\"%s: cannot drop to stage #0\",\n \t\t\t\t     new_ce->name);\n \t}\ndiff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\nindex bbd64587c..5034ed931 100755\n--- a/t/t1015-read-index-unmerged.sh\n+++ b/t/t1015-read-index-unmerged.sh\n@@ -30,7 +30,7 @@ test_expect_success 'setup modify/delete + directory/file conflict' '\n \t)\n '\n \n-test_expect_failure 'read-tree --reset cleans unmerged entries' '\n+test_expect_success 'read-tree --reset cleans unmerged entries' '\n \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n \t(\n@@ -45,7 +45,7 @@ test_expect_failure 'read-tree --reset cleans unmerged entries' '\n \t)\n '\n \n-test_expect_failure 'One reset --hard cleans unmerged entries' '\n+test_expect_success 'One reset --hard cleans unmerged entries' '\n \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n \t(\n@@ -87,7 +87,7 @@ test_expect_success 'setup directory/file conflict + simple edit/edit' '\n \t)\n '\n \n-test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n+test_expect_success 'git merge --abort succeeds despite D/F conflict' '\n \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n \t(\n@@ -103,7 +103,7 @@ test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n \t)\n '\n \n-test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n+test_expect_success 'git am --skip succeeds despite D/F conflict' '\n \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n \t(\ndiff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex 2af1beec5..46b506b3b 100755\n--- a/t/t6020-merge-df.sh\n+++ b/t/t6020-merge-df.sh\n@@ -89,9 +89,6 @@ test_expect_success 'modify/delete + directory/file conflict' '\n '\n \n test_expect_success 'modify/delete + directory/file conflict; other way' '\n-\t# Yes, we really need the double reset since \"letters\" appears as\n-\t# both a file and a directory.\n-\tgit reset --hard &&\n \tgit reset --hard &&\n \tgit clean -f &&\n \tgit checkout modify^0 &&\ndiff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\nindex 1cbd946fc..583e68997 100755\n--- a/t/t6042-merge-rename-corner-cases.sh\n+++ b/t/t6042-merge-rename-corner-cases.sh\n@@ -323,7 +323,6 @@ test_expect_success 'rename/directory conflict + content merge conflict' '\n \t(\n \t\tcd rename-directory-1 &&\n \n-\t\tgit reset --hard &&\n \t\tgit reset --hard &&\n \t\tgit clean -fdqx &&\n \n-- \n2.18.0.138.g557c5d94c.dirty\n\n"},{"id":"352222","messageId":"20180711092126.GA17315@flurp.local","threadId":"48858","inReplyTo":"20180711051834.28181-2-newren@gmail.com","subject":"Re: [PATCH 1/2] t1015: demonstrate directory/file conflict recovery failures","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-11T09:21:26Z","receivedAt":"2018-07-11T09:21:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 10, 2018 at 10:18:33PM -0700, Elijah Newren wrote:\n> Several \"recovery\" commands outright fail or do not fully recover\n> when directory-file conflicts are present.  This includes:\n>   * git read-tree --reset HEAD\n>   * git am --skip\n>   * git am --abort\n>   * git merge --abort\n>   * git reset --hard\n> \n> Add testcases documenting these shortcomings.\n> \n> Signed-off-by: Elijah Newren <newren@gmail.com>\n> ---\n> diff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\n> @@ -0,0 +1,123 @@\n> +test_expect_success 'setup modify/delete + directory/file conflict' '\n> +\ttest_create_repo df_plus_modify_delete &&\n> +\t(\n> +\t\tcd df_plus_modify_delete &&\n> +\n> +\t\tprintf \"a\\nb\\nc\\nd\\ne\\nf\\ng\\nh\\n\" >letters &&\n\ntest_write_lines a b c d e f g h >letters &&\n\n> +\t\tgit add letters &&\n> +\t\tgit commit -m initial &&\n> +\n> +\t\tgit checkout -b modify &&\n> +\t\t# Throw in letters.txt for sorting order fun\n> +\t\t# (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n> +\t\techo i >>letters &&\n> +\t\techo \"version 2\" >letters.txt &&\n> +\t\tgit add letters letters.txt &&\n> +\t\tgit commit -m modified &&\n> +\n> +\t\tgit checkout -b delete HEAD^ &&\n> +\t\tgit rm letters &&\n> +\t\tmkdir letters &&\n> +\t\t>letters/file &&\n> +\t\techo \"version 1\" >letters.txt &&\n> +\t\tgit add letters letters.txt &&\n> +\t\tgit commit -m deleted\n> +\t)\n> +'\n> +\n> +test_expect_failure 'read-tree --reset cleans unmerged entries' '\n> +\ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n> +\ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n> +\t(\n> +\t\tcd df_plus_modify_delete &&\n> +\t\t...\n> +\t)\n> +'\n\nI wonder how much value these distinct repositories add over not using\nthem:\n\n--- >8 ---\n#!/bin/sh\n\ntest_description='Test various callers of read_index_unmerged'\n. ./test-lib.sh\n\ntest_expect_success 'setup modify/delete + directory/file conflict' '\n\ttest_write_lines a b c d e f g h >letters &&\n\tgit add letters &&\n\tgit commit -m initial &&\n\n\tgit checkout -b modify &&\n\t# Throw in letters.txt for sorting order fun\n\t# (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n\techo i >>letters &&\n\techo \"version 2\" >letters.txt &&\n\tgit add letters letters.txt &&\n\tgit commit -m modified &&\n\n\tgit checkout -b delete HEAD^ &&\n\tgit rm letters &&\n\tmkdir letters &&\n\t>letters/file &&\n\techo \"version 1\" >letters.txt &&\n\tgit add letters letters.txt &&\n\tgit commit -m deleted\n'\n\ntest_expect_failure 'read-tree --reset cleans unmerged entries' '\n\ttest_when_finished \"git clean -f\" &&\n\ttest_when_finished \"git reset --hard\" &&\n\n\tgit checkout delete^0 &&\n\ttest_must_fail git merge modify &&\n\n\tgit read-tree --reset HEAD &&\n\tgit ls-files -u >conflicts &&\n\ttest_must_be_empty conflicts\n'\n\ntest_expect_failure 'One reset --hard cleans unmerged entries' '\n\ttest_when_finished \"git clean -f\" &&\n\ttest_when_finished \"git reset --hard\" &&\n\n\tgit checkout delete^0 &&\n\ttest_must_fail git merge modify &&\n\n\tgit reset --hard &&\n\ttest_path_is_missing .git/MERGE_HEAD &&\n\tgit ls-files -u >conflicts &&\n\ttest_must_be_empty conflicts\n'\n\ntest_expect_success 'setup directory/file conflict + simple edit/edit' '\n\ttest_seq 1 10 >numbers &&\n\tgit add numbers &&\n\tgit commit -m initial &&\n\n\tgit checkout -b d-edit &&\n\tmkdir foo &&\n\techo content >foo/bar &&\n\tgit add foo &&\n\techo 11 >>numbers &&\n\tgit add numbers &&\n\tgit commit -m \"directory and edit\" &&\n\n\tgit checkout -b f-edit d-edit^1 &&\n\techo content >foo &&\n\tgit add foo &&\n\techo eleven >>numbers &&\n\tgit add numbers &&\n\tgit commit -m \"file and edit\"\n'\n\ntest_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n\ttest_when_finished \"git clean -f\" &&\n\ttest_when_finished \"git reset --hard\" &&\n\n\tgit checkout f-edit^0 &&\n\ttest_must_fail git merge d-edit^0 &&\n\n\tgit merge --abort &&\n\ttest_path_is_missing .git/MERGE_HEAD &&\n\tgit ls-files -u >conflicts &&\n\ttest_must_be_empty conflicts\n'\n\ntest_expect_failure 'git am --skip succeeds despite D/F conflict' '\n\ttest_when_finished \"git clean -f\" &&\n\ttest_when_finished \"git reset --hard\" &&\n\n\tgit checkout f-edit^0 &&\n\tgit format-patch -1 d-edit &&\n\ttest_must_fail git am -3 0001*.patch &&\n\n\tgit am --skip &&\n\ttest_path_is_missing .git/rebase-apply &&\n\tgit ls-files -u >conflicts &&\n\ttest_must_be_empty conflicts\n'\n\ntest_done\n--- >8 ---\n"},{"id":"352272","messageId":"xmqqsh4p3dtk.fsf@gitster-ct.c.googlers.com","threadId":"48858","inReplyTo":"20180711051834.28181-3-newren@gmail.com","subject":"Re: [PATCH 2/2] read-cache: fix directory/file conflict handling in read_index_unmerged()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-11T17:03:03Z","receivedAt":"2018-07-11T17:03:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n>     The _only_ reason we want to keep a previously unmerged entry in the\n>     index at stage #0 is so that we don't forget the fact that we have\n>     corresponding file in the work tree in order to be able to remove it\n>     when the tree we are resetting to does not have the path.\n> ...\n> So, that's the intended purpose of this function.  The problem is that\n> when directory/files conflicts are present, trying to add the file to the\n> index at stage 0 fails (because there is still a directory in the way),\n> and the function returns early with a -1 return code to signify the error.\n> As noted above, none of the callers who want the drop-to-stage-0 behavior\n> check the return status, though, so this means all remaining unmerged\n> entries remain in the index and the callers proceed assuming otherwise.\n\nNicely analysed and explained so far.\n\n> ...  The temporary simultaneous appearance of the\n> directory and file entries in the index will be removed by the callers\n> before they attempt to write the index anywhere.\n\nBut this part makes me feel a bit uneasy, as I find this \"will be\nremoved\" a bit hand-wavy.  There are two such callers.  \"am --skip\"\nand \"reset\".  \n\nThe former uses am.c::fast_forward_to that calls unpack_trees() to\ntwo-way merge (aka \"switch to the other branch\") and these entries\nwith CE_CONFLICTED flag get removed in merged/deleted_entry().\n\n\"reset\" (all variants) call unpack_trees() on the index prepared\nwith read_cache_unmerged(), and the unmerged entries that are marked\nwith CE_CONFLICTED bit get removed the same way.\n\nSo perhaps before \"before they attempt to\", saying \"by calling\nunpack_trees(), which excludes these unmerged entries marked with\nCE_CONFLICTED flag from the resulting index,\" or something like that\nwould help uneasy readers?\n\n> Signed-off-by: Elijah Newren <newren@gmail.com>\n> ---\n>  read-cache.c                         | 13 ++++++++-----\n>  t/t1015-read-index-unmerged.sh       |  8 ++++----\n>  t/t6020-merge-df.sh                  |  3 ---\n>  t/t6042-merge-rename-corner-cases.sh |  1 -\n>  4 files changed, 12 insertions(+), 13 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index 372588260..666d295a5 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -2632,10 +2632,13 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n>  \n>  /*\n>   * Read the index file that is potentially unmerged into given\n> - * index_state, dropping any unmerged entries.  Returns true if\n> - * the index is unmerged.  Callers who want to refuse to work\n> - * from an unmerged state can call this and check its return value,\n> - * instead of calling read_cache().\n> + * index_state, dropping any unmerged entries to stage #0 (potentially\n> + * resulting in a path appearing as both a file and a directory in the\n> + * index; the caller is responsible to clear out the extra entries\n> + * before writing the index to a tree).  Returns true if the index is\n> + * unmerged.  Callers who want to refuse to work from an unmerged\n> + * state can call this and check its return value, instead of calling\n> + * read_cache().\n>   */\n>  int read_index_unmerged(struct index_state *istate)\n>  {\n> @@ -2658,7 +2661,7 @@ int read_index_unmerged(struct index_state *istate)\n>  \t\tnew_ce->ce_flags = create_ce_flags(0) | CE_CONFLICTED;\n>  \t\tnew_ce->ce_namelen = len;\n>  \t\tnew_ce->ce_mode = ce->ce_mode;\n> -\t\tif (add_index_entry(istate, new_ce, 0))\n> +\t\tif (add_index_entry(istate, new_ce, ADD_CACHE_SKIP_DFCHECK))\n>  \t\t\treturn error(\"%s: cannot drop to stage #0\",\n>  \t\t\t\t     new_ce->name);\n>  \t}\n> diff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\n> index bbd64587c..5034ed931 100755\n> --- a/t/t1015-read-index-unmerged.sh\n> +++ b/t/t1015-read-index-unmerged.sh\n> @@ -30,7 +30,7 @@ test_expect_success 'setup modify/delete + directory/file conflict' '\n>  \t)\n>  '\n>  \n> -test_expect_failure 'read-tree --reset cleans unmerged entries' '\n> +test_expect_success 'read-tree --reset cleans unmerged entries' '\n>  \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n>  \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n>  \t(\n> @@ -45,7 +45,7 @@ test_expect_failure 'read-tree --reset cleans unmerged entries' '\n>  \t)\n>  '\n>  \n> -test_expect_failure 'One reset --hard cleans unmerged entries' '\n> +test_expect_success 'One reset --hard cleans unmerged entries' '\n>  \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n>  \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n>  \t(\n> @@ -87,7 +87,7 @@ test_expect_success 'setup directory/file conflict + simple edit/edit' '\n>  \t)\n>  '\n>  \n> -test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n> +test_expect_success 'git merge --abort succeeds despite D/F conflict' '\n>  \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n>  \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n>  \t(\n> @@ -103,7 +103,7 @@ test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n>  \t)\n>  '\n>  \n> -test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n> +test_expect_success 'git am --skip succeeds despite D/F conflict' '\n>  \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n>  \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n>  \t(\n> diff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\n> index 2af1beec5..46b506b3b 100755\n> --- a/t/t6020-merge-df.sh\n> +++ b/t/t6020-merge-df.sh\n> @@ -89,9 +89,6 @@ test_expect_success 'modify/delete + directory/file conflict' '\n>  '\n>  \n>  test_expect_success 'modify/delete + directory/file conflict; other way' '\n> -\t# Yes, we really need the double reset since \"letters\" appears as\n> -\t# both a file and a directory.\n> -\tgit reset --hard &&\n>  \tgit reset --hard &&\n>  \tgit clean -f &&\n>  \tgit checkout modify^0 &&\n> diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\n> index 1cbd946fc..583e68997 100755\n> --- a/t/t6042-merge-rename-corner-cases.sh\n> +++ b/t/t6042-merge-rename-corner-cases.sh\n> @@ -323,7 +323,6 @@ test_expect_success 'rename/directory conflict + content merge conflict' '\n>  \t(\n>  \t\tcd rename-directory-1 &&\n>  \n> -\t\tgit reset --hard &&\n>  \t\tgit reset --hard &&\n>  \t\tgit clean -fdqx &&\n"},{"id":"352284","messageId":"CABPp-BHHkL0Uns9QoeG8LHJgZmfRJu=qAesdpTWJcNmmqhJF9A@mail.gmail.com","threadId":"48858","inReplyTo":"20180711092126.GA17315@flurp.local","subject":"Re: [PATCH 1/2] t1015: demonstrate directory/file conflict recovery failures","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-11T18:23:17Z","receivedAt":"2018-07-11T18:23:20Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Eric,\n\nOn Wed, Jul 11, 2018 at 2:21 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Tue, Jul 10, 2018 at 10:18:33PM -0700, Elijah Newren wrote:\n>> Several \"recovery\" commands outright fail or do not fully recover\n>> when directory-file conflicts are present.  This includes:\n>>   * git read-tree --reset HEAD\n>>   * git am --skip\n>>   * git am --abort\n>>   * git merge --abort\n>>   * git reset --hard\n>>\n>> Add testcases documenting these shortcomings.\n>>\n>> Signed-off-by: Elijah Newren <newren@gmail.com>\n>> ---\n>> diff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\n>> @@ -0,0 +1,123 @@\n>> +test_expect_success 'setup modify/delete + directory/file conflict' '\n>> +     test_create_repo df_plus_modify_delete &&\n>> +     (\n>> +             cd df_plus_modify_delete &&\n>> +\n>> +             printf \"a\\nb\\nc\\nd\\ne\\nf\\ng\\nh\\n\" >letters &&\n>\n> test_write_lines a b c d e f g h >letters &&\n\nCopying and modifying an existing testcase, while forgetting to check\nfor anachronisms, strikes again.  As always, thanks for reviewing and\ncatching this; I'll fix it up.\n\n>> +             git add letters &&\n>> +             git commit -m initial &&\n>> +\n>> +             git checkout -b modify &&\n>> +             # Throw in letters.txt for sorting order fun\n>> +             # (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n>> +             echo i >>letters &&\n>> +             echo \"version 2\" >letters.txt &&\n>> +             git add letters letters.txt &&\n>> +             git commit -m modified &&\n>> +\n>> +             git checkout -b delete HEAD^ &&\n>> +             git rm letters &&\n>> +             mkdir letters &&\n>> +             >letters/file &&\n>> +             echo \"version 1\" >letters.txt &&\n>> +             git add letters letters.txt &&\n>> +             git commit -m deleted\n>> +     )\n>> +'\n>> +\n>> +test_expect_failure 'read-tree --reset cleans unmerged entries' '\n>> +     test_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n>> +     test_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n>> +     (\n>> +             cd df_plus_modify_delete &&\n>> +             ...\n>> +     )\n>> +'\n>\n> I wonder how much value these distinct repositories add over not using\n> them:\n\nIn my opinion, that'd be much worse.  Personally, I think we should\nmove in the opposite direction and try to migrate more of the\ntestsuite elsewhere towards clearly independent tests.  A huge pet\npeeve of mine is that trying to debug a test often requires working\nthrough dozens and dozens of unrelated tests and their setup just to\nunderstand which part of the repository state is related to the test\nat hand and which parts can be ignored.  It's happened enough times\nthat I just intentionally try to make it clear which tests of mine are\nindependent by making sure they have their own separate repo (and I\nused to do a git reset --hard && git clean -fdqx && rm -rf .git && git\ninit at the beginning of tests).  If folks have a better suggestion\nfor how to ensure test independence than using test_create_repo, I'm\nall ears, but I'm strongly against just adding more files into the\nrepo than what previous tests did and continuing running from there.\nI feel that's especially important for future readers when dealing\nwith weird edge and corner cases for merges, but I'd really like to\nsee that clean separation spread throughout the test suite.\n\n> --- >8 ---\n> #!/bin/sh\n>\n> test_description='Test various callers of read_index_unmerged'\n> . ./test-lib.sh\n>\n> test_expect_success 'setup modify/delete + directory/file conflict' '\n>         test_write_lines a b c d e f g h >letters &&\n>         git add letters &&\n>         git commit -m initial &&\n>\n>         git checkout -b modify &&\n>         # Throw in letters.txt for sorting order fun\n>         # (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n>         echo i >>letters &&\n>         echo \"version 2\" >letters.txt &&\n>         git add letters letters.txt &&\n>         git commit -m modified &&\n>\n>         git checkout -b delete HEAD^ &&\n>         git rm letters &&\n>         mkdir letters &&\n>         >letters/file &&\n>         echo \"version 1\" >letters.txt &&\n>         git add letters letters.txt &&\n>         git commit -m deleted\n> '\n>\n> test_expect_failure 'read-tree --reset cleans unmerged entries' '\n>         test_when_finished \"git clean -f\" &&\n>         test_when_finished \"git reset --hard\" &&\n>\n>         git checkout delete^0 &&\n>         test_must_fail git merge modify &&\n>\n>         git read-tree --reset HEAD &&\n>         git ls-files -u >conflicts &&\n>         test_must_be_empty conflicts\n> '\n>\n> test_expect_failure 'One reset --hard cleans unmerged entries' '\n>         test_when_finished \"git clean -f\" &&\n>         test_when_finished \"git reset --hard\" &&\n>\n>         git checkout delete^0 &&\n>         test_must_fail git merge modify &&\n>\n>         git reset --hard &&\n>         test_path_is_missing .git/MERGE_HEAD &&\n>         git ls-files -u >conflicts &&\n>         test_must_be_empty conflicts\n> '\n>\n> test_expect_success 'setup directory/file conflict + simple edit/edit' '\n>         test_seq 1 10 >numbers &&\n>         git add numbers &&\n>         git commit -m initial &&\n>\n>         git checkout -b d-edit &&\n>         mkdir foo &&\n>         echo content >foo/bar &&\n>         git add foo &&\n>         echo 11 >>numbers &&\n>         git add numbers &&\n>         git commit -m \"directory and edit\" &&\n>\n>         git checkout -b f-edit d-edit^1 &&\n>         echo content >foo &&\n>         git add foo &&\n>         echo eleven >>numbers &&\n>         git add numbers &&\n>         git commit -m \"file and edit\"\n> '\n>\n> test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n>         test_when_finished \"git clean -f\" &&\n>         test_when_finished \"git reset --hard\" &&\n>\n>         git checkout f-edit^0 &&\n>         test_must_fail git merge d-edit^0 &&\n>\n>         git merge --abort &&\n>         test_path_is_missing .git/MERGE_HEAD &&\n>         git ls-files -u >conflicts &&\n>         test_must_be_empty conflicts\n> '\n>\n> test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n>         test_when_finished \"git clean -f\" &&\n>         test_when_finished \"git reset --hard\" &&\n>\n>         git checkout f-edit^0 &&\n>         git format-patch -1 d-edit &&\n>         test_must_fail git am -3 0001*.patch &&\n>\n>         git am --skip &&\n>         test_path_is_missing .git/rebase-apply &&\n>         git ls-files -u >conflicts &&\n>         test_must_be_empty conflicts\n> '\n>\n> test_done\n> --- >8 ---\n"},{"id":"352285","messageId":"CABPp-BEzsj6T2eaz-OEi_+CMJetHVsMTBgy6DBfVY26e3M1NoA@mail.gmail.com","threadId":"48858","inReplyTo":"xmqqsh4p3dtk.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] read-cache: fix directory/file conflict handling in read_index_unmerged()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-11T18:24:55Z","receivedAt":"2018-07-11T18:24:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jul 11, 2018 at 10:03 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Elijah Newren <newren@gmail.com> writes:\n>\n>>     The _only_ reason we want to keep a previously unmerged entry in the\n>>     index at stage #0 is so that we don't forget the fact that we have\n>>     corresponding file in the work tree in order to be able to remove it\n>>     when the tree we are resetting to does not have the path.\n>> ...\n>> So, that's the intended purpose of this function.  The problem is that\n>> when directory/files conflicts are present, trying to add the file to the\n>> index at stage 0 fails (because there is still a directory in the way),\n>> and the function returns early with a -1 return code to signify the error.\n>> As noted above, none of the callers who want the drop-to-stage-0 behavior\n>> check the return status, though, so this means all remaining unmerged\n>> entries remain in the index and the callers proceed assuming otherwise.\n>\n> Nicely analysed and explained so far.\n>\n>> ...  The temporary simultaneous appearance of the\n>> directory and file entries in the index will be removed by the callers\n>> before they attempt to write the index anywhere.\n>\n> But this part makes me feel a bit uneasy, as I find this \"will be\n> removed\" a bit hand-wavy.  There are two such callers.  \"am --skip\"\n> and \"reset\".\n>\n> The former uses am.c::fast_forward_to that calls unpack_trees() to\n> two-way merge (aka \"switch to the other branch\") and these entries\n> with CE_CONFLICTED flag get removed in merged/deleted_entry().\n>\n> \"reset\" (all variants) call unpack_trees() on the index prepared\n> with read_cache_unmerged(), and the unmerged entries that are marked\n> with CE_CONFLICTED bit get removed the same way.\n>\n> So perhaps before \"before they attempt to\", saying \"by calling\n> unpack_trees(), which excludes these unmerged entries marked with\n> CE_CONFLICTED flag from the resulting index,\" or something like that\n> would help uneasy readers?\n\nMakes sense, I'll include that in my re-roll after waiting a little\nbit for any further reviews.\n"},{"id":"352473","messageId":"20180713163331.22446-1-newren@gmail.com","threadId":"48858","inReplyTo":"20180711051834.28181-1-newren@gmail.com","subject":"[PATCH v2 0/2] Address recovery failures with directory/file conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-13T16:33:29Z","receivedAt":"2018-07-13T16:33:40Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series fixes several \"recovery\" commands that outright fail\nor do not fully recover when directory-file conflicts are present.\nThis includes:\n   * git read-tree --reset HEAD\n   * git am --skip\n   * git am --abort\n   * git merge --abort (or git reset --merge)\n   * git reset --hard\n\nChanges since v1 (full range-diff below):\n  - Make use of test_write_lines, as suggested by Eric.\n  - Provide a little more explanation in one of the commit messages, as\n    suggested by Junio.\n\nElijah Newren (2):\n  t1015: demonstrate directory/file conflict recovery failures\n  read-cache: fix directory/file conflict handling in\n    read_index_unmerged()\n\n read-cache.c                         |  13 +--\n t/t1015-read-index-unmerged.sh       | 123 +++++++++++++++++++++++++++\n t/t6020-merge-df.sh                  |   3 -\n t/t6042-merge-rename-corner-cases.sh |   1 -\n 4 files changed, 131 insertions(+), 9 deletions(-)\n create mode 100755 t/t1015-read-index-unmerged.sh\n\n1:  a85e462914 ! 1:  8f53327a8d t1015: demonstrate directory/file conflict recovery failures\n    @@ -29,7 +29,7 @@\n     +\t(\n     +\t\tcd df_plus_modify_delete &&\n     +\n    -+\t\tprintf \"a\\nb\\nc\\nd\\ne\\nf\\ng\\nh\\n\" >letters &&\n    ++\t\ttest_write_lines a b c d e f g h >letters &&\n     +\t\tgit add letters &&\n     +\t\tgit commit -m initial &&\n     +\n2:  43d5b0a5ae ! 2:  990a469d44 read-cache: fix directory/file conflict handling in read_index_unmerged()\n    @@ -53,7 +53,9 @@\n         we can just skip the DFCHECK and allow both the file and directory to\n         appear in the index.  The temporary simultaneous appearance of the\n         directory and file entries in the index will be removed by the callers\n    -    before they attempt to write the index anywhere.\n    +    by calling unpack_trees(), which excludes these unmerged entries marked\n    +    with CE_CONFLICTED flag from the resulting index, before they attempt to\n    +    write the index anywhere.\n     \n         Signed-off-by: Elijah Newren <newren@gmail.com>\n     \n-- \n2.18.0.645.g72fe132ec2\n"},{"id":"352474","messageId":"20180713163331.22446-3-newren@gmail.com","threadId":"48858","inReplyTo":"20180713163331.22446-1-newren@gmail.com","subject":"[PATCH v2 2/2] read-cache: fix directory/file conflict handling in read_index_unmerged()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-13T16:33:31Z","receivedAt":"2018-07-13T16:33:41Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"read_index_unmerged() has two intended purposes:\n  * return 1 if there are any unmerged entries, 0 otherwise\n  * drops any higher-stage entries down to stage #0\n\nThere are several callers of read_index_unmerged() that check the return\nvalue to see if it is non-zero, all of which then die() if that condition\nis met.  For these callers, dropping higher-stage entries down to stage #0\nis a waste of resources, and returning immediately on first unmerged entry\nwould be better.  But it's probably only a very minor difference and isn't\nthe focus of this series.\n\nThe remaining callers ignore the return value and call this function for\nthe side effect of dropping higher-stage entries down to stage #0.  As\nmentioned in commit e11d7b596970 (\"'reset --merge': fix unmerged case\",\n2009-12-31),\n\n    The _only_ reason we want to keep a previously unmerged entry in the\n    index at stage #0 is so that we don't forget the fact that we have\n    corresponding file in the work tree in order to be able to remove it\n    when the tree we are resetting to does not have the path.\n\nIn fact, prior to commit d1a43f2aa4bf (\"reset --hard/read-tree --reset -u:\nremove unmerged new paths\", 2008-10-15), read_index_unmerged() did just\nremove unmerged entries from the cache immediately but that had the\nunwanted effect of leaving around new untracked files in the tree from\naborted merges.\n\nSo, that's the intended purpose of this function.  The problem is that\nwhen directory/files conflicts are present, trying to add the file to the\nindex at stage 0 fails (because there is still a directory in the way),\nand the function returns early with a -1 return code to signify the error.\nAs noted above, none of the callers who want the drop-to-stage-0 behavior\ncheck the return status, though, so this means all remaining unmerged\nentries remain in the index and the callers proceed assuming otherwise.\nUsers then see errors of the form:\n\n    error: 'DIR-OR-FILE' appears as both a file and as a directory\n    error: DIR-OR-FILE: cannot drop to stage #0\n\nand potentially also messages about other unmerged entries which came\nlexicographically later than whatever pathname was both a file and a\ndirectory.  Google finds a few hits searching for those messages,\nsuggesting there were probably a couple people who hit this besides me.\nLuckily, calling `git reset --hard` multiple times would workaround\nthis bug.\n\nSince the whole purpose here is to just put the entry *temporarily* into\nthe index so that any associated file in the working copy can be removed,\nwe can just skip the DFCHECK and allow both the file and directory to\nappear in the index.  The temporary simultaneous appearance of the\ndirectory and file entries in the index will be removed by the callers\nby calling unpack_trees(), which excludes these unmerged entries marked\nwith CE_CONFLICTED flag from the resulting index, before they attempt to\nwrite the index anywhere.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n read-cache.c                         | 13 ++++++++-----\n t/t1015-read-index-unmerged.sh       |  8 ++++----\n t/t6020-merge-df.sh                  |  3 ---\n t/t6042-merge-rename-corner-cases.sh |  1 -\n 4 files changed, 12 insertions(+), 13 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 372588260e..666d295a5a 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2632,10 +2632,13 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \n /*\n  * Read the index file that is potentially unmerged into given\n- * index_state, dropping any unmerged entries.  Returns true if\n- * the index is unmerged.  Callers who want to refuse to work\n- * from an unmerged state can call this and check its return value,\n- * instead of calling read_cache().\n+ * index_state, dropping any unmerged entries to stage #0 (potentially\n+ * resulting in a path appearing as both a file and a directory in the\n+ * index; the caller is responsible to clear out the extra entries\n+ * before writing the index to a tree).  Returns true if the index is\n+ * unmerged.  Callers who want to refuse to work from an unmerged\n+ * state can call this and check its return value, instead of calling\n+ * read_cache().\n  */\n int read_index_unmerged(struct index_state *istate)\n {\n@@ -2658,7 +2661,7 @@ int read_index_unmerged(struct index_state *istate)\n \t\tnew_ce->ce_flags = create_ce_flags(0) | CE_CONFLICTED;\n \t\tnew_ce->ce_namelen = len;\n \t\tnew_ce->ce_mode = ce->ce_mode;\n-\t\tif (add_index_entry(istate, new_ce, 0))\n+\t\tif (add_index_entry(istate, new_ce, ADD_CACHE_SKIP_DFCHECK))\n \t\t\treturn error(\"%s: cannot drop to stage #0\",\n \t\t\t\t     new_ce->name);\n \t}\ndiff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\nindex 32ef6bdcfa..55d22da32c 100755\n--- a/t/t1015-read-index-unmerged.sh\n+++ b/t/t1015-read-index-unmerged.sh\n@@ -30,7 +30,7 @@ test_expect_success 'setup modify/delete + directory/file conflict' '\n \t)\n '\n \n-test_expect_failure 'read-tree --reset cleans unmerged entries' '\n+test_expect_success 'read-tree --reset cleans unmerged entries' '\n \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n \t(\n@@ -45,7 +45,7 @@ test_expect_failure 'read-tree --reset cleans unmerged entries' '\n \t)\n '\n \n-test_expect_failure 'One reset --hard cleans unmerged entries' '\n+test_expect_success 'One reset --hard cleans unmerged entries' '\n \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n \t(\n@@ -87,7 +87,7 @@ test_expect_success 'setup directory/file conflict + simple edit/edit' '\n \t)\n '\n \n-test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n+test_expect_success 'git merge --abort succeeds despite D/F conflict' '\n \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n \t(\n@@ -103,7 +103,7 @@ test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n \t)\n '\n \n-test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n+test_expect_success 'git am --skip succeeds despite D/F conflict' '\n \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n \t(\ndiff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex 2af1beec5f..46b506b3b7 100755\n--- a/t/t6020-merge-df.sh\n+++ b/t/t6020-merge-df.sh\n@@ -89,9 +89,6 @@ test_expect_success 'modify/delete + directory/file conflict' '\n '\n \n test_expect_success 'modify/delete + directory/file conflict; other way' '\n-\t# Yes, we really need the double reset since \"letters\" appears as\n-\t# both a file and a directory.\n-\tgit reset --hard &&\n \tgit reset --hard &&\n \tgit clean -f &&\n \tgit checkout modify^0 &&\ndiff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\nindex 1cbd946fc2..583e68997e 100755\n--- a/t/t6042-merge-rename-corner-cases.sh\n+++ b/t/t6042-merge-rename-corner-cases.sh\n@@ -323,7 +323,6 @@ test_expect_success 'rename/directory conflict + content merge conflict' '\n \t(\n \t\tcd rename-directory-1 &&\n \n-\t\tgit reset --hard &&\n \t\tgit reset --hard &&\n \t\tgit clean -fdqx &&\n \n-- \n2.18.0.645.g72fe132ec2\n\n"},{"id":"352475","messageId":"20180713163331.22446-2-newren@gmail.com","threadId":"48858","inReplyTo":"20180713163331.22446-1-newren@gmail.com","subject":"[PATCH v2 1/2] t1015: demonstrate directory/file conflict recovery failures","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-13T16:33:30Z","receivedAt":"2018-07-13T16:33:42Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Several \"recovery\" commands outright fail or do not fully recover\nwhen directory-file conflicts are present.  This includes:\n  * git read-tree --reset HEAD\n  * git am --skip\n  * git am --abort\n  * git merge --abort\n  * git reset --hard\n\nAdd testcases documenting these shortcomings.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n t/t1015-read-index-unmerged.sh | 123 +++++++++++++++++++++++++++++++++\n 1 file changed, 123 insertions(+)\n create mode 100755 t/t1015-read-index-unmerged.sh\n\ndiff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\nnew file mode 100755\nindex 0000000000..32ef6bdcfa\n--- /dev/null\n+++ b/t/t1015-read-index-unmerged.sh\n@@ -0,0 +1,123 @@\n+#!/bin/sh\n+\n+test_description='Test various callers of read_index_unmerged'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup modify/delete + directory/file conflict' '\n+\ttest_create_repo df_plus_modify_delete &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\ttest_write_lines a b c d e f g h >letters &&\n+\t\tgit add letters &&\n+\t\tgit commit -m initial &&\n+\n+\t\tgit checkout -b modify &&\n+\t\t# Throw in letters.txt for sorting order fun\n+\t\t# (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n+\t\techo i >>letters &&\n+\t\techo \"version 2\" >letters.txt &&\n+\t\tgit add letters letters.txt &&\n+\t\tgit commit -m modified &&\n+\n+\t\tgit checkout -b delete HEAD^ &&\n+\t\tgit rm letters &&\n+\t\tmkdir letters &&\n+\t\t>letters/file &&\n+\t\techo \"version 1\" >letters.txt &&\n+\t\tgit add letters letters.txt &&\n+\t\tgit commit -m deleted\n+\t)\n+'\n+\n+test_expect_failure 'read-tree --reset cleans unmerged entries' '\n+\ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\tgit checkout delete^0 &&\n+\t\ttest_must_fail git merge modify &&\n+\n+\t\tgit read-tree --reset HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_failure 'One reset --hard cleans unmerged entries' '\n+\ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\tgit checkout delete^0 &&\n+\t\ttest_must_fail git merge modify &&\n+\n+\t\tgit reset --hard &&\n+\t\ttest_path_is_missing .git/MERGE_HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_success 'setup directory/file conflict + simple edit/edit' '\n+\ttest_create_repo df_plus_edit_edit &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\ttest_seq 1 10 >numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m initial &&\n+\n+\t\tgit checkout -b d-edit &&\n+\t\tmkdir foo &&\n+\t\techo content >foo/bar &&\n+\t\tgit add foo &&\n+\t\techo 11 >>numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m \"directory and edit\" &&\n+\n+\t\tgit checkout -b f-edit d-edit^1 &&\n+\t\techo content >foo &&\n+\t\tgit add foo &&\n+\t\techo eleven >>numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m \"file and edit\"\n+\t)\n+'\n+\n+test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n+\ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\tgit checkout f-edit^0 &&\n+\t\ttest_must_fail git merge d-edit^0 &&\n+\n+\t\tgit merge --abort &&\n+\t\ttest_path_is_missing .git/MERGE_HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n+\ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\tgit checkout f-edit^0 &&\n+\t\tgit format-patch -1 d-edit &&\n+\t\ttest_must_fail git am -3 0001*.patch &&\n+\n+\t\tgit am --skip &&\n+\t\ttest_path_is_missing .git/rebase-apply &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_done\n-- \n2.18.0.645.g72fe132ec2\n\n"},{"id":"352477","messageId":"CABPp-BGtrsAzvT7=95vKUXveQ=3X6FkAc5VgEYBNRXSzsqY0DA@mail.gmail.com","threadId":"48858","inReplyTo":"20180713163331.22446-1-newren@gmail.com","subject":"Re: [PATCH v2 0/2] Address recovery failures with directory/file conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-13T16:45:47Z","receivedAt":"2018-07-13T16:45:50Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jul 13, 2018 at 9:33 AM, Elijah Newren <newren@gmail.com> wrote:\n> This patch series fixes several \"recovery\" commands that outright fail\n> or do not fully recover when directory-file conflicts are present.\n> This includes:\n>    * git read-tree --reset HEAD\n>    * git am --skip\n>    * git am --abort\n>    * git merge --abort (or git reset --merge)\n>    * git reset --hard\n>\n> Changes since v1 (full range-diff below):\n>   - Make use of test_write_lines, as suggested by Eric.\n>   - Provide a little more explanation in one of the commit messages, as\n>     suggested by Junio.\n\nI forgot to mention: there is also an expected failing merge --abort\ntestcase in en/t7405-recursive-submodule-conflicts (currently in pu),\nand the fact that this series does not fix that testcase is known.\nFor the curious...\n\nThis read-index-unmerged series is part of the solution for the\nproblem highlighted in t7405; in fact, the similarity between\nfile/directory and submodule/directory conflicts is such that this\npatch series does also happen to fix recovery problems for many\nsubmodule/directory conflicts as well.  However, additional work is\nneeded when the same path is found under both the submodule and the\ndirectory, and the testcase in en/t7405-recursive-submodule-conflicts\nis precisely one of those cases that needs additional work.\n\n(This is just a long winded way of saying that this series and\nen/t7405-recursive-submodule-conflicts don't conflict or duplicate\neach other.)\n"},{"id":"354061","messageId":"20180731171205.4794-1-newren@gmail.com","threadId":"48858","inReplyTo":"20180713163331.22446-1-newren@gmail.com","subject":"[PATCH v3 0/2] Address recovery failures with directory/file conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-31T17:12:03Z","receivedAt":"2018-07-31T17:12:16Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series fixes several \"recovery\" commands that outright fail\nor do not fully recover when directory-file conflicts are present.\nThis includes:\n   * git read-tree --reset HEAD\n   * git am --skip\n   * git am --abort\n   * git merge --abort (or git reset --merge)\n   * git reset --hard\n\nChanges since v2 (full range-diff below):\n  - Backported to maint (there were some textual conflicts in t6042\n    due to the merging of en/merge-recursive-tests to master), because\n    of this comment from Junio's what's cooking email:\n\n    \"This may have to be rebased on an older maintenance track before\n     moving forward.\"\n\nElijah Newren (2):\n  t1015: demonstrate directory/file conflict recovery failures\n  read-cache: fix directory/file conflict handling in\n    read_index_unmerged()\n\n read-cache.c                         |  13 +--\n t/t1015-read-index-unmerged.sh       | 123 +++++++++++++++++++++++++++\n t/t6020-merge-df.sh                  |   3 -\n t/t6042-merge-rename-corner-cases.sh |   2 -\n 4 files changed, 131 insertions(+), 10 deletions(-)\n create mode 100755 t/t1015-read-index-unmerged.sh\n\n1:  4a1c9c3368 ! 1:  00f94a8b41 t1015: demonstrate directory/file conflict recovery failures\n    @@ -14,7 +14,6 @@\n     \n         Signed-off-by: Elijah Newren <newren@gmail.com>\n         Signed-off-by: Junio C Hamano <gitster@pobox.com>\n    -    Message-Id: <20180713163331.22446-2-newren@gmail.com>\n     \n     diff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\n     new file mode 100755\n2:  e105e8bfbd ! 2:  d3b8d7edb6 read-cache: fix directory/file conflict handling in read_index_unmerged()\n    @@ -59,7 +59,6 @@\n     \n         Signed-off-by: Elijah Newren <newren@gmail.com>\n         Signed-off-by: Junio C Hamano <gitster@pobox.com>\n    -    Message-Id: <20180713163331.22446-3-newren@gmail.com>\n     \n     diff --git a/read-cache.c b/read-cache.c\n     --- a/read-cache.c\n    @@ -150,10 +149,18 @@\n     --- a/t/t6042-merge-rename-corner-cases.sh\n     +++ b/t/t6042-merge-rename-corner-cases.sh\n     @@\n    - \t(\n    - \t\tcd rename-directory-1 &&\n    + '\n    +\n    + test_expect_success 'rename/directory conflict + clean content merge' '\n    +-\tgit reset --hard &&\n    +\tgit reset --hard &&\n    +\tgit clean -fdqx &&\n      \n    --\t\tgit reset --hard &&\n    - \t\tgit reset --hard &&\n    - \t\tgit clean -fdqx &&\n    +@@\n    + '\n    +\n    + test_expect_success 'rename/directory conflict + content merge conflict' '\n    +-\tgit reset --hard &&\n    +\tgit reset --hard &&\n    +\tgit clean -fdqx &&\n      \n-- \n2.18.0.2.gf4c50c7885\n"},{"id":"354062","messageId":"20180731171205.4794-3-newren@gmail.com","threadId":"48858","inReplyTo":"20180731171205.4794-1-newren@gmail.com","subject":"[PATCH v3 2/2] read-cache: fix directory/file conflict handling in read_index_unmerged()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-31T17:12:05Z","receivedAt":"2018-07-31T17:12:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"read_index_unmerged() has two intended purposes:\n  * return 1 if there are any unmerged entries, 0 otherwise\n  * drops any higher-stage entries down to stage #0\n\nThere are several callers of read_index_unmerged() that check the return\nvalue to see if it is non-zero, all of which then die() if that condition\nis met.  For these callers, dropping higher-stage entries down to stage #0\nis a waste of resources, and returning immediately on first unmerged entry\nwould be better.  But it's probably only a very minor difference and isn't\nthe focus of this series.\n\nThe remaining callers ignore the return value and call this function for\nthe side effect of dropping higher-stage entries down to stage #0.  As\nmentioned in commit e11d7b596970 (\"'reset --merge': fix unmerged case\",\n2009-12-31),\n\n    The _only_ reason we want to keep a previously unmerged entry in the\n    index at stage #0 is so that we don't forget the fact that we have\n    corresponding file in the work tree in order to be able to remove it\n    when the tree we are resetting to does not have the path.\n\nIn fact, prior to commit d1a43f2aa4bf (\"reset --hard/read-tree --reset -u:\nremove unmerged new paths\", 2008-10-15), read_index_unmerged() did just\nremove unmerged entries from the cache immediately but that had the\nunwanted effect of leaving around new untracked files in the tree from\naborted merges.\n\nSo, that's the intended purpose of this function.  The problem is that\nwhen directory/files conflicts are present, trying to add the file to the\nindex at stage 0 fails (because there is still a directory in the way),\nand the function returns early with a -1 return code to signify the error.\nAs noted above, none of the callers who want the drop-to-stage-0 behavior\ncheck the return status, though, so this means all remaining unmerged\nentries remain in the index and the callers proceed assuming otherwise.\nUsers then see errors of the form:\n\n    error: 'DIR-OR-FILE' appears as both a file and as a directory\n    error: DIR-OR-FILE: cannot drop to stage #0\n\nand potentially also messages about other unmerged entries which came\nlexicographically later than whatever pathname was both a file and a\ndirectory.  Google finds a few hits searching for those messages,\nsuggesting there were probably a couple people who hit this besides me.\nLuckily, calling `git reset --hard` multiple times would workaround\nthis bug.\n\nSince the whole purpose here is to just put the entry *temporarily* into\nthe index so that any associated file in the working copy can be removed,\nwe can just skip the DFCHECK and allow both the file and directory to\nappear in the index.  The temporary simultaneous appearance of the\ndirectory and file entries in the index will be removed by the callers\nby calling unpack_trees(), which excludes these unmerged entries marked\nwith CE_CONFLICTED flag from the resulting index, before they attempt to\nwrite the index anywhere.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n read-cache.c                         | 13 ++++++++-----\n t/t1015-read-index-unmerged.sh       |  8 ++++----\n t/t6020-merge-df.sh                  |  3 ---\n t/t6042-merge-rename-corner-cases.sh |  2 --\n 4 files changed, 12 insertions(+), 14 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 372588260e..666d295a5a 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2632,10 +2632,13 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \n /*\n  * Read the index file that is potentially unmerged into given\n- * index_state, dropping any unmerged entries.  Returns true if\n- * the index is unmerged.  Callers who want to refuse to work\n- * from an unmerged state can call this and check its return value,\n- * instead of calling read_cache().\n+ * index_state, dropping any unmerged entries to stage #0 (potentially\n+ * resulting in a path appearing as both a file and a directory in the\n+ * index; the caller is responsible to clear out the extra entries\n+ * before writing the index to a tree).  Returns true if the index is\n+ * unmerged.  Callers who want to refuse to work from an unmerged\n+ * state can call this and check its return value, instead of calling\n+ * read_cache().\n  */\n int read_index_unmerged(struct index_state *istate)\n {\n@@ -2658,7 +2661,7 @@ int read_index_unmerged(struct index_state *istate)\n \t\tnew_ce->ce_flags = create_ce_flags(0) | CE_CONFLICTED;\n \t\tnew_ce->ce_namelen = len;\n \t\tnew_ce->ce_mode = ce->ce_mode;\n-\t\tif (add_index_entry(istate, new_ce, 0))\n+\t\tif (add_index_entry(istate, new_ce, ADD_CACHE_SKIP_DFCHECK))\n \t\t\treturn error(\"%s: cannot drop to stage #0\",\n \t\t\t\t     new_ce->name);\n \t}\ndiff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\nindex 32ef6bdcfa..55d22da32c 100755\n--- a/t/t1015-read-index-unmerged.sh\n+++ b/t/t1015-read-index-unmerged.sh\n@@ -30,7 +30,7 @@ test_expect_success 'setup modify/delete + directory/file conflict' '\n \t)\n '\n \n-test_expect_failure 'read-tree --reset cleans unmerged entries' '\n+test_expect_success 'read-tree --reset cleans unmerged entries' '\n \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n \t(\n@@ -45,7 +45,7 @@ test_expect_failure 'read-tree --reset cleans unmerged entries' '\n \t)\n '\n \n-test_expect_failure 'One reset --hard cleans unmerged entries' '\n+test_expect_success 'One reset --hard cleans unmerged entries' '\n \ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n \ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n \t(\n@@ -87,7 +87,7 @@ test_expect_success 'setup directory/file conflict + simple edit/edit' '\n \t)\n '\n \n-test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n+test_expect_success 'git merge --abort succeeds despite D/F conflict' '\n \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n \t(\n@@ -103,7 +103,7 @@ test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n \t)\n '\n \n-test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n+test_expect_success 'git am --skip succeeds despite D/F conflict' '\n \ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n \ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n \t(\ndiff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex 2af1beec5f..46b506b3b7 100755\n--- a/t/t6020-merge-df.sh\n+++ b/t/t6020-merge-df.sh\n@@ -89,9 +89,6 @@ test_expect_success 'modify/delete + directory/file conflict' '\n '\n \n test_expect_success 'modify/delete + directory/file conflict; other way' '\n-\t# Yes, we really need the double reset since \"letters\" appears as\n-\t# both a file and a directory.\n-\tgit reset --hard &&\n \tgit reset --hard &&\n \tgit clean -f &&\n \tgit checkout modify^0 &&\ndiff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\nindex 411550d2b6..de77bfaf4f 100755\n--- a/t/t6042-merge-rename-corner-cases.sh\n+++ b/t/t6042-merge-rename-corner-cases.sh\n@@ -235,7 +235,6 @@ test_expect_success 'setup content merge + rename/directory conflict' '\n '\n \n test_expect_success 'rename/directory conflict + clean content merge' '\n-\tgit reset --hard &&\n \tgit reset --hard &&\n \tgit clean -fdqx &&\n \n@@ -259,7 +258,6 @@ test_expect_success 'rename/directory conflict + clean content merge' '\n '\n \n test_expect_success 'rename/directory conflict + content merge conflict' '\n-\tgit reset --hard &&\n \tgit reset --hard &&\n \tgit clean -fdqx &&\n \n-- \n2.18.0.2.gf4c50c7885\n\n"},{"id":"354063","messageId":"20180731171205.4794-2-newren@gmail.com","threadId":"48858","inReplyTo":"20180731171205.4794-1-newren@gmail.com","subject":"[PATCH v3 1/2] t1015: demonstrate directory/file conflict recovery failures","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-31T17:12:04Z","receivedAt":"2018-07-31T17:12:18Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Several \"recovery\" commands outright fail or do not fully recover\nwhen directory-file conflicts are present.  This includes:\n  * git read-tree --reset HEAD\n  * git am --skip\n  * git am --abort\n  * git merge --abort\n  * git reset --hard\n\nAdd testcases documenting these shortcomings.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t1015-read-index-unmerged.sh | 123 +++++++++++++++++++++++++++++++++\n 1 file changed, 123 insertions(+)\n create mode 100755 t/t1015-read-index-unmerged.sh\n\ndiff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh\nnew file mode 100755\nindex 0000000000..32ef6bdcfa\n--- /dev/null\n+++ b/t/t1015-read-index-unmerged.sh\n@@ -0,0 +1,123 @@\n+#!/bin/sh\n+\n+test_description='Test various callers of read_index_unmerged'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup modify/delete + directory/file conflict' '\n+\ttest_create_repo df_plus_modify_delete &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\ttest_write_lines a b c d e f g h >letters &&\n+\t\tgit add letters &&\n+\t\tgit commit -m initial &&\n+\n+\t\tgit checkout -b modify &&\n+\t\t# Throw in letters.txt for sorting order fun\n+\t\t# (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n+\t\techo i >>letters &&\n+\t\techo \"version 2\" >letters.txt &&\n+\t\tgit add letters letters.txt &&\n+\t\tgit commit -m modified &&\n+\n+\t\tgit checkout -b delete HEAD^ &&\n+\t\tgit rm letters &&\n+\t\tmkdir letters &&\n+\t\t>letters/file &&\n+\t\techo \"version 1\" >letters.txt &&\n+\t\tgit add letters letters.txt &&\n+\t\tgit commit -m deleted\n+\t)\n+'\n+\n+test_expect_failure 'read-tree --reset cleans unmerged entries' '\n+\ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\tgit checkout delete^0 &&\n+\t\ttest_must_fail git merge modify &&\n+\n+\t\tgit read-tree --reset HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_failure 'One reset --hard cleans unmerged entries' '\n+\ttest_when_finished \"git -C df_plus_modify_delete clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_modify_delete reset --hard\" &&\n+\t(\n+\t\tcd df_plus_modify_delete &&\n+\n+\t\tgit checkout delete^0 &&\n+\t\ttest_must_fail git merge modify &&\n+\n+\t\tgit reset --hard &&\n+\t\ttest_path_is_missing .git/MERGE_HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_success 'setup directory/file conflict + simple edit/edit' '\n+\ttest_create_repo df_plus_edit_edit &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\ttest_seq 1 10 >numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m initial &&\n+\n+\t\tgit checkout -b d-edit &&\n+\t\tmkdir foo &&\n+\t\techo content >foo/bar &&\n+\t\tgit add foo &&\n+\t\techo 11 >>numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m \"directory and edit\" &&\n+\n+\t\tgit checkout -b f-edit d-edit^1 &&\n+\t\techo content >foo &&\n+\t\tgit add foo &&\n+\t\techo eleven >>numbers &&\n+\t\tgit add numbers &&\n+\t\tgit commit -m \"file and edit\"\n+\t)\n+'\n+\n+test_expect_failure 'git merge --abort succeeds despite D/F conflict' '\n+\ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\tgit checkout f-edit^0 &&\n+\t\ttest_must_fail git merge d-edit^0 &&\n+\n+\t\tgit merge --abort &&\n+\t\ttest_path_is_missing .git/MERGE_HEAD &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_expect_failure 'git am --skip succeeds despite D/F conflict' '\n+\ttest_when_finished \"git -C df_plus_edit_edit clean -f\" &&\n+\ttest_when_finished \"git -C df_plus_edit_edit reset --hard\" &&\n+\t(\n+\t\tcd df_plus_edit_edit &&\n+\n+\t\tgit checkout f-edit^0 &&\n+\t\tgit format-patch -1 d-edit &&\n+\t\ttest_must_fail git am -3 0001*.patch &&\n+\n+\t\tgit am --skip &&\n+\t\ttest_path_is_missing .git/rebase-apply &&\n+\t\tgit ls-files -u >conflicts &&\n+\t\ttest_must_be_empty conflicts\n+\t)\n+'\n+\n+test_done\n-- \n2.18.0.2.gf4c50c7885\n\n"}]}