git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Varun Naik <vcnaik94@gmail.com>
To: vcnaik94@gmail.com
Cc: git@vger.kernel.org, gitster@pobox.com, pclouds@gmail.com,
	"René Scharfe" <l.s.r@web.de>
Subject: [PATCH v2] unpack-trees.c: distinguish ita files from empty files
Date: Thu, 15 Aug 2019 09:21:15 -0700	[thread overview]
Message-ID: <20190815162115.65008-1-vcnaik94@gmail.com> (raw)
In-Reply-To: <20190813160353.50018-1-vcnaik94@gmail.com>

It is possible to delete a committed file from the index and then add it
as intent-to-add. Several variations of "reset" and "checkout" should
resurrect the file in the index from HEAD. "merge" (non-fast-forward),
"cherry-pick", and "revert" should all fail with an error message. This
patch provides the desired behavior even when the file is empty in HEAD.

Furthermore, "merge --ff-only" should prevent a fast-forward merge if a
file is marked as intent-to-add. This patch provides the desired
behavior even when the file exists with empty contents in the final
commit.

The affected commands all compare two cache entries by calling
unpack-trees.c:same(). Since a cache entry for an ita file and a cache
entry for an empty file have the same oid, if the two files had the same
mode, then the cache entries were previously considered the "same". This
fix checks the intent-to-add bits so if one or both of the cache entries
are marked as ita, then the two cache entries are not considered the
"same".

Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Varun Naik <vcnaik94@gmail.com>
---
I rewrote the code in same(), as René suggested.

New in v2 is a test for "merge --ff-only", which calls
unpack-trees.c:twoway_merge(), which calls unpack-trees.c:same().
Previously, if the file existed with empty contents in the final commit,
then the fast-forward merge went through, but the file ended up as a
deleted ita file in the index. Now, the merge fails with an error.

 t/t3030-merge-recursive.sh    | 25 +++++++++++++++---
 t/t3501-revert-cherry-pick.sh | 49 ++++++++++++++++++++++++++++++++++-
 t/t7104-reset-hard.sh         | 11 ++++++++
 t/t7110-reset-merge.sh        | 31 ++++++++++++++++++++++
 t/t7201-co.sh                 | 12 +++++++++
 t/t7607-merge-overwrite.sh    | 23 ++++++++++++++++
 unpack-trees.c                |  3 ++-
 7 files changed, 149 insertions(+), 5 deletions(-)

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index ff641b348a..8aebb829a6 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -303,13 +303,32 @@ test_expect_success 'fail if the index has unresolved entries' '
 	git checkout -f "$c1" &&
 
 	test_must_fail git merge "$c5" &&
-	test_must_fail git merge "$c5" 2> out &&
+	test_must_fail git merge "$c5" 2>out &&
 	test_i18ngrep "not possible because you have unmerged files" out &&
 	git add -u &&
-	test_must_fail git merge "$c5" 2> out &&
+	test_must_fail git merge "$c5" 2>out &&
 	test_i18ngrep "You have not concluded your merge" out &&
 	rm -f .git/MERGE_HEAD &&
-	test_must_fail git merge "$c5" 2> out &&
+	test_must_fail git merge "$c5" 2>out &&
+	test_i18ngrep "Your local changes to the following files would be overwritten by merge:" out
+'
+
+test_expect_success 'fail if a deleted intent-to-add file exists in the index' '
+	git checkout -f "$c1" &&
+	echo "nonempty" >nonempty &&
+	git add nonempty &&
+	git commit -m "create file to be deleted" &&
+	git rm --cached nonempty &&
+	git add -N nonempty &&
+	test_must_fail git merge "$c5" 2>out &&
+	test_i18ngrep "Your local changes to the following files would be overwritten by merge:" out &&
+	git checkout -f "$c1" &&
+	>empty &&
+	git add empty &&
+	git commit -m "create file to be deleted" &&
+	git rm --cached empty &&
+	git add -N empty &&
+	test_must_fail git merge "$c5" 2>out &&
 	test_i18ngrep "Your local changes to the following files would be overwritten by merge:" out
 '
 
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index d1c68af8c5..45d816fc0c 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -91,16 +91,63 @@ test_expect_success 'cherry-pick on stat-dirty working tree' '
 	)
 '
 
