{"thread":{"id":"51638","subject":"[RFC PATCH] unpack-trees.c: handle empty deleted ita files","startedAt":"2019-08-13T16:04:52Z","lastAt":"2020-02-14T17:15:32Z","messageCount":12,"participants":["Varun Naik","René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"380392","messageId":"20190813160353.50018-1-vcnaik94@gmail.com","threadId":"51638","inReplyTo":null,"subject":"[RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"Varun Naik","fromEmail":"vcnaik94@gmail.com","sentAt":"2019-08-13T16:03:53Z","receivedAt":"2019-08-13T16:04:52Z","isPatch":true,"sender":{"key":"vcnaik94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2382501?v=4"},"body":"It is possible to delete a committed file from the index and then add it\nas intent-to-add. Several variations of \"reset\" and \"checkout\" should\nresurrect the file in the index from HEAD. \"merge\", \"cherry-pick\", and\n\"revert\" should all fail with an error message. This patch provides the\ndesired behavior even when the file is empty in HEAD.\n\nThe affected commands all compare two cache entries by calling\nunpack-trees.c:same(). A cache entry for an ita file and a cache entry\nfor an empty file have the same oid. So, the cache entry for an empty\ndeleted ita file was previously considered the \"same\" as the cache entry\nfor an empty file. This fix adds a comparison of the intent-to-add bits\nso that the two cache entries are no longer considered the \"same\".\n\nSigned-off-by: Varun Naik <vcnaik94@gmail.com>\n---\nI am marking this patch as RFC because it is changing code deep in\nunpack-trees.c, and I'm sure it will generate some controversy :)\n\nThe affected \"reset\" and \"checkout\" commands call\nunpack-trees.c:oneway_merge(), which calls same(). The affected \"merge\",\n\"cherry-pick\", and \"revert\" commands call\nunpack-trees.c:threeway_merge(), which calls same(). \"stash\" also calls\noneway_merge(), and \"rebase\" also calls threeway_merge(), but they are\nnot included in the test cases because their behaviors have not changed.\n\nThe new tests are not comprehensive. In particular, they don't call\nplumbing commands, such as \"read-tree\". But hopefully they provide\nenough coverage to prevent most regressions.\n\nThe new test cases for \"cherry-pick\" and \"revert\" grep for the single\nword \"overwritten\" rather than a more precise error message because the\nerror message for an empty deleted ita file changes slightly if the\npatch in [0] is also applied. In retrospect, the commands affected by\n[0], [1], and this patch were all intertwined, and it would have been\nbetter to create a single large patch instead of three smaller patches.\n\n[0]: https://public-inbox.org/git/20190801161558.12838-1-vcnaik94@gmail.com/\n[1]: https://public-inbox.org/git/20190802162852.14498-1-vcnaik94@gmail.com/\n\n t/t3030-merge-recursive.sh    | 25 +++++++++++++++---\n t/t3501-revert-cherry-pick.sh | 49 ++++++++++++++++++++++++++++++++++-\n t/t7104-reset-hard.sh         | 11 ++++++++\n t/t7110-reset-merge.sh        | 31 ++++++++++++++++++++++\n t/t7201-co.sh                 | 12 +++++++++\n unpack-trees.c                |  1 +\n 6 files changed, 125 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex ff641b348a..8aebb829a6 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -303,13 +303,32 @@ test_expect_success 'fail if the index has unresolved entries' '\n \tgit checkout -f \"$c1\" &&\n \n \ttest_must_fail git merge \"$c5\" &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"not possible because you have unmerged files\" out &&\n \tgit add -u &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"You have not concluded your merge\" out &&\n \trm -f .git/MERGE_HEAD &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n+'\n+\n+test_expect_success 'fail if a deleted intent-to-add file exists in the index' '\n+\tgit checkout -f \"$c1\" &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n+\tgit checkout -f \"$c1\" &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n '\n \ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex d1c68af8c5..45d816fc0c 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -91,16 +91,63 @@ test_expect_success 'cherry-pick on stat-dirty working tree' '\n \t)\n '\n \n-test_expect_success 'revert forbidden on dirty working tree' '\n+test_expect_success 'cherry-pick forbidden on dirty working tree' '\n \n+\tgit checkout -b temp &&\n \techo content >extra_file &&\n \tgit add extra_file &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"your local changes would be overwritten by \" errors\n+\n+'\n+\n+test_expect_success 'revert forbidden on dirty working tree' '\n+\n \ttest_must_fail git revert HEAD 2>errors &&\n \ttest_i18ngrep \"your local changes would be overwritten by \" errors\n \n '\n \n+test_expect_success 'cherry-pick fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors\n+'\n+\n+test_expect_success 'revert fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors\n+'\n+\n test_expect_success 'cherry-pick on unborn branch' '\n+\tgit checkout -f rename1 &&\n \tgit checkout --orphan unborn &&\n \tgit rm --cached -r . &&\n \trm -rf * &&\ndiff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh\nindex 16faa07813..96a0b779e7 100755\n--- a/t/t7104-reset-hard.sh\n+++ b/t/t7104-reset-hard.sh\n@@ -43,4 +43,15 @@ test_expect_success 'reset --hard did not corrupt index or cached-tree' '\n \n '\n \n+test_expect_success 'reset --hard adds deleted intent-to-add file back to index' '\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit reset --hard &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\nindex a82a07a04a..3346759375 100755\n--- a/t/t7110-reset-merge.sh\n+++ b/t/t7110-reset-merge.sh\n@@ -292,4 +292,35 @@ test_expect_success '--keep fails with added/deleted merge' '\n     test_i18ngrep \"middle of a merge\" err.log\n '\n \n+test_expect_success 'reset --merge adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    git add nonempty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached nonempty &&\n+    git add -N nonempty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep nonempty err.log | grep \"not uptodate\" &&\n+    git reset --hard initial &&\n+    >empty &&\n+    git add empty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached empty &&\n+    git add -N empty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep empty err.log | grep \"not uptodate\"\n+'\n+\n+test_expect_success 'reset --keep adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    >empty &&\n+    git add nonempty empty &&\n+    git commit -m \"create files to be deleted\" &&\n+    git rm --cached nonempty empty &&\n+    git add -N nonempty empty &&\n+    git reset --keep HEAD &&\n+    git diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 5990299fc9..4c0c33ce33 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -674,4 +674,16 @@ test_expect_success 'custom merge driver with checkout -m' '\n \ttest_cmp expect arm\n '\n \n+test_expect_success 'checkout -f HEAD adds deleted intent-to-add file back to index' '\n+\tgit reset --hard master &&\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit checkout -f HEAD &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 50189909b8..9b7e6b01c4 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1661,6 +1661,7 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n \tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n \t\treturn 0;\n \treturn a->ce_mode == b->ce_mode &&\n+\t       !ce_intent_to_add(a) == !ce_intent_to_add(b) &&\n \t       oideq(&a->oid, &b->oid);\n }\n \n-- \n2.22.0\n\n"},{"id":"380402","messageId":"b7f4b745-8942-6d90-dbc5-7f79f2cc323e@web.de","threadId":"51638","inReplyTo":"20190813160353.50018-1-vcnaik94@gmail.com","subject":"Re: [RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-13T18:08:06Z","receivedAt":"2019-08-13T18:08:15Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 13.08.19 um 18:03 schrieb Varun Naik:\n> It is possible to delete a committed file from the index and then add it\n> as intent-to-add. Several variations of \"reset\" and \"checkout\" should\n> resurrect the file in the index from HEAD. \"merge\", \"cherry-pick\", and\n> \"revert\" should all fail with an error message. This patch provides the\n> desired behavior even when the file is empty in HEAD.\n>\n> The affected commands all compare two cache entries by calling\n> unpack-trees.c:same(). A cache entry for an ita file and a cache entry\n> for an empty file have the same oid. So, the cache entry for an empty\n> deleted ita file was previously considered the \"same\" as the cache entry\n> for an empty file. This fix adds a comparison of the intent-to-add bits\n> so that the two cache entries are no longer considered the \"same\".\n>\n> Signed-off-by: Varun Naik <vcnaik94@gmail.com>\n> ---\n> I am marking this patch as RFC because it is changing code deep in\n> unpack-trees.c, and I'm sure it will generate some controversy :)\n\nLacking experience with intent-to-add I don't see why this would be\ncontroversial.  Copying Duy and quoting in full, as he might have more\nto say on that topic.\n\nI have just one silly question below.\n\n>\n> The affected \"reset\" and \"checkout\" commands call\n> unpack-trees.c:oneway_merge(), which calls same(). The affected \"merge\",\n> \"cherry-pick\", and \"revert\" commands call\n> unpack-trees.c:threeway_merge(), which calls same(). \"stash\" also calls\n> oneway_merge(), and \"rebase\" also calls threeway_merge(), but they are\n> not included in the test cases because their behaviors have not changed.\n>\n> The new tests are not comprehensive. In particular, they don't call\n> plumbing commands, such as \"read-tree\". But hopefully they provide\n> enough coverage to prevent most regressions.\n>\n> The new test cases for \"cherry-pick\" and \"revert\" grep for the single\n> word \"overwritten\" rather than a more precise error message because the\n> error message for an empty deleted ita file changes slightly if the\n> patch in [0] is also applied. In retrospect, the commands affected by\n> [0], [1], and this patch were all intertwined, and it would have been\n> better to create a single large patch instead of three smaller patches.\n>\n> [0]: https://public-inbox.org/git/20190801161558.12838-1-vcnaik94@gmail.com/\n> [1]: https://public-inbox.org/git/20190802162852.14498-1-vcnaik94@gmail.com/\n>\n>  t/t3030-merge-recursive.sh    | 25 +++++++++++++++---\n>  t/t3501-revert-cherry-pick.sh | 49 ++++++++++++++++++++++++++++++++++-\n>  t/t7104-reset-hard.sh         | 11 ++++++++\n>  t/t7110-reset-merge.sh        | 31 ++++++++++++++++++++++\n>  t/t7201-co.sh                 | 12 +++++++++\n>  unpack-trees.c                |  1 +\n>  6 files changed, 125 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\n> index ff641b348a..8aebb829a6 100755\n> --- a/t/t3030-merge-recursive.sh\n> +++ b/t/t3030-merge-recursive.sh\n> @@ -303,13 +303,32 @@ test_expect_success 'fail if the index has unresolved entries' '\n>  \tgit checkout -f \"$c1\" &&\n>\n>  \ttest_must_fail git merge \"$c5\" &&\n> -\ttest_must_fail git merge \"$c5\" 2> out &&\n> +\ttest_must_fail git merge \"$c5\" 2>out &&\n>  \ttest_i18ngrep \"not possible because you have unmerged files\" out &&\n>  \tgit add -u &&\n> -\ttest_must_fail git merge \"$c5\" 2> out &&\n> +\ttest_must_fail git merge \"$c5\" 2>out &&\n>  \ttest_i18ngrep \"You have not concluded your merge\" out &&\n>  \trm -f .git/MERGE_HEAD &&\n> -\ttest_must_fail git merge \"$c5\" 2> out &&\n> +\ttest_must_fail git merge \"$c5\" 2>out &&\n> +\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n> +'\n> +\n> +test_expect_success 'fail if a deleted intent-to-add file exists in the index' '\n> +\tgit checkout -f \"$c1\" &&\n> +\techo \"nonempty\" >nonempty &&\n> +\tgit add nonempty &&\n> +\tgit commit -m \"create file to be deleted\" &&\n> +\tgit rm --cached nonempty &&\n> +\tgit add -N nonempty &&\n> +\ttest_must_fail git merge \"$c5\" 2>out &&\n> +\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n> +\tgit checkout -f \"$c1\" &&\n> +\t>empty &&\n> +\tgit add empty &&\n> +\tgit commit -m \"create file to be deleted\" &&\n> +\tgit rm --cached empty &&\n> +\tgit add -N empty &&\n> +\ttest_must_fail git merge \"$c5\" 2>out &&\n>  \ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n>  '\n>\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index d1c68af8c5..45d816fc0c 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -91,16 +91,63 @@ test_expect_success 'cherry-pick on stat-dirty working tree' '\n>  \t)\n>  '\n>\n> -test_expect_success 'revert forbidden on dirty working tree' '\n> +test_expect_success 'cherry-pick forbidden on dirty working tree' '\n>\n> +\tgit checkout -b temp &&\n>  \techo content >extra_file &&\n>  \tgit add extra_file &&\n> +\ttest_must_fail git cherry-pick rename2 2>errors &&\n> +\ttest_i18ngrep \"your local changes would be overwritten by \" errors\n> +\n> +'\n> +\n> +test_expect_success 'revert forbidden on dirty working tree' '\n> +\n>  \ttest_must_fail git revert HEAD 2>errors &&\n>  \ttest_i18ngrep \"your local changes would be overwritten by \" errors\n>\n>  '\n>\n> +test_expect_success 'cherry-pick fails if a deleted intent-to-add file exists in the index' '\n> +\tgit reset --hard rename1 &&\n> +\techo \"nonempty\" >nonempty &&\n> +\tgit add nonempty &&\n> +\tgit commit -m \"create file to be deleted\" &&\n> +\tgit rm --cached nonempty &&\n> +\tgit add -N nonempty &&\n> +\ttest_must_fail git cherry-pick rename2 2>errors &&\n> +\ttest_i18ngrep \"overwritten\" errors &&\n> +\tgit reset --hard rename1 &&\n> +\t>empty &&\n> +\tgit add empty &&\n> +\tgit commit -m \"create file to be deleted\" &&\n> +\tgit rm --cached empty &&\n> +\tgit add -N empty &&\n> +\ttest_must_fail git cherry-pick rename2 2>errors &&\n> +\ttest_i18ngrep \"overwritten\" errors\n> +'\n> +\n> +test_expect_success 'revert fails if a deleted intent-to-add file exists in the index' '\n> +\tgit reset --hard rename1 &&\n> +\techo \"nonempty\" >nonempty &&\n> +\tgit add nonempty &&\n> +\tgit commit -m \"create file to be deleted\" &&\n> +\tgit rm --cached nonempty &&\n> +\tgit add -N nonempty &&\n> +\ttest_must_fail git revert rename2 2>errors &&\n> +\ttest_i18ngrep \"overwritten\" errors &&\n> +\tgit reset --hard rename1 &&\n> +\t>empty &&\n> +\tgit add empty &&\n> +\tgit commit -m \"create file to be deleted\" &&\n> +\tgit rm --cached empty &&\n> +\tgit add -N empty &&\n> +\ttest_must_fail git revert rename2 2>errors &&\n> +\ttest_i18ngrep \"overwritten\" errors\n> +'\n> +\n>  test_expect_success 'cherry-pick on unborn branch' '\n> +\tgit checkout -f rename1 &&\n>  \tgit checkout --orphan unborn &&\n>  \tgit rm --cached -r . &&\n>  \trm -rf * &&\n> diff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh\n> index 16faa07813..96a0b779e7 100755\n> --- a/t/t7104-reset-hard.sh\n> +++ b/t/t7104-reset-hard.sh\n> @@ -43,4 +43,15 @@ test_expect_success 'reset --hard did not corrupt index or cached-tree' '\n>\n>  '\n>\n> +test_expect_success 'reset --hard adds deleted intent-to-add file back to index' '\n> +\techo \"nonempty\" >nonempty &&\n> +\t>empty &&\n> +\tgit add nonempty empty &&\n> +\tgit commit -m \"create files to be deleted\" &&\n> +\tgit rm --cached nonempty empty &&\n> +\tgit add -N nonempty empty &&\n> +\tgit reset --hard &&\n> +\tgit diff --cached --exit-code nonempty empty\n> +'\n> +\n>  test_done\n> diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\n> index a82a07a04a..3346759375 100755\n> --- a/t/t7110-reset-merge.sh\n> +++ b/t/t7110-reset-merge.sh\n> @@ -292,4 +292,35 @@ test_expect_success '--keep fails with added/deleted merge' '\n>      test_i18ngrep \"middle of a merge\" err.log\n>  '\n>\n> +test_expect_success 'reset --merge adds deleted intent-to-add file back to index' '\n> +    git reset --hard initial &&\n> +    echo \"nonempty\" >nonempty &&\n> +    git add nonempty &&\n> +    git commit -m \"create file to be deleted\" &&\n> +    git rm --cached nonempty &&\n> +    git add -N nonempty &&\n> +    test_must_fail git reset --merge HEAD 2>err.log &&\n> +    grep nonempty err.log | grep \"not uptodate\" &&\n> +    git reset --hard initial &&\n> +    >empty &&\n> +    git add empty &&\n> +    git commit -m \"create file to be deleted\" &&\n> +    git rm --cached empty &&\n> +    git add -N empty &&\n> +    test_must_fail git reset --merge HEAD 2>err.log &&\n> +    grep empty err.log | grep \"not uptodate\"\n> +'\n> +\n> +test_expect_success 'reset --keep adds deleted intent-to-add file back to index' '\n> +    git reset --hard initial &&\n> +    echo \"nonempty\" >nonempty &&\n> +    >empty &&\n> +    git add nonempty empty &&\n> +    git commit -m \"create files to be deleted\" &&\n> +    git rm --cached nonempty empty &&\n> +    git add -N nonempty empty &&\n> +    git reset --keep HEAD &&\n> +    git diff --cached --exit-code nonempty empty\n> +'\n> +\n>  test_done\n> diff --git a/t/t7201-co.sh b/t/t7201-co.sh\n> index 5990299fc9..4c0c33ce33 100755\n> --- a/t/t7201-co.sh\n> +++ b/t/t7201-co.sh\n> @@ -674,4 +674,16 @@ test_expect_success 'custom merge driver with checkout -m' '\n>  \ttest_cmp expect arm\n>  '\n>\n> +test_expect_success 'checkout -f HEAD adds deleted intent-to-add file back to index' '\n> +\tgit reset --hard master &&\n> +\techo \"nonempty\" >nonempty &&\n> +\t>empty &&\n> +\tgit add nonempty empty &&\n> +\tgit commit -m \"create files to be deleted\" &&\n> +\tgit rm --cached nonempty empty &&\n> +\tgit add -N nonempty empty &&\n> +\tgit checkout -f HEAD &&\n> +\tgit diff --cached --exit-code nonempty empty\n> +'\n> +\n>  test_done\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 50189909b8..9b7e6b01c4 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -1661,6 +1661,7 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n>  \tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n>  \t\treturn 0;\n>  \treturn a->ce_mode == b->ce_mode &&\n> +\t       !ce_intent_to_add(a) == !ce_intent_to_add(b) &&\n\nWhy the bangs?  This would work just as well and be slightly easier to\nread without negating both sides, wouldn't it?\n\n>  \t       oideq(&a->oid, &b->oid);\n>  }\n>\n>\n\n"},{"id":"380416","messageId":"xmqqr25o7qmf.fsf@gitster-ct.c.googlers.com","threadId":"51638","inReplyTo":"b7f4b745-8942-6d90-dbc5-7f79f2cc323e@web.de","subject":"Re: [RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-13T20:32:56Z","receivedAt":"2019-08-13T20:33:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Am 13.08.19 um 18:03 schrieb Varun Naik:\n>> It is possible to delete a committed file from the index and then add it\n>> as intent-to-add. Several variations of \"reset\" and \"checkout\" should\n>> resurrect the file in the index from HEAD. \"merge\", \"cherry-pick\", and\n>> \"revert\" should all fail with an error message. This patch provides the\n>> desired behavior even when the file is empty in HEAD.\n>>\n>> The affected commands all compare two cache entries by calling\n>> unpack-trees.c:same(). A cache entry for an ita file and a cache entry\n>> for an empty file have the same oid. So, the cache entry for an empty\n>> deleted ita file was previously considered the \"same\" as the cache entry\n>> for an empty file. This fix adds a comparison of the intent-to-add bits\n>> so that the two cache entries are no longer considered the \"same\".\n>>\n>> Signed-off-by: Varun Naik <vcnaik94@gmail.com>\n>> ---\n>> I am marking this patch as RFC because it is changing code deep in\n>> unpack-trees.c, and I'm sure it will generate some controversy :)\n>\n> Lacking experience with intent-to-add I don't see why this would be\n> controversial.\n\nThe \"same()\" function here is used to compare two cache entries\n(either came from the in-core index, or fabricated out of one of the\ntree objects as a potential merge result to replace the one in the\nindex), and is expected to tell if they are the same or the\ndifferent, which is used for example in a logic like \"if the one in\nthe index and the one came from HEAD are not the same, then it is\nnot safe to continue the merge\", \"if the one came from HEAD and the\none from the other branch are the same, just keep the one from the\nindex (which may or may not be the same as HEAD)\".\n\nThe original code considered that two entries with the same mode and\nthe same \"contents\" are the same.  As nobody sane tracks an empty\nfile for an extended span of history, that meant that most of the\ntime, intent-to-add entries, which has the normal mode bits for the\nblobs (with or without the executable bit) and object name for a\nzero length blob, would have been judged \"different\".\n\nThis change extends the logic to the case in which the contents\nrecorded in the tree-ishes is also a zero length blob.  We used to\nsay the I-T-A entry was the same as the recorded blob, but whether\nthe recorded one in the tree-ish is 0-length or 1-byte blob, this\npatch does not want it to be declared the \"same\" as any I-T-A entry.\n\nSo in that sense, it makes the behaviour for I-T-A entries\nconsistent.  But it is a separate matter if the consistency is good;\nwe do not want our code to be consistently wrong ;-)\n\n>> diff --git a/unpack-trees.c b/unpack-trees.c\n>> index 50189909b8..9b7e6b01c4 100644\n>> --- a/unpack-trees.c\n>> +++ b/unpack-trees.c\n>> @@ -1661,6 +1661,7 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n>>  \tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n>>  \t\treturn 0;\n>>  \treturn a->ce_mode == b->ce_mode &&\n>> +\t       !ce_intent_to_add(a) == !ce_intent_to_add(b) &&\n>\n> Why the bangs?  This would work just as well and be slightly easier to\n> read without negating both sides, wouldn't it?\n\nLooking at the implementation of that macro, I agree.  If it were\n\"returns non-zero for true, zero for false, we do not guarantee that\nwe return the same non-zero value for true all the time\", then these\nbangs do make sense, but that is not the case here.\n\nBut more importantly, can both sides of the comparison be I-T-A\nentries?\n\nI offhand do not think such a situation can arise (a cache entry\nwith I-T-A bit on can only come from the in-core index, IIUC, and\nnever from a tree-ish in this codepath), but if we encouter such a\ncase, I would imagine that we do not want to treat an I-T-A entry to\nbe the same with anything else, including another I-T-A entry.\n\nSo perhaps \n\n+\t!ce_intent_to_add(a) && !ce_intent_to_add(b) &&\n\ni.e. \"a cache entry is eligible to be same with something else only\nwhen its I-T-A bit is unset\".\n\nI dunno.\n\n>>  \t       oideq(&a->oid, &b->oid);\n>>  }\n>>\n>>\n"},{"id":"380427","messageId":"22b814f7-4b71-aa68-3078-9cc40338a37d@web.de","threadId":"51638","inReplyTo":"xmqqr25o7qmf.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-14T04:30:50Z","receivedAt":"2019-08-14T04:31:03Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 13.08.19 um 22:32 schrieb Junio C Hamano:\n> So perhaps\n>\n> +\t!ce_intent_to_add(a) && !ce_intent_to_add(b) &&\n>\n> i.e. \"a cache entry is eligible to be same with something else only\n> when its I-T-A bit is unset\".\n\nFWIW, here's another way to express that:\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 62276d4fef..a62d67d131 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1658,7 +1658,7 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n \t\treturn 0;\n \tif (!a && !b)\n \t\treturn 1;\n-\tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n+\tif ((a->ce_flags | b->ce_flags) & (CE_CONFLICTED | CE_INTENT_TO_ADD))\n \t\treturn 0;\n \treturn a->ce_mode == b->ce_mode &&\n \t       oideq(&a->oid, &b->oid);\n\n"},{"id":"380476","messageId":"CAK_rgsGSfL2zQLP59O+PUCLGz0X8c4UQ3Zg=9M+nCww5C+X_gA@mail.gmail.com","threadId":"51638","inReplyTo":"xmqqr25o7qmf.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"Varun Naik","fromEmail":"vcnaik94@gmail.com","sentAt":"2019-08-15T16:17:09Z","receivedAt":"2019-08-15T16:17:22Z","isPatch":true,"sender":{"key":"vcnaik94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2382501?v=4"},"body":"On Tue, Aug 13, 2019 at 1:33 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> The original code considered that two entries with the same mode and\n> the same \"contents\" are the same.  As nobody sane tracks an empty\n> file for an extended span of history, that meant that most of the\n> time, intent-to-add entries, which has the normal mode bits for the\n> blobs (with or without the executable bit) and object name for a\n> zero length blob, would have been judged \"different\".\n>\n\nI agree, this edge case is really arcane. The rabbit hole was deep :)\n\n> So perhaps\n>\n> +       !ce_intent_to_add(a) && !ce_intent_to_add(b) &&\n>\n> i.e. \"a cache entry is eligible to be same with something else only\n> when its I-T-A bit is unset\".\n>\n\nI decided to follow René's suggestion in response to this. Patch coming soon.\n\nVarun\n"},{"id":"380477","messageId":"20190815162115.65008-1-vcnaik94@gmail.com","threadId":"51638","inReplyTo":"20190813160353.50018-1-vcnaik94@gmail.com","subject":"[PATCH v2] unpack-trees.c: distinguish ita files from empty files","fromName":"Varun Naik","fromEmail":"vcnaik94@gmail.com","sentAt":"2019-08-15T16:21:15Z","receivedAt":"2019-08-15T16:21:43Z","isPatch":true,"sender":{"key":"vcnaik94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2382501?v=4"},"body":"It is possible to delete a committed file from the index and then add it\nas intent-to-add. Several variations of \"reset\" and \"checkout\" should\nresurrect the file in the index from HEAD. \"merge\" (non-fast-forward),\n\"cherry-pick\", and \"revert\" should all fail with an error message. This\npatch provides the desired behavior even when the file is empty in HEAD.\n\nFurthermore, \"merge --ff-only\" should prevent a fast-forward merge if a\nfile is marked as intent-to-add. This patch provides the desired\nbehavior even when the file exists with empty contents in the final\ncommit.\n\nThe affected commands all compare two cache entries by calling\nunpack-trees.c:same(). Since a cache entry for an ita file and a cache\nentry for an empty file have the same oid, if the two files had the same\nmode, then the cache entries were previously considered the \"same\". This\nfix checks the intent-to-add bits so if one or both of the cache entries\nare marked as ita, then the two cache entries are not considered the\n\"same\".\n\nHelped-by: René Scharfe <l.s.r@web.de>\nSigned-off-by: Varun Naik <vcnaik94@gmail.com>\n---\nI rewrote the code in same(), as René suggested.\n\nNew in v2 is a test for \"merge --ff-only\", which calls\nunpack-trees.c:twoway_merge(), which calls unpack-trees.c:same().\nPreviously, if the file existed with empty contents in the final commit,\nthen the fast-forward merge went through, but the file ended up as a\ndeleted ita file in the index. Now, the merge fails with an error.\n\n t/t3030-merge-recursive.sh    | 25 +++++++++++++++---\n t/t3501-revert-cherry-pick.sh | 49 ++++++++++++++++++++++++++++++++++-\n t/t7104-reset-hard.sh         | 11 ++++++++\n t/t7110-reset-merge.sh        | 31 ++++++++++++++++++++++\n t/t7201-co.sh                 | 12 +++++++++\n t/t7607-merge-overwrite.sh    | 23 ++++++++++++++++\n unpack-trees.c                |  3 ++-\n 7 files changed, 149 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex ff641b348a..8aebb829a6 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -303,13 +303,32 @@ test_expect_success 'fail if the index has unresolved entries' '\n \tgit checkout -f \"$c1\" &&\n \n \ttest_must_fail git merge \"$c5\" &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"not possible because you have unmerged files\" out &&\n \tgit add -u &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"You have not concluded your merge\" out &&\n \trm -f .git/MERGE_HEAD &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n+'\n+\n+test_expect_success 'fail if a deleted intent-to-add file exists in the index' '\n+\tgit checkout -f \"$c1\" &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n+\tgit checkout -f \"$c1\" &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n '\n \ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex d1c68af8c5..45d816fc0c 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -91,16 +91,63 @@ test_expect_success 'cherry-pick on stat-dirty working tree' '\n \t)\n '\n \n-test_expect_success 'revert forbidden on dirty working tree' '\n+test_expect_success 'cherry-pick forbidden on dirty working tree' '\n \n+\tgit checkout -b temp &&\n \techo content >extra_file &&\n \tgit add extra_file &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"your local changes would be overwritten by \" errors\n+\n+'\n+\n+test_expect_success 'revert forbidden on dirty working tree' '\n+\n \ttest_must_fail git revert HEAD 2>errors &&\n \ttest_i18ngrep \"your local changes would be overwritten by \" errors\n \n '\n \n+test_expect_success 'cherry-pick fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors\n+'\n+\n+test_expect_success 'revert fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors\n+'\n+\n test_expect_success 'cherry-pick on unborn branch' '\n+\tgit checkout -f rename1 &&\n \tgit checkout --orphan unborn &&\n \tgit rm --cached -r . &&\n \trm -rf * &&\ndiff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh\nindex 16faa07813..96a0b779e7 100755\n--- a/t/t7104-reset-hard.sh\n+++ b/t/t7104-reset-hard.sh\n@@ -43,4 +43,15 @@ test_expect_success 'reset --hard did not corrupt index or cached-tree' '\n \n '\n \n+test_expect_success 'reset --hard adds deleted intent-to-add file back to index' '\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit reset --hard &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\nindex a82a07a04a..3346759375 100755\n--- a/t/t7110-reset-merge.sh\n+++ b/t/t7110-reset-merge.sh\n@@ -292,4 +292,35 @@ test_expect_success '--keep fails with added/deleted merge' '\n     test_i18ngrep \"middle of a merge\" err.log\n '\n \n+test_expect_success 'reset --merge adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    git add nonempty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached nonempty &&\n+    git add -N nonempty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep nonempty err.log | grep \"not uptodate\" &&\n+    git reset --hard initial &&\n+    >empty &&\n+    git add empty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached empty &&\n+    git add -N empty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep empty err.log | grep \"not uptodate\"\n+'\n+\n+test_expect_success 'reset --keep adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    >empty &&\n+    git add nonempty empty &&\n+    git commit -m \"create files to be deleted\" &&\n+    git rm --cached nonempty empty &&\n+    git add -N nonempty empty &&\n+    git reset --keep HEAD &&\n+    git diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 5990299fc9..4c0c33ce33 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -674,4 +674,16 @@ test_expect_success 'custom merge driver with checkout -m' '\n \ttest_cmp expect arm\n '\n \n+test_expect_success 'checkout -f HEAD adds deleted intent-to-add file back to index' '\n+\tgit reset --hard master &&\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit checkout -f HEAD &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex dd8ab7ede1..7e6b7b9a07 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -20,6 +20,16 @@ test_expect_success 'setup' '\n \tgit add sub/f sub2/f &&\n \tgit commit -m sub &&\n \tgit tag sub &&\n+\tgit reset --hard c0 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m nonempty &&\n+\tgit tag nonempty &&\n+\tgit reset --hard c0 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m empty &&\n+\tgit tag empty &&\n \techo \"VERY IMPORTANT CHANGES\" > important\n '\n \n@@ -192,4 +202,17 @@ test_expect_success 'will not clobber WT/index when merging into unborn' '\n \tgrep bar untracked-file\n '\n \n+test_expect_success 'will not overwrite ita file on fast-forward merge' '\n+\tgit reset --hard c0 &&\n+\t>nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git merge --ff-only nonempty 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n+\tgit reset --hard c0 &&\n+\t>empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git merge --ff-only empty 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n+'\n+\n test_done\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 50189909b8..5e6d88f36b 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1658,9 +1658,10 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n \t\treturn 0;\n \tif (!a && !b)\n \t\treturn 1;\n-\tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n+\tif ((a->ce_flags | b->ce_flags) & (CE_CONFLICTED | CE_INTENT_TO_ADD))\n \t\treturn 0;\n \treturn a->ce_mode == b->ce_mode &&\n+\t       !ce_intent_to_add(a) == !ce_intent_to_add(b) &&\n \t       oideq(&a->oid, &b->oid);\n }\n \n-- \n2.22.0\n\n"},{"id":"380479","messageId":"xmqqtvai4cc4.fsf@gitster-ct.c.googlers.com","threadId":"51638","inReplyTo":"CAK_rgsGSfL2zQLP59O+PUCLGz0X8c4UQ3Zg=9M+nCww5C+X_gA@mail.gmail.com","subject":"Re: [RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-15T16:34:19Z","receivedAt":"2019-08-15T16:34:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Varun Naik <vcnaik94@gmail.com> writes:\n\n> On Tue, Aug 13, 2019 at 1:33 PM Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> The original code considered that two entries with the same mode and\n>> the same \"contents\" are the same.  As nobody sane tracks an empty\n>> file for an extended span of history, that meant that most of the\n>> time, intent-to-add entries, which has the normal mode bits for the\n>> blobs (with or without the executable bit) and object name for a\n>> zero length blob, would have been judged \"different\".\n>>\n>\n> I agree, this edge case is really arcane. The rabbit hole was deep :)\n\nIn retrospect, perhaps I shouldn't have used the empty-blob SHA-1\nfor I-T-A entries (instead, perhaps 0{40} or something impossible)\nwhen I invented this feature at 39425819 (\"git-add --intent-to-add\n(-N)\", 2008-08-21).  It made the code to \"diff\" easier than having\nto special case comparison between a real blob and an I-T-A entry,\nbut we are paying the price for that laziness with a discussion and\na patch like this one.\n\n>> So perhaps\n>>\n>> +       !ce_intent_to_add(a) && !ce_intent_to_add(b) &&\n>>\n>> i.e. \"a cache entry is eligible to be same with something else only\n>> when its I-T-A bit is unset\".\n>>\n>\n> I decided to follow René's suggestion in response to this. Patch coming soon.\n\nEither is fine as the implementation of the same semantics; I\nhowever am not sure if two I-T-A entries should compare \"same\" or\n\"not same\", or if we are better off catching the caller that feeds\ntwo I-T-A entries to same() with a BUG().\n\nThanks.\n"},{"id":"380481","messageId":"a19fd436-b5f3-462f-6f9b-f8eeb46c78da@web.de","threadId":"51638","inReplyTo":"20190815162115.65008-1-vcnaik94@gmail.com","subject":"Re: [PATCH v2] unpack-trees.c: distinguish ita files from empty files","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-15T16:46:29Z","receivedAt":"2019-08-15T16:46:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.08.19 um 18:21 schrieb Varun Naik:\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 50189909b8..5e6d88f36b 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -1658,9 +1658,10 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n>  \t\treturn 0;\n>  \tif (!a && !b)\n>  \t\treturn 1;\n> -\tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n> +\tif ((a->ce_flags | b->ce_flags) & (CE_CONFLICTED | CE_INTENT_TO_ADD))\n>  \t\treturn 0;\n\nIf any of the two entries has the intent-to-add flag set, they won't be\nconsidered the same and we exit early.\n\n>  \treturn a->ce_mode == b->ce_mode &&\n> +\t       !ce_intent_to_add(a) == !ce_intent_to_add(b) &&\n\nSo if we reach this point, the flag can't be set, ce_intent_to_add()\nreturns 0 on both sides and thus the added condition is always true.\nYou might as well remove it.\n\n>  \t       oideq(&a->oid, &b->oid);\n>  }\n>\n>\n"},{"id":"380710","messageId":"CAK_rgsHKaLkO_wputOKcHBTDTnyvOUNTABUKK3fKc3ddiaNzQQ@mail.gmail.com","threadId":"51638","inReplyTo":"xmqqtvai4cc4.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"Varun Naik","fromEmail":"vcnaik94@gmail.com","sentAt":"2019-08-19T15:35:58Z","receivedAt":"2019-08-19T15:36:12Z","isPatch":true,"sender":{"key":"vcnaik94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2382501?v=4"},"body":"On Thu, Aug 15, 2019 at 9:34 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Varun Naik <vcnaik94@gmail.com> writes:\n>\n> > On Tue, Aug 13, 2019 at 1:33 PM Junio C Hamano <gitster@pobox.com> wrote:\n> >>\n> >> So perhaps\n> >>\n> >> +       !ce_intent_to_add(a) && !ce_intent_to_add(b) &&\n> >>\n> >> i.e. \"a cache entry is eligible to be same with something else only\n> >> when its I-T-A bit is unset\".\n> >>\n> >\n> > I decided to follow René's suggestion in response to this. Patch coming soon.\n>\n> Either is fine as the implementation of the same semantics; I\n> however am not sure if two I-T-A entries should compare \"same\" or\n> \"not same\", or if we are better off catching the caller that feeds\n> two I-T-A entries to same() with a BUG().\n\nI'd argue that two ita cache entries should be a BUG. Since we believe\nthat a cache entry in the tree can never have the intent-to-add bit set,\nit suffices to show that no call to same() ever passes two cache entries\nfrom the index.\n\nThe call in unpack-trees.c:merged_entry() compares the \"old\" entry\n(which comes from the index in all cases) to a newly created \"merge\"\nentry (which is a duplicate of an entry from a tree in all cases). All\nother calls compare either entries from two trees, or an entry from a\ntree and an entry from the index. I also can't think of any case where\nsomeone would want to check if two index entries are the \"same\" in the\nfuture.\n\nThe same argument probably extends to the conflicted bit, but changing\nthat is probably out of scope of this patch.\n\nVarun\n"},{"id":"380725","messageId":"xmqq5zmtx75x.fsf@gitster-ct.c.googlers.com","threadId":"51638","inReplyTo":"CAK_rgsHKaLkO_wputOKcHBTDTnyvOUNTABUKK3fKc3ddiaNzQQ@mail.gmail.com","subject":"Re: [RFC PATCH] unpack-trees.c: handle empty deleted ita files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-19T19:54:34Z","receivedAt":"2019-08-19T19:54:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Varun Naik <vcnaik94@gmail.com> writes:\n\n>> Either is fine as the implementation of the same semantics; I\n>> however am not sure if two I-T-A entries should compare \"same\" or\n>> \"not same\", or if we are better off catching the caller that feeds\n>> two I-T-A entries to same() with a BUG().\n>\n> I'd argue that two ita cache entries should be a BUG. Since we believe\n> that a cache entry in the tree can never have the intent-to-add bit set,\n> it suffices to show that no call to same() ever passes two cache entries\n> from the index.\n> ...\n> The same argument probably extends to the conflicted bit, but changing\n> that is probably out of scope of this patch.\n\nYup.  I think the patch as-posted is fine.  I also agree that\ntightening the validity check of parameters to same() is better done\nas a separate topic.\n"},{"id":"380874","messageId":"20190821032100.73917-1-vcnaik94@gmail.com","threadId":"51638","inReplyTo":"20190813160353.50018-1-vcnaik94@gmail.com","subject":"[PATCH v3] unpack-trees.c: distinguish ita files from empty files","fromName":"Varun Naik","fromEmail":"vcnaik94@gmail.com","sentAt":"2019-08-21T03:21:00Z","receivedAt":"2019-08-21T03:21:29Z","isPatch":true,"sender":{"key":"vcnaik94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2382501?v=4"},"body":"It is possible to delete a committed file from the index and then add it\nas intent-to-add. Several variations of \"reset\" and \"checkout\" should\nresurrect the file in the index from HEAD. \"merge\" (non-fast-forward),\n\"cherry-pick\", and \"revert\" should all fail with an error message. This\npatch provides the desired behavior even when the file is empty in HEAD.\n\nFurthermore, \"merge --ff-only\" should prevent a fast-forward merge if a\nfile is marked as intent-to-add. This patch provides the desired\nbehavior even when the file exists with empty contents in the final\ncommit.\n\nThe affected commands all compare two cache entries by calling\nunpack-trees.c:same(). Since a cache entry for an ita file and a cache\nentry for an empty file have the same oid, if the two files had the same\nmode, then the cache entries were previously considered the \"same\". This\nfix checks the intent-to-add bits so if one or both of the cache entries\nare marked as ita, then the two cache entries are not considered the\n\"same\".\n\nHelped-by: René Scharfe <l.s.r@web.de>\nSigned-off-by: Varun Naik <vcnaik94@gmail.com>\n---\nI rewrote the code in same(), as René suggested. Somehow the extra line\nslipped through into v2.\n\nAs Junio suggested, I'm ignoring the validity check of the CE_CONFLICTED\nand CE_INTENT_TO_ADD bits in this patch.\n\n t/t3030-merge-recursive.sh    | 25 +++++++++++++++---\n t/t3501-revert-cherry-pick.sh | 49 ++++++++++++++++++++++++++++++++++-\n t/t7104-reset-hard.sh         | 11 ++++++++\n t/t7110-reset-merge.sh        | 31 ++++++++++++++++++++++\n t/t7201-co.sh                 | 12 +++++++++\n t/t7607-merge-overwrite.sh    | 23 ++++++++++++++++\n unpack-trees.c                |  2 +-\n 7 files changed, 148 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex ff641b348a..8aebb829a6 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -303,13 +303,32 @@ test_expect_success 'fail if the index has unresolved entries' '\n \tgit checkout -f \"$c1\" &&\n \n \ttest_must_fail git merge \"$c5\" &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"not possible because you have unmerged files\" out &&\n \tgit add -u &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"You have not concluded your merge\" out &&\n \trm -f .git/MERGE_HEAD &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n+'\n+\n+test_expect_success 'fail if a deleted intent-to-add file exists in the index' '\n+\tgit checkout -f \"$c1\" &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n+\tgit checkout -f \"$c1\" &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n '\n \ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex d1c68af8c5..45d816fc0c 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -91,16 +91,63 @@ test_expect_success 'cherry-pick on stat-dirty working tree' '\n \t)\n '\n \n-test_expect_success 'revert forbidden on dirty working tree' '\n+test_expect_success 'cherry-pick forbidden on dirty working tree' '\n \n+\tgit checkout -b temp &&\n \techo content >extra_file &&\n \tgit add extra_file &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"your local changes would be overwritten by \" errors\n+\n+'\n+\n+test_expect_success 'revert forbidden on dirty working tree' '\n+\n \ttest_must_fail git revert HEAD 2>errors &&\n \ttest_i18ngrep \"your local changes would be overwritten by \" errors\n \n '\n \n+test_expect_success 'cherry-pick fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors\n+'\n+\n+test_expect_success 'revert fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"overwritten\" errors\n+'\n+\n test_expect_success 'cherry-pick on unborn branch' '\n+\tgit checkout -f rename1 &&\n \tgit checkout --orphan unborn &&\n \tgit rm --cached -r . &&\n \trm -rf * &&\ndiff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh\nindex 16faa07813..96a0b779e7 100755\n--- a/t/t7104-reset-hard.sh\n+++ b/t/t7104-reset-hard.sh\n@@ -43,4 +43,15 @@ test_expect_success 'reset --hard did not corrupt index or cached-tree' '\n \n '\n \n+test_expect_success 'reset --hard adds deleted intent-to-add file back to index' '\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit reset --hard &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\nindex a82a07a04a..3346759375 100755\n--- a/t/t7110-reset-merge.sh\n+++ b/t/t7110-reset-merge.sh\n@@ -292,4 +292,35 @@ test_expect_success '--keep fails with added/deleted merge' '\n     test_i18ngrep \"middle of a merge\" err.log\n '\n \n+test_expect_success 'reset --merge adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    git add nonempty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached nonempty &&\n+    git add -N nonempty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep nonempty err.log | grep \"not uptodate\" &&\n+    git reset --hard initial &&\n+    >empty &&\n+    git add empty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached empty &&\n+    git add -N empty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep empty err.log | grep \"not uptodate\"\n+'\n+\n+test_expect_success 'reset --keep adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    >empty &&\n+    git add nonempty empty &&\n+    git commit -m \"create files to be deleted\" &&\n+    git rm --cached nonempty empty &&\n+    git add -N nonempty empty &&\n+    git reset --keep HEAD &&\n+    git diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 5990299fc9..4c0c33ce33 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -674,4 +674,16 @@ test_expect_success 'custom merge driver with checkout -m' '\n \ttest_cmp expect arm\n '\n \n+test_expect_success 'checkout -f HEAD adds deleted intent-to-add file back to index' '\n+\tgit reset --hard master &&\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit checkout -f HEAD &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex dd8ab7ede1..7e6b7b9a07 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -20,6 +20,16 @@ test_expect_success 'setup' '\n \tgit add sub/f sub2/f &&\n \tgit commit -m sub &&\n \tgit tag sub &&\n+\tgit reset --hard c0 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m nonempty &&\n+\tgit tag nonempty &&\n+\tgit reset --hard c0 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m empty &&\n+\tgit tag empty &&\n \techo \"VERY IMPORTANT CHANGES\" > important\n '\n \n@@ -192,4 +202,17 @@ test_expect_success 'will not clobber WT/index when merging into unborn' '\n \tgrep bar untracked-file\n '\n \n+test_expect_success 'will not overwrite ita file on fast-forward merge' '\n+\tgit reset --hard c0 &&\n+\t>nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git merge --ff-only nonempty 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n+\tgit reset --hard c0 &&\n+\t>empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git merge --ff-only empty 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n+'\n+\n test_done\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 50189909b8..8d974667f2 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1658,7 +1658,7 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n \t\treturn 0;\n \tif (!a && !b)\n \t\treturn 1;\n-\tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n+\tif ((a->ce_flags | b->ce_flags) & (CE_CONFLICTED | CE_INTENT_TO_ADD))\n \t\treturn 0;\n \treturn a->ce_mode == b->ce_mode &&\n \t       oideq(&a->oid, &b->oid);\n-- \n2.23.0\n\n"},{"id":"391771","messageId":"20200214171459.25754-1-vcnaik94@gmail.com","threadId":"51638","inReplyTo":"20190821032100.73917-1-vcnaik94@gmail.com","subject":"[PATCH v4] unpack-trees.c: distinguish ita files from empty files","fromName":"Varun Naik","fromEmail":"vcnaik94@gmail.com","sentAt":"2020-02-14T17:14:59Z","receivedAt":"2020-02-14T17:15:32Z","isPatch":true,"sender":{"key":"vcnaik94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2382501?v=4"},"body":"It is possible to delete a committed file from the index and then add it\nas intent-to-add. Several variations of \"reset\" and \"checkout\" should\nresurrect the file in the index from HEAD. \"merge\" (non-fast-forward),\n\"cherry-pick\", and \"revert\" should all fail with an error message. This\npatch provides the desired behavior even when the file is empty in HEAD.\n\nFurthermore, \"merge --ff-only\" should prevent a fast-forward merge if a\nfile is marked as intent-to-add. This patch provides the desired\nbehavior even when the file exists with empty contents in the final\ncommit.\n\nThe affected commands all compare two cache entries by calling\nunpack-trees.c:same(). Since a cache entry for an ita file and a cache\nentry for an empty file have the same oid, if the two files had the same\nmode, then the cache entries were previously considered the \"same\". This\nfix checks the intent-to-add bits so if one or both of the cache entries\nare marked as ita, then the two cache entries are not considered the\n\"same\".\n\nHelped-by: René Scharfe <l.s.r@web.de>\nSigned-off-by: Varun Naik <vcnaik94@gmail.com>\n---\nThis patch never made it into pu, but I'll try one more time. I decided to\ndiscard [0], so I tweaked the test cases in this patch.\n\nTo repeat, I'm ignoring the validity check of the CE_CONFLICTED and\nCE_INTENT_TO_ADD bits in this patch.\n\n[0]: https://public-inbox.org/git/20190801161558.12838-1-vcnaik94@gmail.com/\n\n t/t3030-merge-recursive.sh    | 25 +++++++++++++++---\n t/t3501-revert-cherry-pick.sh | 49 ++++++++++++++++++++++++++++++++++-\n t/t7104-reset-hard.sh         | 11 ++++++++\n t/t7110-reset-merge.sh        | 31 ++++++++++++++++++++++\n t/t7201-co.sh                 | 12 +++++++++\n t/t7607-merge-overwrite.sh    | 23 ++++++++++++++++\n unpack-trees.c                |  2 +-\n 7 files changed, 148 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 2170758e38..b07153fa14 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -303,13 +303,32 @@ test_expect_success 'fail if the index has unresolved entries' '\n \tgit checkout -f \"$c1\" &&\n \n \ttest_must_fail git merge \"$c5\" &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"not possible because you have unmerged files\" out &&\n \tgit add -u &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"You have not concluded your merge\" out &&\n \trm -f .git/MERGE_HEAD &&\n-\ttest_must_fail git merge \"$c5\" 2> out &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n+'\n+\n+test_expect_success 'fail if a deleted intent-to-add file exists in the index' '\n+\tgit checkout -f \"$c1\" &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n+\tgit checkout -f \"$c1\" &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git merge \"$c5\" 2>out &&\n \ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n '\n \ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 7c1da21df1..88c15466ea 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -91,16 +91,63 @@ test_expect_success 'cherry-pick on stat-dirty working tree' '\n \t)\n '\n \n-test_expect_success 'revert forbidden on dirty working tree' '\n+test_expect_success 'cherry-pick forbidden on dirty working tree' '\n \n+\tgit checkout -b temp &&\n \techo content >extra_file &&\n \tgit add extra_file &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"your local changes would be overwritten by \" errors\n+\n+'\n+\n+test_expect_success 'revert forbidden on dirty working tree' '\n+\n \ttest_must_fail git revert HEAD 2>errors &&\n \ttest_i18ngrep \"your local changes would be overwritten by \" errors\n \n '\n \n+test_expect_success 'cherry-pick fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"your local changes would be overwritten by \" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git cherry-pick rename2 2>errors &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" errors\n+'\n+\n+test_expect_success 'revert fails if a deleted intent-to-add file exists in the index' '\n+\tgit reset --hard rename1 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"your local changes would be overwritten by \" errors &&\n+\tgit reset --hard rename1 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m \"create file to be deleted\" &&\n+\tgit rm --cached empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git revert rename2 2>errors &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" errors\n+'\n+\n test_expect_success 'cherry-pick on unborn branch' '\n+\tgit checkout -f rename1 &&\n \tgit checkout --orphan unborn &&\n \tgit rm --cached -r . &&\n \trm -rf * &&\ndiff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh\nindex 16faa07813..96a0b779e7 100755\n--- a/t/t7104-reset-hard.sh\n+++ b/t/t7104-reset-hard.sh\n@@ -43,4 +43,15 @@ test_expect_success 'reset --hard did not corrupt index or cached-tree' '\n \n '\n \n+test_expect_success 'reset --hard adds deleted intent-to-add file back to index' '\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit reset --hard &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\nindex a82a07a04a..3346759375 100755\n--- a/t/t7110-reset-merge.sh\n+++ b/t/t7110-reset-merge.sh\n@@ -292,4 +292,35 @@ test_expect_success '--keep fails with added/deleted merge' '\n     test_i18ngrep \"middle of a merge\" err.log\n '\n \n+test_expect_success 'reset --merge adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    git add nonempty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached nonempty &&\n+    git add -N nonempty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep nonempty err.log | grep \"not uptodate\" &&\n+    git reset --hard initial &&\n+    >empty &&\n+    git add empty &&\n+    git commit -m \"create file to be deleted\" &&\n+    git rm --cached empty &&\n+    git add -N empty &&\n+    test_must_fail git reset --merge HEAD 2>err.log &&\n+    grep empty err.log | grep \"not uptodate\"\n+'\n+\n+test_expect_success 'reset --keep adds deleted intent-to-add file back to index' '\n+    git reset --hard initial &&\n+    echo \"nonempty\" >nonempty &&\n+    >empty &&\n+    git add nonempty empty &&\n+    git commit -m \"create files to be deleted\" &&\n+    git rm --cached nonempty empty &&\n+    git add -N nonempty empty &&\n+    git reset --keep HEAD &&\n+    git diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex b696bae5f5..f00c16d615 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -674,4 +674,16 @@ test_expect_success 'custom merge driver with checkout -m' '\n \ttest_cmp expect arm\n '\n \n+test_expect_success 'checkout -f HEAD adds deleted intent-to-add file back to index' '\n+\tgit reset --hard master &&\n+\techo \"nonempty\" >nonempty &&\n+\t>empty &&\n+\tgit add nonempty empty &&\n+\tgit commit -m \"create files to be deleted\" &&\n+\tgit rm --cached nonempty empty &&\n+\tgit add -N nonempty empty &&\n+\tgit checkout -f HEAD &&\n+\tgit diff --cached --exit-code nonempty empty\n+'\n+\n test_done\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex dd8ab7ede1..7e6b7b9a07 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -20,6 +20,16 @@ test_expect_success 'setup' '\n \tgit add sub/f sub2/f &&\n \tgit commit -m sub &&\n \tgit tag sub &&\n+\tgit reset --hard c0 &&\n+\techo \"nonempty\" >nonempty &&\n+\tgit add nonempty &&\n+\tgit commit -m nonempty &&\n+\tgit tag nonempty &&\n+\tgit reset --hard c0 &&\n+\t>empty &&\n+\tgit add empty &&\n+\tgit commit -m empty &&\n+\tgit tag empty &&\n \techo \"VERY IMPORTANT CHANGES\" > important\n '\n \n@@ -192,4 +202,17 @@ test_expect_success 'will not clobber WT/index when merging into unborn' '\n \tgrep bar untracked-file\n '\n \n+test_expect_success 'will not overwrite ita file on fast-forward merge' '\n+\tgit reset --hard c0 &&\n+\t>nonempty &&\n+\tgit add -N nonempty &&\n+\ttest_must_fail git merge --ff-only nonempty 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out &&\n+\tgit reset --hard c0 &&\n+\t>empty &&\n+\tgit add -N empty &&\n+\ttest_must_fail git merge --ff-only empty 2>out &&\n+\ttest_i18ngrep \"Your local changes to the following files would be overwritten by merge:\" out\n+'\n+\n test_done\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 2399b6818b..3e63d175f8 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1715,7 +1715,7 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)\n \t\treturn 0;\n \tif (!a && !b)\n \t\treturn 1;\n-\tif ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)\n+\tif ((a->ce_flags | b->ce_flags) & (CE_CONFLICTED | CE_INTENT_TO_ADD))\n \t\treturn 0;\n \treturn a->ce_mode == b->ce_mode &&\n \t       oideq(&a->oid, &b->oid);\n-- \n2.25.0\n\n"}]}