git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] apply: --intent-to-add should imply --index
  2021-11-06 11:24 Re* [PATCH v2] apply: make --intent-to-add not stomp index Johannes Altmanninger
@ 2021-11-06 11:42 ` Johannes Altmanninger
  2021-11-06 11:47   ` Johannes Altmanninger
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Altmanninger @ 2021-11-06 11:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: rhodges, git, rphodges, Johannes Altmanninger

Otherwise we do not read the current index, and more importantly, we
do not check with the current index, losing all the safety.

And the worst part of the story is that we still write the result
out to the index, which loses all the files that are not mentioned
in the incoming patch.

Make --intent-to-add imply --index. This means that apply
--intent-to-add will error without a repo, and refuse to modify
untracked files (except for added files). Add a test for the latter, and
another one to make sure that combinations like "--cached -N" keep working.
as documented (-N is ignored, otherwise it would do weird things to the index).

Use --intent-to-add instead of -N because we don't document -N in
git-apply.txt, which might be because it's much more obscure than "add -N".

Reported-by: Ryan Hodges <rhodges@cisco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Altmanninger <aclopte@gmail.com>
---

Not sure about the log message, it feels a bit stitched together.

Most importantly this adds a test to show the difference between v2 (where
ita did not imply check_index).

I wrapped the doc changes to 80 columns, but not the entire paragraph,
since we are inconsistent about that.

 Documentation/git-apply.txt |  7 +++----
 apply.c                     |  9 +++++++--
 t/t2203-add-intent.sh       |  2 +-
 t/t4140-apply-ita.sh        | 29 +++++++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index aa1ae56a25..18ddb4cf8a 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -77,10 +77,9 @@ OPTIONS
 --intent-to-add::
 	When applying the patch only to the working tree, mark new
 	files to be added to the index later (see `--intent-to-add`
-	option in linkgit:git-add[1]). This option is ignored unless
-	running in a Git repository and `--index` is not specified.
-	Note that `--index` could be implied by other options such
-	as `--cached` or `--3way`.
+	option in linkgit:git-add[1]). This option has is ignored if `--index`
+	is specified.  Note that `--index` could be implied by other options
+	such as `--cached` or `--3way`.
 
 -3::
 --3way::
diff --git a/apply.c b/apply.c
index 43a0aebf4e..b0239b7482 100644
--- a/apply.c
+++ b/apply.c
@@ -153,8 +153,13 @@ int check_apply_state(struct apply_state *state, int force_apply)
 			return error(_("--cached outside a repository"));
 		state->check_index = 1;
 	}
-	if (state->ita_only && (state->check_index || is_not_gitdir))
+	if (state->ita_only && state->check_index)
 		state->ita_only = 0;
+	if (state->ita_only) {
+		if (is_not_gitdir)
+			return error(_("--intent-to-add outside a repository"));
+		state->check_index = 1;
+	}
 	if (state->check_index)
 		state->unsafe_paths = 0;
 