-test_expect_success 'revert forbidden on dirty working tree' '
+test_expect_success 'cherry-pick forbidden on dirty working tree' '
 
+	git checkout -b temp &&
 	echo content >extra_file &&
 	git add extra_file &&
+	test_must_fail git cherry-pick rename2 2>errors &&
+	test_i18ngrep "your local changes would be overwritten by " errors
+
+'
+
+test_expect_success 'revert forbidden on dirty working tree' '
+
 	test_must_fail git revert HEAD 2>errors &&
 	test_i18ngrep "your local changes would be overwritten by " errors
 
 '
 
+test_expect_success 'cherry-pick fails if a deleted intent-to-add file exists in the index' '
+	git reset --hard rename1 &&
+	echo "nonempty" >nonempty &&
+	git add nonempty &&
+	git commit -m "create file to be deleted" &&
+	git rm --cached nonempty &&
+	git add -N nonempty &&
+	test_must_fail git cherry-pick rename2 2>errors &&
+	test_i18ngrep "overwritten" errors &&
+	git reset --hard rename1 &&
+	>empty &&
+	git add empty &&
+	git commit -m "create file to be deleted" &&
+	git rm --cached empty &&
+	git add -N empty &&
+	test_must_fail git cherry-pick rename2 2>errors &&
+	test_i18ngrep "overwritten" errors
+'
+
+test_expect_success 'revert fails if a deleted intent-to-add file exists in the index' '
+	git reset --hard rename1 &&
+	echo "nonempty" >nonempty &&
+	git add nonempty &&
+	git commit -m "create file to be deleted" &&
+	git rm --cached nonempty &&
+	git add -N nonempty &&
+	test_must_fail git revert rename2 2>errors &&
+	test_i18ngrep "overwritten" errors &&
+	git reset --hard rename1 &&
+	>empty &&
+	git add empty &&
+	git commit -m "create file to be deleted" &&
+	git rm --cached empty &&
+	git add -N empty &&
+	test_must_fail git revert rename2 2>errors &&
+	test_i18ngrep "overwritten" errors
+'
+
 test_expect_success 'cherry-pick on unborn branch' '
+	git checkout -f rename1 &&
 	git checkout --orphan unborn &&
 	git rm --cached -r . &&
 	rm -rf * &&
diff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh
index 16faa07813..96a0b779e7 100755
--- a/t/t7104-reset-hard.sh
+++ b/t/t7104-reset-hard.sh
@@ -43,4 +43,15 @@ test_expect_success 'reset --hard did not corrupt index or cached-tree' '
 
 '
 
+test_expect_success 'reset --hard adds deleted intent-to-add file back to index' '
+	echo "nonempty" >nonempty &&
+	>empty &&
+	git add nonempty empty &&
+	git commit -m "create files to be deleted" &&
+	git rm --cached nonempty empty &&
+	git add -N nonempty empty &&
+	git reset --hard &&
+	git diff --cached --exit-code nonempty empty
+'
+
 test_done
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index a82a07a04a..3346759375 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -292,4 +292,35 @@ test_expect_success '--keep fails with added/deleted merge' '
     test_i18ngrep "middle of a merge" err.log
 '
 
+test_expect_success 'reset --merge adds deleted intent-to-add file back to index' '
+    git reset --hard initial &&
+    echo "nonempty" >nonempty &&
+    git add nonempty &&
+    git commit -m "create file to be deleted" &&
+    git rm --cached nonempty &&
+    git add -N nonempty &&
+    test_must_fail git reset --merge HEAD 2>err.log &&
+    grep nonempty err.log | grep "not uptodate" &&
+    git reset --hard initial &&
+    >empty &&
+    git add empty &&
+    git commit -m "create file to be deleted" &&
+    git rm --cached empty &&
+    git add -N empty &&
+    test_must_fail git reset --merge HEAD 2>err.log &&
+    grep empty err.log | grep "not uptodate"
+'
+
+test_expect_success 'reset --keep adds deleted intent-to-add file back to index' '
+    git reset --hard initial &&
+    echo "nonempty" >nonempty &&
+    >empty &&
+    git add nonempty empty &&
+    git commit -m "create files to be deleted" &&
+    git rm --cached nonempty empty &&
+    git add -N nonempty empty &&
+    git reset --keep HEAD &&
+    git diff --cached --exit-code nonempty empty
+'
+
 test_done
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5990299fc9..4c0c33ce33 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -674,4 +674,16 @@ test_expect_success 'custom merge driver with checkout -m' '
 	test_cmp expect arm
 '
 
