[PATCH] checkout.c: unstage empty deleted ita files

[PATCH] checkout.c: unstage empty deleted ita files

[Varun Naik]

It is possible to delete a committed file from the index and then add it
as intent-to-add. After `git checkout HEAD` or `git restore --staged`,
the file should be identical in the index and HEAD. This patch provides
the desired behavior even when the file is empty in the index.

Signed-off-by: Varun Naik <vcnaik94@gmail.com>
---
CC Jeff because you wrote the code that I am changing now.

checkout.c:update_some() discards the newly created cache entry when its
mode and oid match those of the old entry. Since an ita file has the
same oid as an empty file, an empty deleted ita file passes both of
these checks, and the new entry is discarded. In this case, the file
should be added to the cache instead.

This change should not affect newly added ita files. For those, inside
tree.c:read_tree_1(), tree_entry_interesting() returns
entry_not_interesting, so fn (which points to update_some()) is never
called.

To the best of my understanding, the only other command that makes
changes to the index differently for nonempty vs empty deleted ita files
is "reset", which I am fixing in [0]. I am separating the two changes
because this change affects "restore", which has not reached maint yet.

[0]: https://public-inbox.org/git/20190726044806.2216-1-vcnaik94@gmail.com/

 builtin/checkout.c        |  1 +
 t/t2022-checkout-paths.sh | 11 +++++++++++
 t/t2070-restore.sh        | 11 +++++++++++
 3 files changed, 23 insertions(+)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 91f8509f85..27daa09c3c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -126,6 +126,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
 	if (pos >= 0) {
 		struct cache_entry *old = active_cache[pos];
 		if (ce->ce_mode == old->ce_mode &&
+		    !ce_intent_to_add(old) &&
 		    oideq(&ce->oid, &old->oid)) {
 			old->ce_flags |= CE_UPDATE;
 			discard_cache_entry(ce);
diff --git a/t/t2022-checkout-paths.sh b/t/t2022-checkout-paths.sh
index fc3eb43b89..74add853fd 100755
--- a/t/t2022-checkout-paths.sh
+++ b/t/t2022-checkout-paths.sh
@@ -78,4 +78,15 @@ test_expect_success 'do not touch files that are already up-to-date' '
 	test_cmp expect actual
 '
 
+test_expect_success 'checkout HEAD 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 checkout HEAD nonempty empty &&
+	git diff --staged --exit-code
+'
+
 test_done
diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh
index 2650df1966..09b1543a5b 100755
--- a/t/t2070-restore.sh
+++ b/t/t2070-restore.sh
@@ -95,4 +95,15 @@ test_expect_success 'restore --ignore-unmerged ignores unmerged entries' '
 	)
 '
 
+test_expect_success 'restore --staged 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 restore --staged nonempty empty &&
+	git diff --staged --exit-code
+'
+
 test_done
-- 
2.22.0

Re: [PATCH] checkout.c: unstage empty deleted ita files

[Varun Naik]

Got the CC list wrong in the first email...

Varun


On Thu, Jul 25, 2019 at 9:57 PM Varun Naik <vcnaik94@gmail.com> wrote:
>
> It is possible to delete a committed file from the index and then add it
> as intent-to-add. After `git checkout HEAD` or `git restore --staged`,
> the file should be identical in the index and HEAD. This patch provides
> the desired behavior even when the file is empty in the index.
>
> Signed-off-by: Varun Naik <vcnaik94@gmail.com>
> ---
> CC Jeff because you wrote the code that I am changing now.
>
> checkout.c:update_some() discards the newly created cache entry when its
> mode and oid match those of the old entry. Since an ita file has the
> same oid as an empty file, an empty deleted ita file passes both of
> these checks, and the new entry is discarded. In this case, the file
> should be added to the cache instead.
>
> This change should not affect newly added ita files. For those, inside
> tree.c:read_tree_1(), tree_entry_interesting() returns
> entry_not_interesting, so fn (which points to update_some()) is never
> called.
>
> To the best of my understanding, the only other command that makes
> changes to the index differently for nonempty vs empty deleted ita files
> is "reset", which I am fixing in [0]. I am separating the two changes
> because this change affects "restore", which has not reached maint yet.
>
> [0]: https://public-inbox.org/git/20190726044806.2216-1-vcnaik94@gmail.com/
>
>  builtin/checkout.c        |  1 +
>  t/t2022-checkout-paths.sh | 11 +++++++++++
>  t/t2070-restore.sh        | 11 +++++++++++
>  3 files changed, 23 insertions(+)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 91f8509f85..27daa09c3c 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -126,6 +126,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
>         if (pos >= 0) {
>                 struct cache_entry *old = active_cache[pos];
>                 if (ce->ce_mode == old->ce_mode &&
> +                   !ce_intent_to_add(old) &&
>                     oideq(&ce->oid, &old->oid)) {
>                         old->ce_flags |= CE_UPDATE;
>                         discard_cache_entry(ce);
> diff --git a/t/t2022-checkout-paths.sh b/t/t2022-checkout-paths.sh
> index fc3eb43b89..74add853fd 100755
> --- a/t/t2022-checkout-paths.sh
> +++ b/t/t2022-checkout-paths.sh
> @@ -78,4 +78,15 @@ test_expect_success 'do not touch files that are already up-to-date' '
>         test_cmp expect actual
>  '
>
> +test_expect_success 'checkout HEAD 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 checkout HEAD nonempty empty &&
> +       git diff --staged --exit-code
> +'
> +
>  test_done
> diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh
> index 2650df1966..09b1543a5b 100755
> --- a/t/t2070-restore.sh
> +++ b/t/t2070-restore.sh
> @@ -95,4 +95,15 @@ test_expect_success 'restore --ignore-unmerged ignores unmerged entries' '
>         )
>  '
>
> +test_expect_success 'restore --staged 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 restore --staged nonempty empty &&
> +       git diff --staged --exit-code
> +'
> +
>  test_done
> --
> 2.22.0
>

Re: [PATCH] checkout.c: unstage empty deleted ita files

[Jeff King]

On Thu, Jul 25, 2019 at 09:56:45PM -0700, Varun Naik wrote:

> It is possible to delete a committed file from the index and then add it
> as intent-to-add. After `git checkout HEAD` or `git restore --staged`,
> the file should be identical in the index and HEAD. This patch provides
> the desired behavior even when the file is empty in the index.

OK, so the issue is that ITA entries have an empty-file sha1, so they
confuse the logic to decide if we can use the old entry. Your fix makes
sense.

> ---
> CC Jeff because you wrote the code that I am changing now.
> 
> checkout.c:update_some() discards the newly created cache entry when its
> mode and oid match those of the old entry. Since an ita file has the
> same oid as an empty file, an empty deleted ita file passes both of
> these checks, and the new entry is discarded. In this case, the file
> should be added to the cache instead.
> 
> This change should not affect newly added ita files. For those, inside
> tree.c:read_tree_1(), tree_entry_interesting() returns
> entry_not_interesting, so fn (which points to update_some()) is never
> called.

These two paragraphs would be a nice addition to the actual commit
message.

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 91f8509f85..27daa09c3c 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -126,6 +126,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
>  	if (pos >= 0) {
>  		struct cache_entry *old = active_cache[pos];
>  		if (ce->ce_mode == old->ce_mode &&
> +		    !ce_intent_to_add(old) &&
>  		    oideq(&ce->oid, &old->oid)) {
>  			old->ce_flags |= CE_UPDATE;
>  			discard_cache_entry(ce);

My first thought here was that we could skip ITA entries here only when
the HEAD hash is also the empty blob, which would let us retain index
results in more cases. But it doesn't help. If the HEAD entry isn't the
empty blob, then we'll have !oideq() and we'll skip anyway, because an
ITA entry must be the empty blob (if we `git add` some other content,
then it ceases to be ITA).

So it makes sense to just always skip this "retain the old index entry"
block for any ITA entry.

> +test_expect_success 'checkout HEAD 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 checkout HEAD nonempty empty &&
> +	git diff --staged --exit-code
> +'

This clearly demonstrates the problem. Nice.

> +test_expect_success 'restore --staged 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 restore --staged nonempty empty &&
> +	git diff --staged --exit-code
> +'

Hmm. This git-restore test means we don't apply to maint. But wouldn't
we want the fix for "checkout" there?

I.e., I'd expect a patch to fix and test git-checkout, and then an
additional patch to be added on the merge of that plus master to test
git-restore.

Other than that, the patch looks good to me.

-Peff

Re: [PATCH] checkout.c: unstage empty deleted ita files

[Varun Naik]

On Fri, Jul 26, 2019 at 1:57 AM Jeff King <peff@peff.net> wrote:
>
> On Thu, Jul 25, 2019 at 09:56:45PM -0700, Varun Naik wrote:
>
> > It is possible to delete a committed file from the index and then add it
> > as intent-to-add. After `git checkout HEAD` or `git restore --staged`,
> > the file should be identical in the index and HEAD. This patch provides
> > the desired behavior even when the file is empty in the index.
>
> OK, so the issue is that ITA entries have an empty-file sha1, so they
> confuse the logic to decide if we can use the old entry. Your fix makes
> sense.
>
> > ---
> > CC Jeff because you wrote the code that I am changing now.
> >
> > checkout.c:update_some() discards the newly created cache entry when its
> > mode and oid match those of the old entry. Since an ita file has the
> > same oid as an empty file, an empty deleted ita file passes both of
> > these checks, and the new entry is discarded. In this case, the file
> > should be added to the cache instead.
> >
> > This change should not affect newly added ita files. For those, inside
> > tree.c:read_tree_1(), tree_entry_interesting() returns
> > entry_not_interesting, so fn (which points to update_some()) is never
> > called.
>
> These two paragraphs would be a nice addition to the actual commit
> message.
>

I will add them to the commit message, with some minor changes.

> > diff --git a/builtin/checkout.c b/builtin/checkout.c
> > index 91f8509f85..27daa09c3c 100644
> > --- a/builtin/checkout.c
> > +++ b/builtin/checkout.c
> > @@ -126,6 +126,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
> >       if (pos >= 0) {
> >               struct cache_entry *old = active_cache[pos];
> >               if (ce->ce_mode == old->ce_mode &&
> > +                 !ce_intent_to_add(old) &&
> >                   oideq(&ce->oid, &old->oid)) {
> >                       old->ce_flags |= CE_UPDATE;
> >                       discard_cache_entry(ce);
>
> My first thought here was that we could skip ITA entries here only when
> the HEAD hash is also the empty blob, which would let us retain index
> results in more cases. But it doesn't help. If the HEAD entry isn't the
> empty blob, then we'll have !oideq() and we'll skip anyway, because an
> ITA entry must be the empty blob (if we `git add` some other content,
> then it ceases to be ITA).
>
> So it makes sense to just always skip this "retain the old index entry"
> block for any ITA entry.
>
> > +test_expect_success 'checkout HEAD 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 checkout HEAD nonempty empty &&
> > +     git diff --staged --exit-code
> > +'
>
> This clearly demonstrates the problem. Nice.
>
> > +test_expect_success 'restore --staged 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 restore --staged nonempty empty &&
> > +     git diff --staged --exit-code
> > +'
>
> Hmm. This git-restore test means we don't apply to maint. But wouldn't
> we want the fix for "checkout" there?
>
> I.e., I'd expect a patch to fix and test git-checkout, and then an
> additional patch to be added on the merge of that plus master to test
> git-restore.
>

To make sure I understand, do you mean that I should omit the test
case for "restore" right now, wait for the patch to reach master, and
then create another patch for the "restore" test case?

> Other than that, the patch looks good to me.
>
> -Peff

Varun

Re: [PATCH] checkout.c: unstage empty deleted ita files

[Jeff King]

On Sun, Jul 28, 2019 at 11:54:38PM -0700, Varun Naik wrote:

> > These two paragraphs would be a nice addition to the actual commit
> > message.
> 
> I will add them to the commit message, with some minor changes.

Thanks!

> > Hmm. This git-restore test means we don't apply to maint. But wouldn't
> > we want the fix for "checkout" there?
> >
> > I.e., I'd expect a patch to fix and test git-checkout, and then an
> > additional patch to be added on the merge of that plus master to test
> > git-restore.
> >
> 
> To make sure I understand, do you mean that I should omit the test
> case for "restore" right now, wait for the patch to reach master, and
> then create another patch for the "restore" test case?

I think we could do it all right now. If you split it into two patches,
one for "fix checkout" and another for "add restore tests", then Junio
could do something like:

  git checkout -b vn/checkout-ita maint
  git am fix-checkout.patch

  git checkout -b vn/restore-ita-tests master
  git merge vn/checkout-ita
  git am restore-tests.patch

The justification for the commit message in the latter patch is
something like "this is fixed already, but let's add a regression test
to make sure it remains fixed".

-Peff

[PATCH v2] checkout.c: unstage empty deleted ita files

[Varun Naik]

It is possible to delete a committed file from the index and then add it
as intent-to-add. After `git checkout HEAD`, the file should be
identical in the index and HEAD. The command already works correctly if
the file has contents in HEAD. This patch provides the desired behavior
even when the file is empty in HEAD.

`git checkout HEAD` calls tree.c:read_tree_1(), with fn pointing to
checkout.c:update_some(). update_some() creates a new cache entry but
discards it when its mode and oid match those of the old entry. A cache
entry for an ita file and a cache entry for an empty file have the same
oid. Therefore, an empty deleted ita file previously passed both of
these checks, and the new entry was discarded, so the file remained
unchanged in the index. After this fix, if the file is marked as ita in
the cache, then we avoid discarding the new entry and add the new entry
to the cache instead.

This change should not affect newly added ita files. For those, inside
tree.c:read_tree_1(), tree_entry_interesting() returns
entry_not_interesting, so fn is never called.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Varun Naik <vcnaik94@gmail.com>
---
This patch fixes and tests only "checkout", because "restore" has not
reached maint yet. A second patch on the merge of this patch into master
with a test case for "restore" is coming.

 builtin/checkout.c        |  1 +
 t/t2022-checkout-paths.sh | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ffa776c6e1..c41dfa53a9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -105,6 +105,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
 	if (pos >= 0) {
 		struct cache_entry *old = active_cache[pos];
 		if (ce->ce_mode == old->ce_mode &&
+		    !ce_intent_to_add(old) &&
 		    oideq(&ce->oid, &old->oid)) {
 			old->ce_flags |= CE_UPDATE;
 			discard_cache_entry(ce);
diff --git a/t/t2022-checkout-paths.sh b/t/t2022-checkout-paths.sh
index fc3eb43b89..6844afafc0 100755
--- a/t/t2022-checkout-paths.sh
+++ b/t/t2022-checkout-paths.sh
@@ -78,4 +78,15 @@ test_expect_success 'do not touch files that are already up-to-date' '
 	test_cmp expect actual
 '
 
+test_expect_success 'checkout HEAD 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 checkout HEAD nonempty empty &&
+	git diff --cached --exit-code
+'
+
 test_done
-- 
2.22.0

[PATCH] restore: add test for deleted ita files

[Varun Naik]

`git restore --staged` uses the same machinery as `git checkout HEAD`,
so there should be a similar test case for "restore" as the existing
test case for "checkout" with deleted ita files.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Varun Naik <vcnaik94@gmail.com>
---
This is the "restore" patch based on the merge of the "checkout" patch
into master.

 t/t2070-restore.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh
index 2650df1966..21c3f84459 100755
--- a/t/t2070-restore.sh
+++ b/t/t2070-restore.sh
@@ -95,4 +95,15 @@ test_expect_success 'restore --ignore-unmerged ignores unmerged entries' '
 	)
 '
 
+test_expect_success 'restore --staged 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 restore --staged nonempty empty &&
+	git diff --cached --exit-code
+'
+
 test_done
-- 
2.22.0

Re: [PATCH v2] checkout.c: unstage empty deleted ita files

[Junio C Hamano]

Varun Naik <vcnaik94@gmail.com> writes:

> It is possible to delete a committed file from the index and then add it
> as intent-to-add. After `git checkout HEAD`, the file should be
> identical in the index and HEAD.

We should write this as `git checkout HEAD <pathspec>`; with the
command without the <pathspec> form, the files changed in various
ways should not be changed with it at all.

	$ echo modified >>file1
	$ rm file2
	$ git rm --cached file3 && git add -N file3
	$ git checkout HEAD
	M	file1
	D	file2
	M	file3

> `git checkout HEAD` calls tree.c:read_tree_1(), with fn pointing to
> checkout.c:update_some(). update_some() creates a new cache entry but
> discards it when its mode and oid match those of the old entry. A cache
> entry for an ita file and a cache entry for an empty file have the same
> oid. Therefore, an empty deleted ita file previously passed both of
> these checks, and the new entry was discarded, so the file remained
> unchanged in the index. After this fix, if the file is marked as ita in
> the cache, then we avoid discarding the new entry and add the new entry
> to the cache instead.

Thanks; the flow of thought above is quite straight-forward to
follow.

Re: [PATCH v2] checkout.c: unstage empty deleted ita files

[Jeff King]

On Thu, Aug 01, 2019 at 09:07:56AM -0700, Varun Naik wrote:

> It is possible to delete a committed file from the index and then add it
> as intent-to-add. After `git checkout HEAD`, the file should be
> identical in the index and HEAD. The command already works correctly if
> the file has contents in HEAD. This patch provides the desired behavior
> even when the file is empty in HEAD.
> 
> `git checkout HEAD` calls tree.c:read_tree_1(), with fn pointing to
> checkout.c:update_some(). update_some() creates a new cache entry but
> discards it when its mode and oid match those of the old entry. A cache
> entry for an ita file and a cache entry for an empty file have the same
> oid. Therefore, an empty deleted ita file previously passed both of
> these checks, and the new entry was discarded, so the file remained
> unchanged in the index. After this fix, if the file is marked as ita in
> the cache, then we avoid discarding the new entry and add the new entry
> to the cache instead.
> 
> This change should not affect newly added ita files. For those, inside
> tree.c:read_tree_1(), tree_entry_interesting() returns
> entry_not_interesting, so fn is never called.
> 
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Varun Naik <vcnaik94@gmail.com>
> ---
> This patch fixes and tests only "checkout", because "restore" has not
> reached maint yet. A second patch on the merge of this patch into master
> with a test case for "restore" is coming.

Thanks, this version looks good to me!

-Peff

Re: [PATCH] restore: add test for deleted ita files

[Junio C Hamano]

Varun Naik <vcnaik94@gmail.com> writes:

> `git restore --staged` uses the same machinery as `git checkout HEAD`,
> so there should be a similar test case for "restore" as the existing
> test case for "checkout" with deleted ita files.
>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Varun Naik <vcnaik94@gmail.com>
> ---
> This is the "restore" patch based on the merge of the "checkout" patch
> into master.

Makes sense.  Thanks.

[PATCH v3] checkout.c: unstage empty deleted ita files

[Varun Naik]

It is possible to delete a committed file from the index and then add it
as intent-to-add. After `git checkout HEAD <pathspec>`, the file should
be identical in the index and HEAD. The command already works correctly
if the file has contents in HEAD. This patch provides the desired
behavior even when the file is empty in HEAD.

`git checkout HEAD <pathspec>` calls tree.c:read_tree_1(), with fn
pointing to checkout.c:update_some(). update_some() creates a new cache
entry but discards it when its mode and oid match those of the old
entry. A cache entry for an ita file and a cache entry for an empty file
have the same oid. Therefore, an empty deleted ita file previously
passed both of these checks, and the new entry was discarded, so the
file remained unchanged in the index. After this fix, if the file is
marked as ita in the cache, then we avoid discarding the new entry and
add the new entry to the cache instead.

This change should not affect newly added ita files. For those, inside
tree.c:read_tree_1(), tree_entry_interesting() returns
entry_not_interesting, so fn is never called.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Varun Naik <vcnaik94@gmail.com>
---
This patch fixes and tests "checkout". New in v3 is an improved commit
message that clarifies that the affected command is `git checkout HEAD
<pathspec>`.

 builtin/checkout.c        |  1 +
 t/t2022-checkout-paths.sh | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ffa776c6e1..c41dfa53a9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -105,6 +105,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
 	if (pos >= 0) {
 		struct cache_entry *old = active_cache[pos];
 		if (ce->ce_mode == old->ce_mode &&
+		    !ce_intent_to_add(old) &&
 		    oideq(&ce->oid, &old->oid)) {
 			old->ce_flags |= CE_UPDATE;
 			discard_cache_entry(ce);
diff --git a/t/t2022-checkout-paths.sh b/t/t2022-checkout-paths.sh
index fc3eb43b89..6844afafc0 100755
--- a/t/t2022-checkout-paths.sh
+++ b/t/t2022-checkout-paths.sh
@@ -78,4 +78,15 @@ test_expect_success 'do not touch files that are already up-to-date' '
 	test_cmp expect actual
 '
 
+test_expect_success 'checkout HEAD 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 checkout HEAD nonempty empty &&
+	git diff --cached --exit-code
+'
+
 test_done
-- 
2.22.0