@@ -4760,7 +4765,7 @@ static int apply_patch(struct apply_state *state,
 	if (state->whitespace_error && (state->ws_error_action == die_on_ws_error))
 		state->apply = 0;
 
-	state->update_index = (state->check_index || state->ita_only) && state->apply;
+	state->update_index = state->check_index && state->apply;
 	if (state->update_index && !is_lock_file_locked(&state->lock_file)) {
 		if (state->index_file)
 			hold_lock_file_for_update(&state->lock_file,
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
 '
 
diff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh
index c614eaf04c..4db1ae4e7e 100755
--- a/t/t4140-apply-ita.sh
+++ b/t/t4140-apply-ita.sh
@@ -53,4 +53,33 @@ test_expect_success 'apply deletion patch to ita path (--index)' '
 	git ls-files --stage --error-unmatch test-file
 '
 
+test_expect_success 'apply --intent-to-add is not allowed to modify untracked file' '
+	echo version1 >file &&
+	! git apply --intent-to-add <<-EOF
+	diff --git a/file b/file
+	index 1234567..89abcde 100644
+	--- b/file
+	+++ b/file
+	@@ -1 +1 @@
+	-version1
+	+version2
+	EOF
+'
+
+test_expect_success 'apply --index --intent-to-add ignores --intent-to-add, so it does not set i-t-a bit of touched file' '
+	echo >file &&
+	git add file &&
+	git apply --index --intent-to-add <<-EOF &&
+	diff --git a/file b/file
+	deleted file mode 100644
+	index 1234567..89abcde 100644
+	--- a/file
+	+++ /dev/null
+	@@ -1 +0,0 @@
+	-
+	EOF
+	git ls-files file >actual &&
+	test_must_be_empty actual
+'
+
 test_done
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] apply: --intent-to-add should imply --index
  2021-11-06 11:42 ` [PATCH v3] apply: --intent-to-add should imply --index Johannes Altmanninger
@ 2021-11-06 11:47   ` Johannes Altmanninger
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Altmanninger @ 2021-11-06 11:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: rhodges, git, rphodges

On Sat, Nov 06, 2021 at 12:42:02PM +0100, Johannes Altmanninger wrote:
> +test_expect_success 'apply --intent-to-add is not allowed to modify untracked file' '
> +	echo version1 >file &&
> +	! git apply --intent-to-add <<-EOF

I guess s/!/test_must_fail/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v3] apply: --intent-to-add should imply --index
@ 2025-05-01  9:03 Jason Cho
  2025-05-01 12:48 ` Kristoffer Haugsbakk
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Cho @ 2025-05-01  9:03 UTC (permalink / raw)
  To: git@vger.kernel.org
  Cc: aclopte@gmail.com, gitster@pobox.com, rhodges@cisco.com,
	rphodges@gmail.com

I'm following up on the bug reported by Ryan Hodges on October 26, 2021,
regarding the `git apply --intent-to-add` command incorrectly marking all 
other tracked files as deleted from the index.

Johannes Altmanninger submitted patch v3 titled "apply: --intent-to-add 
should imply --index" to fix this issue. 

Is this fix merged? If so, which Git version includes this fix.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] apply: --intent-to-add should imply --index
  2025-05-01  9:03 [PATCH v3] apply: --intent-to-add should imply --index Jason Cho
@ 2025-05-01 12:48 ` Kristoffer Haugsbakk
  2025-05-01 16:31   ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Kristoffer Haugsbakk @ 2025-05-01 12:48 UTC (permalink / raw)
  To: Jason Cho, git@vger.kernel.org
  Cc: aclopte@gmail.com, Junio C Hamano, rhodges@cisco.com,
	rphodges@gmail.com


On Thu, May 1, 2025, at 11:03, Jason Cho wrote:
> I'm following up on the bug reported by Ryan Hodges on October 26, 2021,
> regarding the `git apply --intent-to-add` command incorrectly marking all
> other tracked files as deleted from the index.
>
> Johannes Altmanninger submitted patch v3 titled "apply: --intent-to-add
> should imply --index" to fix this issue.
>
> Is this fix merged? If so, which Git version includes this fix.

I can’t find any commits by Johannes Altmanninger that addresses this.
I also can’t find any commits that start with `apply: --intent-to-add`.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] apply: --intent-to-add should imply --index
  2025-05-01 12:48 ` Kristoffer Haugsbakk
@ 2025-05-01 16:31   ` Junio C Hamano
  2025-05-02  4:11     ` Ryan Hodges
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2025-05-01 16:31 UTC (permalink / raw)
  To: Kristoffer Haugsbakk
  Cc: Jason Cho, git@vger.kernel.org, aclopte@gmail.com,
	rhodges@cisco.com, rphodges@gmail.com

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

>> Johannes Altmanninger submitted patch v3 titled "apply: --intent-to-add
>> should imply --index" to fix this issue.
>>
>> Is this fix merged? If so, which Git version includes this fix.
>
> I can’t find any commits by Johannes Altmanninger that addresses this.
> I also can’t find any commits that start with `apply: --intent-to-add`.

The documentation says this:

    --intent-to-add::
            When applying the patch only to the working tree, mark new
            files to be added to the index later (see `--intent-to-add`
            option in linkgit:git-add[1]). This option is ignored unless
            running in a Git repository and `--index` is not specified.
            Note that `--index` could be implied by other options such
            as `--cached` or `--3way`.

It is clear that whoever wrote it understands that for this option
to be effective, the patch needs to affect the index, and one way to
do so is for the user to pass `--index`.  But at the same time, that
is not the only option that makes the command touch the index (e.g.,
`--cached` does, too), and it would make it behave incorrectly if a
patch automatically pretends that `--index` was given when this
option was given.

I can't find the patch either, but given the above documentation, is
it even still relevant?

Thanks.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] apply: --intent-to-add should imply --index
  2025-05-01 16:31   ` Junio C Hamano