+test_expect_success 'checkout -f HEAD adds deleted intent-to-add file back to index' '
+	git reset --hard master &&
+	echo "nonempty" >nonempty &&
+	>empty &&
+	git add nonempty empty &&
+	git commit -m "create files to be deleted" &&
+	git rm --cached nonempty empty &&
+	git add -N nonempty empty &&
+	git checkout -f HEAD &&
+	git diff --cached --exit-code nonempty empty
+'
+
 test_done
diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
index dd8ab7ede1..7e6b7b9a07 100755
--- a/t/t7607-merge-overwrite.sh
+++ b/t/t7607-merge-overwrite.sh
@@ -20,6 +20,16 @@ test_expect_success 'setup' '
 	git add sub/f sub2/f &&
 	git commit -m sub &&
 	git tag sub &&
+	git reset --hard c0 &&
+	echo "nonempty" >nonempty &&
+	git add nonempty &&
+	git commit -m nonempty &&
+	git tag nonempty &&
+	git reset --hard c0 &&
+	>empty &&
+	git add empty &&
+	git commit -m empty &&
+	git tag empty &&
 	echo "VERY IMPORTANT CHANGES" > important
 '
 
@@ -192,4 +202,17 @@ test_expect_success 'will not clobber WT/index when merging into unborn' '
 	grep bar untracked-file
 '
 
+test_expect_success 'will not overwrite ita file on fast-forward merge' '
+	git reset --hard c0 &&
+	>nonempty &&
+	git add -N nonempty &&
+	test_must_fail git merge --ff-only nonempty 2>out &&
+	test_i18ngrep "Your local changes to the following files would be overwritten by merge:" out &&
+	git reset --hard c0 &&
+	>empty &&
+	git add -N empty &&
+	test_must_fail git merge --ff-only empty 2>out &&
+	test_i18ngrep "Your local changes to the following files would be overwritten by merge:" out
+'
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 50189909b8..5e6d88f36b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1658,9 +1658,10 @@ static int same(const struct cache_entry *a, const struct cache_entry *b)
 		return 0;
 	if (!a && !b)
 		return 1;
-	if ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)
+	if ((a->ce_flags | b->ce_flags) & (CE_CONFLICTED | CE_INTENT_TO_ADD))
 		return 0;
 	return a->ce_mode == b->ce_mode &&
+	       !ce_intent_to_add(a) == !ce_intent_to_add(b) &&
 	       oideq(&a->oid, &b->oid);
 }
 
-- 
2.22.0


  parent reply	other threads:[~2019-08-15 16:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-13 16:03 [RFC PATCH] unpack-trees.c: handle empty deleted ita files Varun Naik
2019-08-13 18:08 ` René Scharfe
2019-08-13 20:32   ` Junio C Hamano
2019-08-14  4:30     ` René Scharfe
2019-08-15 16:17     ` Varun Naik
2019-08-15 16:34       ` Junio C Hamano
2019-08-19 15:35         ` Varun Naik
2019-08-19 19:54           ` Junio C Hamano
2019-08-15 16:21 ` Varun Naik [this message]
2019-08-15 16:46   ` [PATCH v2] unpack-trees.c: distinguish ita files from empty files René Scharfe
2019-08-21  3:21 ` [PATCH v3] " Varun Naik
2020-02-14 17:14   ` [PATCH v4] " Varun Naik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190815162115.65008-1-vcnaik94@gmail.com \
    --to=vcnaik94@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).