git apply --indent-to-add deletes other files from the index

Hi all,
 
I’ve got a quick question about ‘git apply –intent-to-add’.  If I’ve got a patch that just adds one file to the tree:
 
[sjc-ads-2565:t.git]$ git diff

diff --git a/c.c b/c.c
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/c.c

@@ -0,0 +1 @@
+test
 

and I apply that patch with –intent-to-add:
 
[sjc-ads-2565:t.git]$ git apply --intent-to-add c.diff
 
The newly added file is tracked but other files in the tree get marked as deleted:
 
[sjc-ads-2565:t.git]$ git status
On branch master
Changes to be committed:
  (use “git restore –staged <file>…” to unstage)
                deleted:    a.c
                deleted:    b.c
 
Changes not staged for commit:
  (use “git add <file>…” to update what will be committed)
  (use “git restore <file>…” to discard changes in working directory)
                new file:   c.c
 
It looks like Git created a new index with only the newly added file in the patch.  However, I’d like Git to just add one entry to the index corresponding to the newly added file in the patch.  Is this a bug or am I completely misinterpreting the goal of ‘intent-to-add’.  I just started looking at the source but a quick message from the experts would be much appreciated. 
 
I’m currently testing with Git version 2.33.
 
Regards,
Ryan
 

On Tue, Oct 26, 2021 at 03:11:36PM +0000, Ryan Hodges (rhodges) wrote:

Yep, looks like a bug to me.
git apply should never change the status of files that are not mentioned in
the input patch.

                deleted:    b.c
 
Changes not staged for commit:
  (use “git add <file>…” to update what will be committed)
  (use “git restore <file>…” to discard changes in working directory)
                new file:   c.c
 
It looks like Git created a new index with only the newly added file in the patch.

Seems so.

However, I’d like Git to just add one entry to the index corresponding
to the newly added file in the patch.  Is this a bug or am I completely
misinterpreting the goal of ‘intent-to-add’.

Yeah, I think your "git apply --intent-to-add c.diff" should behave exactly like

	echo test > c.c && git add --intent-to-add c.c

Commit cff5dc09ed (apply: add --intent-to-add, 2018-05-26) introduced
"apply -N" plus a test to make sure it behaves exactly as "add -N"
when given equivalent changes.  However, the test only checks working
tree changes. Now "apply -N" forgot to read the index, so it left
all tracked files as deleted, except for the ones it touched.

Fix this by reading the index file, like we do for "apply --cached".
and test that we leave no content changes in the index.

Reported-by: Ryan Hodges <redacted>
Signed-off-by: Johannes Altmanninger <redacted>
---
 apply.c               | 2 +-
 t/t2203-add-intent.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index 43a0aebf4e..4f740e373b 100644
--- a/apply.c
+++ b/apply.c

@@ -4771,7 +4771,7 @@ static int apply_patch(struct apply_state *state,
 					       LOCK_DIE_ON_ERROR);
 	}
 
-	if (state->check_index && read_apply_cache(state) < 0) {
+	if ((state->check_index || state->ita_only) && read_apply_cache(state) < 0) {
 		error(_("unable to read index file"));
 		res = -128;
 		goto end;

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index cf0175ad6e..035ce3a2b9 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh

@@ -307,7 +307,7 @@ test_expect_success 'apply --intent-to-add' '
 	grep "new file" expected &&
 	git reset --hard &&
 	git apply --intent-to-add expected &&
-	git diff >actual &&
+	(git diff && git diff --cached) >actual &&
 	test_cmp expected actual
 '
 

-- 
2.33.1

Commit cff5dc09ed (apply: add --intent-to-add, 2018-05-26) introduced
"apply -N" plus a test to make sure it behaves exactly as "add -N"
when given equivalent changes.  However, the test only checks working
tree changes. Now "apply -N" forgot to read the index, so it left
all tracked files as deleted, except for the ones it touched.

Fix this by reading the index file, like we do for "apply --cached".
and test that we leave no content changes in the index.

Reported-by: Ryan Hodges <redacted>
Signed-off-by: Johannes Altmanninger <redacted>
---

Sorry I used the wrong Reported-by: address in v1

 apply.c               | 2 +-
 t/t2203-add-intent.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index 43a0aebf4e..4f740e373b 100644
--- a/apply.c
+++ b/apply.c

@@ -4771,7 +4771,7 @@ static int apply_patch(struct apply_state *state,
 					       LOCK_DIE_ON_ERROR);
 	}
 
-	if (state->check_index && read_apply_cache(state) < 0) {
+	if ((state->check_index || state->ita_only) && read_apply_cache(state) < 0) {
 		error(_("unable to read index file"));
 		res = -128;
 		goto end;

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index cf0175ad6e..035ce3a2b9 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh

@@ -307,7 +307,7 @@ test_expect_success 'apply --intent-to-add' '
 	grep "new file" expected &&
 	git reset --hard &&
 	git apply --intent-to-add expected &&
-	git diff >actual &&
+	(git diff && git diff --cached) >actual &&
 	test_cmp expected actual
 '
 

-- 
2.33.1

Thank you. I was hoping to be the one that fixed this because it was a level of logic that matched my current knowledge level.  I appreciate you jumping in with a fix and also confirming this was unexpected behavior.  I was kind of surprised no one has reported this before.

Cheers,
Ryan

On Oct 30, 2021, at 1:39 PM, Johannes Altmanninger [off-list ref] wrote:

On Tue, Oct 26, 2021 at 03:11:36PM +0000, Ryan Hodges (rhodges) wrote:

quoted

Hi all,

I’ve got a quick question about ‘git apply –intent-to-add’.  If I’ve got a patch that just adds one file to the tree:

[sjc-ads-2565:t.git]$ git diff

diff --git a/c.c b/c.c
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/c.c

@@ -0,0 +1 @@
+test

and I apply that patch with –intent-to-add:

[sjc-ads-2565:t.git]$ git apply --intent-to-add c.diff

The newly added file is tracked but other files in the tree get marked as deleted:

[sjc-ads-2565:t.git]$ git status
On branch master
Changes to be committed:
 (use “git restore –staged <file>…” to unstage)
               deleted:    a.c

Yep, looks like a bug to me.
git apply should never change the status of files that are not mentioned in
the input patch.

quoted

               deleted:    b.c

Changes not staged for commit:
 (use “git add <file>…” to update what will be committed)
 (use “git restore <file>…” to discard changes in working directory)
               new file:   c.c

It looks like Git created a new index with only the newly added file in the patch.

Seems so.

quoted

However, I’d like Git to just add one entry to the index corresponding
to the newly added file in the patch.  Is this a bug or am I completely
misinterpreting the goal of ‘intent-to-add’.

Yeah, I think your "git apply --intent-to-add c.diff" should behave exactly like

	echo test > c.c && git add --intent-to-add c.c

On Sat, Oct 30, 2021 at 02:42:42PM -0700, Ryan Hodges wrote:

Thank you. I was hoping to be the one that fixed this because it was a level of logic that matched my current knowledge level.

Sorry I should have just confirmed the bug since you had already said to
look into it. (I usually try to send things when they are "done" from my
side to minimize roundtrips.)
I'm sure there are more low-hanging fruits but finding them is the hard part,
see also https://lore.kernel.org/git/xmqq7dl5z425.fsf@gitster.g/

I was kind of surprised no one has reported this before.

I guess no one has used it since it was added in 2018.