@ 2025-05-02  4:11     ` Ryan Hodges
  2025-05-03  3:51       ` Raymond E. Pasco
  0 siblings, 1 reply; 16+ messages in thread
From: Ryan Hodges @ 2025-05-02  4:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Kristoffer Haugsbakk, Jason Cho, git@vger.kernel.org,
	aclopte@gmail.com, Ryan Hodges



> On May 1, 2025, at 9:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> 
>>> Johannes Altmanninger submitted patch v3 titled "apply: --intent-to-add
>>> should imply --index" to fix this issue.
>>> 
>>> Is this fix merged? If so, which Git version includes this fix.
>> 
>> I can’t find any commits by Johannes Altmanninger that addresses this.
>> I also can’t find any commits that start with `apply: --intent-to-add`.
> 
> The documentation says this:
> 
>    --intent-to-add::
>            When applying the patch only to the working tree, mark new
>            files to be added to the index later (see `--intent-to-add`
>            option in linkgit:git-add[1]). This option is ignored unless
>            running in a Git repository and `--index` is not specified.
>            Note that `--index` could be implied by other options such
>            as `--cached` or `--3way`.
> 
> It is clear that whoever wrote it understands that for this option
> to be effective, the patch needs to affect the index, and one way to
> do so is for the user to pass `--index`.  But at the same time, that
> is not the only option that makes the command touch the index (e.g.,
> `--cached` does, too), and it would make it behave incorrectly if a
> patch automatically pretends that `--index` was given when this
> option was given.

The person that wrote that understood that there’s a conflict between
—index and —intent-to-add:

—index means add new files to the index and stage them for commit
—intent-to-add means add an entry for the file to the index but don’t stage it; ie it will be added later

The author of this commit decided that if —intent-to-add and —index are both specified, then —index takes precedence.


> 
> I can't find the patch either, but given the above documentation, is

The reason you can’t find the patch is because you took issue with the fix provided by Johannes and asked for more analysis.  If you want I can dig up the details of that thread.

> it even still relevant?
> 

This issue is still very relevant.  Firstly, let me point out the major issue with the current functionality:

Given a repo with one file, “b.c” and the following diff that adds file “a.c":

	ryan@Ryans-Macbook-Pro git-repo % cat diff
	diff --git a/a.c b/a.c
	new file mode 100644
	index 0000000..e69de29

Here’s what happens when you run git apply —intent-to-add:

	ryan@Ryans-Macbook-Pro git-repo % git apply --intent-to-add diff

	ryan@Ryans-Macbook-Pro git-repo % git status
	On branch master
	Changes to be committed:
   	(use "git restore --staged <file>..." to unstage)
        	 deleted:    b.c

Bam! b.c gets marked for deletion.  The whole index gets replaced with just the files that are marked by —intent-to-add. That’s very bad. 


Lastly, I’d like to point out my use case for —intent-to-add.  In many VCSs if you run ‘apply’, ‘diff’, and ‘commit’ in sequence and in a clean workspace with no changes, the diff that gets fed to ‘apply’ should look the same as the diff that you get from running ‘diff’, which should be the same as the diff created by the commit command.  Because of the index, that doesn’t happen with Git. When applying a patch in git, new files get added to the worktree but they are not registered in the index.  Thus git diff never sees them. Additionally, those files will be missed by commit even if you use -a. The —intent-to-add switch fixes this workflow and allows files to remain out of the index until the user is ready for them.

Thanks



> Thanks.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] apply: --intent-to-add should imply --index
  2025-05-02  4:11     ` Ryan Hodges
@ 2025-05-03  3:51       ` Raymond E. Pasco
  2025-05-03  8:22         ` Raymond E. Pasco
  2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
  0 siblings, 2 replies; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-03  3:51 UTC (permalink / raw)
  To: Ryan Hodges
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	git@vger.kernel.org, aclopte@gmail.com, Ryan Hodges

Intents to add are a tricky part of the system; I fixed them up
some time ago for `add -p` which uses apply.c machinery, but not for
`apply -N`, which seems to have never worked since its introduction
in Git 2.19.

To recap how this all works, apply has three modes: with no flag, it
applies a diff to the physical files in the worktree; with --index it
applies a diff to both the physical files in the worktree and to the
index, and with --cached it applies to the index but *not* the physical
files in the worktree.

--intent-to-add / -N is intended to apply only to the first of these
modes; this makes sense, because an intent to add is meant to behave
like a diff not added to the index. However, the intent to add lives in
the index; Git just behaves as though it were a worktree change not in
the index.

The behavior `apply -N` actually exhibits is that it clobbers the index
with a new index containing *only* the contents of the diff, nothing
else; my guess is that it was only tested against repositories with
entirely empty trees. If the tree is not empty, then of course an index
with only the intent to add and nothing else shows up as every file in
the tree being deleted.

The patch discussed here (the headers for the thread seem broken,
but the message id is <20211106114202.3486969-1-aclopte@gmail.com>) does
seem like a mostly complete fix for the issue. However, the message is
entirely wrong and confused about how any of this works, which is likely
why the patch fell through the cracks. (Of course --intent-to-add can't
imply --index, they are mutually exclusive options.)

However, the code appears entirely correct. The combination of --cached
with -N doesn't work, despite the message claiming it does, but it can't
possibly work because it includes the file in the index, so it can't
include it as an intent to add in the index. So this just merits a note
that --intent-to-add is mutually exclusive with both --index and
--cached.

If the original author (Johannes Altmanninger) isn't around or doesn't
want to, I can clean this patch up for resubmission.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] apply: --intent-to-add should imply --index
  2025-05-03  3:51       ` Raymond E. Pasco
@ 2025-05-03  8:22         ` Raymond E. Pasco
  2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
  1 sibling, 0 replies; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-03  8:22 UTC (permalink / raw)
  To: Ryan Hodges
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	git@vger.kernel.org, aclopte@gmail.com, Ryan Hodges

On 25/05/02 11:51PM, Raymond E. Pasco wrote:
> However, the code appears entirely correct. The combination of --cached
> with -N doesn't work, despite the message claiming it does, but it can't
> possibly work because it includes the file in the index, so it can't
> include it as an intent to add in the index. So this just merits a note
> that --intent-to-add is mutually exclusive with both --index and
> --cached.
> 
> If the original author (Johannes Altmanninger) isn't around or doesn't
> want to, I can clean this patch up for resubmission.

In fact, I've just discovered an additional issue that remains even
post-patch (it tries to apply the whole diff, even parts that are
not new file additions, with ita_only on), so it needs slightly more
work than implied above to be complete.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/5] apply: fix apply --intent-to-add
  2025-05-03  3:51       ` Raymond E. Pasco
  2025-05-03  8:22         ` Raymond E. Pasco
@ 2025-05-11  0:36         ` Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 1/5] apply: error on --intent-to-add outside gitdir Raymond E. Pasco
                             ` (4 more replies)
  1 sibling, 5 replies; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-11  0:36 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	aclopte@gmail.com, Ryan Hodges, Raymond E. Pasco

This patch series fixes the -N/--intent-to-add flag for git apply, which
has not worked properly since its introduction in Git 2.19.

In particular, the index is properly initialized, and not touched except
to add the requested intents to add.

Loosely based on the patch from Johannes Altmanninger, message-id
<20211106114202.3486969-1-aclopte@gmail.com>, but that patch turned out to
be incorrect.

Raymond E. Pasco (5):
  apply: error on --intent-to-add outside gitdir
  apply: read in the index in --intent-to-add mode
  apply: only write intents to add for new files
  t4140: test apply --intent-to-add interactions
  apply docs: clarify wording for --intent-to-add

 Documentation/git-apply.adoc |  8 ++++----
 apply.c                      | 12 ++++++++----
 t/t4140-apply-ita.sh         | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 9 deletions(-)

-- 
2.49.0.1106.gc0efa3ba58


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/5] apply: error on --intent-to-add outside gitdir
  2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
@ 2025-05-11  0:36           ` Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 2/5] apply: read in the index in --intent-to-add mode Raymond E. Pasco
                             ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-11  0:36 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	aclopte@gmail.com, Ryan Hodges, Raymond E. Pasco

It makes no sense to register an intent to add outside a repository. We
should error out here.

Based-on-patch-by: Johannes Altmanninger <aclopte@gmail.com>
Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
---
 apply.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index f274a37948..5e39cadde4 100644
--- a/apply.c
+++ b/apply.c
@@ -174,8 +174,12 @@ int check_apply_state(struct apply_state *state, int force_apply)
 			return error(_("'%s' outside a repository"), "--cached");
 		state->check_index = 1;
 	}
-	if (state->ita_only && (state->check_index || is_not_gitdir))
-		state->ita_only = 0;
+	if (state->ita_only) {
+		if (is_not_gitdir)
+			return error(_("'%s' outside a repository"), "--intent-to-add");
+		if (state->check_index)
+			state->ita_only = 0;
+	}
 	if (state->check_index)
 		state->unsafe_paths = 0;
 
-- 
2.49.0.1106.gc0efa3ba58


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] apply: read in the index in --intent-to-add mode
  2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 1/5] apply: error on --intent-to-add outside gitdir Raymond E. Pasco
@ 2025-05-11  0:36           ` Raymond E. Pasco
  2025-05-12  2:13             ` Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 3/5] apply: only write intents to add for new files Raymond E. Pasco
                             ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-11  0:36 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	aclopte@gmail.com, Ryan Hodges, Raymond E. Pasco

There are three main modes of operation for apply: applying only to the
worktree, applying to the worktree and index (--index), and applying
only to the index (--cached).

The --intent-to-add flag modifies the first of these modes, applying
only to the worktree, in a way which touches the index, because
intents to add are special index entries. However, it has not ever
worked correctly in any but the most trivial (empty repository)
cases, because the index was never read in (in apply, this is done
in read_apply_cache()) before writing to it.

The update_index flag is set in apply_patch() to mean that we
are touching the index at all, as opposed to the check_index flag
indicating --index mode.  Therefore, the reading of the index should
be gated by the update_index flag rather than the check_index flag,
so that we are prepared to work with the index before we begin adding
intents to add to it.

Reported-by: Ryan Hodges <rhodges@cisco.com>
Original-patch-by: Johannes Altmanninger <aclopte@gmail.com>
Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
---
 apply.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index 5e39cadde4..3bde54a04a 100644
--- a/apply.c
+++ b/apply.c
@@ -4837,7 +4837,7 @@ static int apply_patch(struct apply_state *state,
 					       LOCK_DIE_ON_ERROR);
 	}
 
-	if (state->check_index && read_apply_cache(state) < 0) {
+	if (state->update_index && read_apply_cache(state) < 0) {
 		error(_("unable to read index file"));
 		res = -128;
 		goto end;
-- 
2.49.0.1106.gc0efa3ba58


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] apply: only write intents to add for new files
  2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 1/5] apply: error on --intent-to-add outside gitdir Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 2/5] apply: read in the index in --intent-to-add mode Raymond E. Pasco
@ 2025-05-11  0:36           ` Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 4/5] t4140: test apply --intent-to-add interactions Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 5/5] apply docs: clarify wording for --intent-to-add Raymond E. Pasco
  4 siblings, 0 replies; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-11  0:36 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	aclopte@gmail.com, Ryan Hodges, Raymond E. Pasco

In the "update only the worktree" mode, the index should not be touched
except to record intents to add when --intent-to-add is on. Because
having --intent-to-add on sets update_index, to indicate that we are
touching the index, we can't rely only on that flag to decide whether to
write an index entry. Instead, we must test whether we are in a mode
which updates the index, or else are in worktree-only mode with
--intent-to-add on and the current file being an addition.

Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
---
 apply.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index 3bde54a04a..a749f904c7 100644
--- a/apply.c
+++ b/apply.c
@@ -4569,7 +4569,7 @@ static int create_file(struct apply_state *state, struct patch *patch)
 
 	if (patch->conflicted_threeway)
 		return add_conflicted_stages_file(state, patch);
-	else if (state->update_index)
+	else if (state->check_index || (state->ita_only && patch->is_new > 0))
 		return add_index_file(state, path, mode, buf, size);
 	return 0;
 }
-- 
2.49.0.1106.gc0efa3ba58


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] t4140: test apply --intent-to-add interactions
  2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
                             ` (2 preceding siblings ...)
  2025-05-11  0:36           ` [PATCH 3/5] apply: only write intents to add for new files Raymond E. Pasco
@ 2025-05-11  0:36           ` Raymond E. Pasco
  2025-05-11  0:36           ` [PATCH 5/5] apply docs: clarify wording for --intent-to-add Raymond E. Pasco
  4 siblings, 0 replies; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-11  0:36 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	aclopte@gmail.com, Ryan Hodges, Raymond E. Pasco

Test that applying a new file creation patch to an existing index works,
and that applying a patch with both modifications and new file creations
works.

Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
---
 t/t4140-apply-ita.sh | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh
index c614eaf04c..c664209c41 100755
--- a/t/t4140-apply-ita.sh
+++ b/t/t4140-apply-ita.sh
@@ -7,6 +7,10 @@ test_description='git apply of i-t-a file'
 test_expect_success setup '
 	test_write_lines 1 2 3 4 5 >blueprint &&
 
+  cat blueprint >committed-file &&
+  git add committed-file &&
+  git commit -m "commit" &&
+
 	cat blueprint >test-file &&
 	git add -N test-file &&
 	git diff >creation-patch &&
@@ -14,7 +18,14 @@ test_expect_success setup '
 
 	rm -f test-file &&
 	git diff >deletion-patch &&
-	grep "deleted file mode 100644" deletion-patch
+	grep "deleted file mode 100644" deletion-patch &&
+
+	git rm -f test-file &&
+	test_write_lines 6 >>committed-file &&
+	cat blueprint >test-file &&
+	git add -N test-file &&
+	git diff >complex-patch &&
+	git restore committed-file
 '
 
 test_expect_success 'apply creation patch to ita path (--cached)' '
@@ -53,4 +64,22 @@ test_expect_success 'apply deletion patch to ita path (--index)' '
 	git ls-files --stage --error-unmatch test-file
 '
 
+test_expect_success 'apply creation patch to existing index with -N' '
+  git rm -f test-file &&
+  cat blueprint >index-file &&
+  git add index-file &&
+  git apply -N creation-patch &&
+
+  git ls-files --stage --error-unmatch index-file &&
+  git ls-files --stage --error-unmatch test-file
+'
+
+test_expect_success 'apply complex patch with -N' '
+  git rm -f test-file index-file &&
+  git apply -N complex-patch &&
+
+  git ls-files --stage --error-unmatch test-file &&
+  git diff | grep "a/committed-file"
+'
+
 test_done
-- 
2.49.0.1106.gc0efa3ba58


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] apply docs: clarify wording for --intent-to-add
  2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
                             ` (3 preceding siblings ...)
  2025-05-11  0:36           ` [PATCH 4/5] t4140: test apply --intent-to-add interactions Raymond E. Pasco
@ 2025-05-11  0:36           ` Raymond E. Pasco
  4 siblings, 0 replies; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-11  0:36 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	aclopte@gmail.com, Ryan Hodges, Raymond E. Pasco

Avoid using a double negative, and keep in mind that --index and
--cached are distinct modes of operation.

Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
---
 Documentation/git-apply.adoc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-apply.adoc b/Documentation/git-apply.adoc
index 952518b8af..a41069c0ab 100644
--- a/Documentation/git-apply.adoc
+++ b/Documentation/git-apply.adoc
@@ -75,13 +75,13 @@ OPTIONS
 	tree. If `--check` is in effect, merely check that it would
 	apply cleanly to the index entry.
 
+-N::
 --intent-to-add::
 	When applying the patch only to the working tree, mark new
 	files to be added to the index later (see `--intent-to-add`
-	option in linkgit:git-add[1]). This option is ignored unless
-	running in a Git repository and `--index` is not specified.
-	Note that `--index` could be implied by other options such
-	as `--cached` or `--3way`.
+	option in linkgit:git-add[1]). This option is ignored if
+	`--index` or `--cached` are used. Note that `--index` could
+	be implied by other options such as `--3way`.
 
 -3::
 --3way::
-- 
2.49.0.1106.gc0efa3ba58


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] apply: read in the index in --intent-to-add mode
  2025-05-11  0:36           ` [PATCH 2/5] apply: read in the index in --intent-to-add mode Raymond E. Pasco
@ 2025-05-12  2:13             ` Raymond E. Pasco
  2025-05-13 17:52               ` Jason Cho
  0 siblings, 1 reply; 16+ messages in thread
From: Raymond E. Pasco @ 2025-05-12  2:13 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Jason Cho,
	aclopte@gmail.com, Ryan Hodges

On 25/05/10 08:36PM, Raymond E. Pasco wrote:
> diff --git a/apply.c b/apply.c
> index 5e39cadde4..3bde54a04a 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -4837,7 +4837,7 @@ static int apply_patch(struct apply_state *state,
>  					       LOCK_DIE_ON_ERROR);
>  	}
>  
> -	if (state->check_index && read_apply_cache(state) < 0) {
> +	if (state->update_index && read_apply_cache(state) < 0) {
>  		error(_("unable to read index file"));
>  		res = -128;
>  		goto end;
> -- 
> 2.49.0.1106.gc0efa3ba58
> 
> 

For some reason, this causes checkout -p to break in a way which which I
don't understand. Directly using (state->check_index || state->ita_only)
doesn't break it, but because it doesn't seem logically possible to me
for this to change any behavior besides behavior when ita_only is on
(the update_index flag is in fact set just a few lines above!), I want
to investigate a bit further to find out what I'm confused about.

Either way, reroll coming, though I'd still appreciate review on the
series in general.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] apply: read in the index in --intent-to-add mode
  2025-05-12  2:13             ` Raymond E. Pasco
@ 2025-05-13 17:52               ` Jason Cho
  0 siblings, 0 replies; 16+ messages in thread
From: Jason Cho @ 2025-05-13 17:52 UTC (permalink / raw)
  To: Raymond E. Pasco
  Cc: git, Junio C Hamano, Kristoffer Haugsbakk, aclopte@gmail.com,
	Ryan Hodges

Raymond, thank you for getting on top of my initial query. Although your current patch has bugs, I'm confident that you are able to fix it.

I'm looking forward to see the option --intent-to-add works correctly.

I appreciate the effort of you and code reviewers.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-05-13 17:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-01  9:03 [PATCH v3] apply: --intent-to-add should imply --index Jason Cho
2025-05-01 12:48 ` Kristoffer Haugsbakk
2025-05-01 16:31   ` Junio C Hamano
2025-05-02  4:11     ` Ryan Hodges
2025-05-03  3:51       ` Raymond E. Pasco
2025-05-03  8:22         ` Raymond E. Pasco
2025-05-11  0:36         ` [PATCH 0/5] apply: fix apply --intent-to-add Raymond E. Pasco
2025-05-11  0:36           ` [PATCH 1/5] apply: error on --intent-to-add outside gitdir Raymond E. Pasco
2025-05-11  0:36           ` [PATCH 2/5] apply: read in the index in --intent-to-add mode Raymond E. Pasco
2025-05-12  2:13             ` Raymond E. Pasco
2025-05-13 17:52               ` Jason Cho
2025-05-11  0:36           ` [PATCH 3/5] apply: only write intents to add for new files Raymond E. Pasco
2025-05-11  0:36           ` [PATCH 4/5] t4140: test apply --intent-to-add interactions Raymond E. Pasco
2025-05-11  0:36           ` [PATCH 5/5] apply docs: clarify wording for --intent-to-add Raymond E. Pasco
  -- strict thread matches above, loose matches on Subject: below --
2021-11-06 11:24 Re* [PATCH v2] apply: make --intent-to-add not stomp index Johannes Altmanninger
2021-11-06 11:42 ` [PATCH v3] apply: --intent-to-add should imply --index Johannes Altmanninger
2021-11-06 11:47   ` Johannes Altmanninger

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 NNTP newsgroup(s).