{"thread":{"id":"63706","subject":"[PATCH 0/5] fix apply --intent-to-add","startedAt":"2025-06-28T22:59:44Z","lastAt":"2025-07-07T17:51:15Z","messageCount":25,"participants":["Raymond E. Pasco","Lidong Yan","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"520854","messageId":"20250628225819.1294068-2-ray@ameretat.dev","threadId":"63706","inReplyTo":null,"subject":"[PATCH 0/5] fix apply --intent-to-add","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-06-28T22:52:02Z","receivedAt":"2025-06-28T22:59:44Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"The --intent-to-add (-N) flag to apply has not worked properly since its\nintroduction in Git 2.19; in particular, it creates an empty index\nexcept for any new files in the patch, making it largely useless except\non blank repositories.\n\nThis patch series fixes it to work as expected and updates the tests\nand documentation for this flag.\n\nEarlier discussion of this issue can be found in the threads associated\nwith message ids <20211106114202.3486969-1-aclopte@gmail.com> and\n<20250511003955.242889-1-ray@ameretat.dev>.\n\nRaymond E. Pasco (5):\n  apply: error on --intent-to-add outside gitdir\n  apply: read in the index in --intent-to-add mode\n  apply: only write intents to add for new files\n  t4140: test apply --intent-to-add interactions\n  apply docs: clarify wording for --intent-to-add\n\n Documentation/git-apply.adoc |  8 ++++----\n apply.c                      | 12 ++++++++----\n t/t4140-apply-ita.sh         | 31 ++++++++++++++++++++++++++++++-\n 3 files changed, 42 insertions(+), 9 deletions(-)\n\n-- \n2.50.0.195.g74e6fc65d0\n\n"},{"id":"520855","messageId":"20250628225819.1294068-3-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250628225819.1294068-2-ray@ameretat.dev","subject":"[PATCH 1/5] apply: error on --intent-to-add outside gitdir","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-06-28T22:52:03Z","receivedAt":"2025-06-28T22:59:58Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"It makes no sense to register an intent to add outside a repository. We\nshould error out here.\n\nBased-on-patch-by: Johannes Altmanninger <aclopte@gmail.com>\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n apply.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 8bbe6ed224..e7856ae6b3 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -174,8 +174,12 @@ int check_apply_state(struct apply_state *state, int force_apply)\n \t\t\treturn error(_(\"'%s' outside a repository\"), \"--cached\");\n \t\tstate->check_index = 1;\n \t}\n-\tif (state->ita_only && (state->check_index || is_not_gitdir))\n-\t\tstate->ita_only = 0;\n+\tif (state->ita_only) {\n+\t\tif (is_not_gitdir)\n+\t\t\treturn error(_(\"'%s' outside a repository\"), \"--intent-to-add\");\n+\t\tif (state->check_index)\n+\t\t\tstate->ita_only = 0;\n+\t}\n \tif (state->check_index)\n \t\tstate->unsafe_paths = 0;\n \n-- \n2.50.0.195.g74e6fc65d0\n\n"},{"id":"520856","messageId":"20250628225819.1294068-4-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250628225819.1294068-2-ray@ameretat.dev","subject":"[PATCH 2/5] apply: read in the index in --intent-to-add mode","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-06-28T22:52:04Z","receivedAt":"2025-06-28T22:59:59Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"There are three main modes of operation for apply: applying only to the\nworktree, applying to the worktree and index (--index), and applying\nonly to the index (--cached).\n\nThe --intent-to-add flag modifies the first of these modes, applying\nonly to the worktree, in a way which touches the index, because\nintents to add are special index entries. However, it has not ever\nworked correctly in any but the most trivial (empty repository)\ncases, because the index was never read in (in apply, this is done\nin read_apply_cache()) before writing to it.\n\nIf we merely gate read_apply_cache() behind update_index, then it will\nnot be read when state->apply is false, even if it must be checked.\nTherefore, we instead read the index if it will be either checked or\nupdated, because reading the index is a prerequisite to either.\n\nReported-by: Ryan Hodges <rhodges@cisco.com>\nOriginal-patch-by: Johannes Altmanninger <aclopte@gmail.com>\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n apply.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex e7856ae6b3..1757d34618 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4837,7 +4837,7 @@ static int apply_patch(struct apply_state *state,\n \t\t\t\t\t       LOCK_DIE_ON_ERROR);\n \t}\n \n-\tif (state->check_index && read_apply_cache(state) < 0) {\n+\tif ((state->check_index || state->update_index) && read_apply_cache(state) < 0) {\n \t\terror(_(\"unable to read index file\"));\n \t\tres = -128;\n \t\tgoto end;\n-- \n2.50.0.195.g74e6fc65d0\n\n"},{"id":"520857","messageId":"20250628225819.1294068-5-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250628225819.1294068-2-ray@ameretat.dev","subject":"[PATCH 3/5] apply: only write intents to add for new files","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-06-28T22:52:05Z","receivedAt":"2025-06-28T22:59:59Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"In the \"update only the worktree\" mode, the index should not be touched\nexcept to record intents to add when --intent-to-add is on. Because\nhaving --intent-to-add on sets update_index, to indicate that we are\ntouching the index, we can't rely only on that flag to decide whether to\nwrite an index entry.\n\nInstead, we must test whether we are in a mode which updates the\nindex, or else are in worktree-only mode with --intent-to-add on and\nthe current file being an addition. We do not need to check\nstate->apply, because we only enter write_out_results() if state->apply\nis already set.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n apply.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex 1757d34618..5064a91fbf 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4569,7 +4569,7 @@ static int create_file(struct apply_state *state, struct patch *patch)\n \n \tif (patch->conflicted_threeway)\n \t\treturn add_conflicted_stages_file(state, patch);\n-\telse if (state->update_index)\n+\telse if (state->check_index || (state->ita_only && patch->is_new > 0))\n \t\treturn add_index_file(state, path, mode, buf, size);\n \treturn 0;\n }\n-- \n2.50.0.195.g74e6fc65d0\n\n"},{"id":"520858","messageId":"20250628225819.1294068-6-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250628225819.1294068-2-ray@ameretat.dev","subject":"[PATCH 4/5] t4140: test apply --intent-to-add interactions","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-06-28T22:52:06Z","receivedAt":"2025-06-28T23:00:00Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"Test that applying a new file creation patch to an existing index works,\nand that applying a patch with both modifications and new file creations\nworks.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n t/t4140-apply-ita.sh | 31 ++++++++++++++++++++++++++++++-\n 1 file changed, 30 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh\nindex c614eaf04c..c664209c41 100755\n--- a/t/t4140-apply-ita.sh\n+++ b/t/t4140-apply-ita.sh\n@@ -7,6 +7,10 @@ test_description='git apply of i-t-a file'\n test_expect_success setup '\n \ttest_write_lines 1 2 3 4 5 >blueprint &&\n \n+  cat blueprint >committed-file &&\n+  git add committed-file &&\n+  git commit -m \"commit\" &&\n+\n \tcat blueprint >test-file &&\n \tgit add -N test-file &&\n \tgit diff >creation-patch &&\n@@ -14,7 +18,14 @@ test_expect_success setup '\n \n \trm -f test-file &&\n \tgit diff >deletion-patch &&\n-\tgrep \"deleted file mode 100644\" deletion-patch\n+\tgrep \"deleted file mode 100644\" deletion-patch &&\n+\n+\tgit rm -f test-file &&\n+\ttest_write_lines 6 >>committed-file &&\n+\tcat blueprint >test-file &&\n+\tgit add -N test-file &&\n+\tgit diff >complex-patch &&\n+\tgit restore committed-file\n '\n \n test_expect_success 'apply creation patch to ita path (--cached)' '\n@@ -53,4 +64,22 @@ test_expect_success 'apply deletion patch to ita path (--index)' '\n \tgit ls-files --stage --error-unmatch test-file\n '\n \n+test_expect_success 'apply creation patch to existing index with -N' '\n+  git rm -f test-file &&\n+  cat blueprint >index-file &&\n+  git add index-file &&\n+  git apply -N creation-patch &&\n+\n+  git ls-files --stage --error-unmatch index-file &&\n+  git ls-files --stage --error-unmatch test-file\n+'\n+\n+test_expect_success 'apply complex patch with -N' '\n+  git rm -f test-file index-file &&\n+  git apply -N complex-patch &&\n+\n+  git ls-files --stage --error-unmatch test-file &&\n+  git diff | grep \"a/committed-file\"\n+'\n+\n test_done\n-- \n2.50.0.195.g74e6fc65d0\n\n"},{"id":"520859","messageId":"20250628225819.1294068-7-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250628225819.1294068-2-ray@ameretat.dev","subject":"[PATCH 5/5] apply docs: clarify wording for --intent-to-add","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-06-28T22:52:07Z","receivedAt":"2025-06-28T23:00:01Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"Avoid using a double negative, and keep in mind that --index and\n--cached are distinct modes of operation.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n Documentation/git-apply.adoc | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-apply.adoc b/Documentation/git-apply.adoc\nindex 952518b8af..a41069c0ab 100644\n--- a/Documentation/git-apply.adoc\n+++ b/Documentation/git-apply.adoc\n@@ -75,13 +75,13 @@ OPTIONS\n \ttree. If `--check` is in effect, merely check that it would\n \tapply cleanly to the index entry.\n \n+-N::\n --intent-to-add::\n \tWhen applying the patch only to the working tree, mark new\n \tfiles to be added to the index later (see `--intent-to-add`\n-\toption in linkgit:git-add[1]). This option is ignored unless\n-\trunning in a Git repository and `--index` is not specified.\n-\tNote that `--index` could be implied by other options such\n-\tas `--cached` or `--3way`.\n+\toption in linkgit:git-add[1]). This option is ignored if\n+\t`--index` or `--cached` are used. Note that `--index` could\n+\tbe implied by other options such as `--3way`.\n \n -3::\n --3way::\n-- \n2.50.0.195.g74e6fc65d0\n\n"},{"id":"520860","messageId":"16943C7D-C331-482E-BFDD-236E8E79552A@gmail.com","threadId":"63706","inReplyTo":"20250628225819.1294068-2-ray@ameretat.dev","subject":"Re: [PATCH 0/5] fix apply --intent-to-add","fromName":"Lidong Yan","fromEmail":"yldhome2d2@gmail.com","sentAt":"2025-06-29T03:10:57Z","receivedAt":"2025-06-29T03:11:12Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"Raymond E. Pasco <ray@ameretat.dev> writes:\n> The --intent-to-add (-N) flag to apply has not worked properly since its\n> introduction in Git 2.19; in particular, it creates an empty index\n> except for any new files in the patch, making it largely useless except\n> on blank repositories.\n\nI find the problem is that when we use 'git apply —intent-to-add’, git doesn’t\nread index file then update the intent-to-add cache entry, git forgot to read\nthe index file and only update one intent-to-add cache entry in index file. \n\n> This patch series fixes it to work as expected and updates the tests\n> and documentation for this flag.\n\nI think [PATCH 2/5] actually solve the problem. [PATCH 3/5] makes 'git apply'\nconsistent with 'git add', which means\n  git add —intent-to-add exist-in-index-file.c\nand\n  git apply —intent-to-add patch-contains-exist-in-index-file\n\nwill ignore intent-to-add and do nothing."},{"id":"520890","messageId":"tjoz7imj4yx7gvyyew5tk6eytfq2ipq4ip4nbgbkxekp6ellcb@4ri4ehz5cwtp","threadId":"63706","inReplyTo":"16943C7D-C331-482E-BFDD-236E8E79552A@gmail.com","subject":"Re: [PATCH 0/5] fix apply --intent-to-add","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-06-30T00:56:49Z","receivedAt":"2025-06-30T00:57:04Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On 25/06/29 11:10AM, Lidong Yan wrote:\n> I think [PATCH 2/5] actually solve the problem. [PATCH 3/5] makes 'git apply'\n> consistent with 'git add', which means\n>   git add —intent-to-add exist-in-index-file.c\n> and\n>   git apply —intent-to-add patch-contains-exist-in-index-file\n> \n> will ignore intent-to-add and do nothing.\n\nThis behavior is correct and expected (even if you don't register an\nintent to add) because the file is there in the worktree, so you can't\ncreate a new file in the worktree with that name.\n\nBut the real reason for that test in create_file is that we're in a mode\nthat shouldn't be touching the index at all, so we shouldn't touch the\nindex (except in the special case where intents to add live in the\nindex). If there's a patch that updates existing file A, and creates new\nfile B, then without that test apply will touch the index for both A and\nB, which is wrong. It must only touch it for B.\n"},{"id":"520950","messageId":"xmqqfrfh13k5.fsf@gitster.g","threadId":"63706","inReplyTo":"20250628225819.1294068-3-ray@ameretat.dev","subject":"Re: [PATCH 1/5] apply: error on --intent-to-add outside gitdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-30T18:34:34Z","receivedAt":"2025-06-30T18:34:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n\n> It makes no sense to register an intent to add outside a repository. We\n> should error out here.\n\nI am not so sure, especially if you still keep the original \"if we\nare using 'git apply' as a better GNU patch, ignore -N\" logic.\n\nNot that I am suggesting to also error out when -N is given without\n--index or --cached, that is.\n\nI think the most problematic is the claim \"it makes no sense\" is not\nlinked to the conclusion \"we should error out\".  The behaviour of\nthe original code, and the behaviour of the code with this patch in\n\"a better GNU patch\" mode is based on \"it makes no sense in such a\ncontext, hence we just ignore (instead of erroring out)\", which\nmakes perfect sense as well.\n\n> Based-on-patch-by: Johannes Altmanninger <aclopte@gmail.com>\n> Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>\n> ---\n>  apply.c | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/apply.c b/apply.c\n> index 8bbe6ed224..e7856ae6b3 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -174,8 +174,12 @@ int check_apply_state(struct apply_state *state, int force_apply)\n>  \t\t\treturn error(_(\"'%s' outside a repository\"), \"--cached\");\n>  \t\tstate->check_index = 1;\n>  \t}\n> -\tif (state->ita_only && (state->check_index || is_not_gitdir))\n> -\t\tstate->ita_only = 0;\n> +\tif (state->ita_only) {\n> +\t\tif (is_not_gitdir)\n> +\t\t\treturn error(_(\"'%s' outside a repository\"), \"--intent-to-add\");\n> +\t\tif (state->check_index)\n> +\t\t\tstate->ita_only = 0;\n> +\t}\n>  \tif (state->check_index)\n>  \t\tstate->unsafe_paths = 0;\n"},{"id":"520951","messageId":"xmqqbjq512y5.fsf@gitster.g","threadId":"63706","inReplyTo":"20250628225819.1294068-4-ray@ameretat.dev","subject":"Re: [PATCH 2/5] apply: read in the index in --intent-to-add mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-30T18:47:46Z","receivedAt":"2025-06-30T18:47:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n\n> There are three main modes of operation for apply: applying only to the\n> worktree, applying to the worktree and index (--index), and applying\n> only to the index (--cached).\n>\n> The --intent-to-add flag modifies the first of these modes, applying\n> only to the worktree, in a way which touches the index, because\n> intents to add are special index entries. However, it has not ever\n> worked correctly in any but the most trivial (empty repository)\n> cases, because the index was never read in (in apply, this is done\n> in read_apply_cache()) before writing to it.\n\nAs the inventor of \"add -N\", I think what \"apply -N\" does may be\nwrong (only judging from the above description; it's been a while\nsince I really read the code in apply.c).  It does not make any\nsense to write a new index that has only the ITA entries.\n\n> If we merely gate read_apply_cache() behind update_index, then it will\n> not be read when state->apply is false, even if it must be checked.\n> Therefore, we instead read the index if it will be either checked or\n> updated, because reading the index is a prerequisite to either.\n\nMakes sense.\n\n> Reported-by: Ryan Hodges <rhodges@cisco.com>\n> Original-patch-by: Johannes Altmanninger <aclopte@gmail.com>\n> Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>\n> ---\n>  apply.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/apply.c b/apply.c\n> index e7856ae6b3..1757d34618 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -4837,7 +4837,7 @@ static int apply_patch(struct apply_state *state,\n>  \t\t\t\t\t       LOCK_DIE_ON_ERROR);\n>  \t}\n>  \n> -\tif (state->check_index && read_apply_cache(state) < 0) {\n> +\tif ((state->check_index || state->update_index) && read_apply_cache(state) < 0) {\n>  \t\terror(_(\"unable to read index file\"));\n>  \t\tres = -128;\n>  \t\tgoto end;\n\n"},{"id":"520956","messageId":"xmqq7c0t12oz.fsf@gitster.g","threadId":"63706","inReplyTo":"20250628225819.1294068-5-ray@ameretat.dev","subject":"Re: [PATCH 3/5] apply: only write intents to add for new files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-30T18:53:16Z","receivedAt":"2025-06-30T18:53:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n\n> In the \"update only the worktree\" mode, the index should not be touched\n> except to record intents to add when --intent-to-add is on. Because\n> having --intent-to-add on sets update_index, to indicate that we are\n> touching the index, we can't rely only on that flag to decide whether to\n> write an index entry.\n\nDoes that let us inspect state->ita_only alone and conclude that\nstate->update_index is set, though?  IOW ...\n\n>  \tif (patch->conflicted_threeway)\n>  \t\treturn add_conflicted_stages_file(state, patch);\n> -\telse if (state->update_index)\n> +\telse if (state->check_index || (state->ita_only && patch->is_new > 0))\n\n... I would have expected the new code to check not just ita_only but\ncheck ita_only only when update_index is in effect.\n\n\n>  \t\treturn add_index_file(state, path, mode, buf, size);\n>  \treturn 0;\n>  }\n"},{"id":"520981","messageId":"lhhs5j47cnvhp7zcqtcfr725els2dlvoufsp7mexn3isivnpnv@ode3ul6ds3sa","threadId":"63706","inReplyTo":"xmqqfrfh13k5.fsf@gitster.g","subject":"Re: [PATCH 1/5] apply: error on --intent-to-add outside gitdir","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-01T05:26:11Z","receivedAt":"2025-07-01T05:26:39Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On 25/06/30 11:34AM, Junio C Hamano wrote:\n> \"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n> \n> > It makes no sense to register an intent to add outside a repository. We\n> > should error out here.\n> \n> I am not so sure, especially if you still keep the original \"if we\n> are using 'git apply' as a better GNU patch, ignore -N\" logic.\n> \n> Not that I am suggesting to also error out when -N is given without\n> --index or --cached, that is.\n> \n> I think the most problematic is the claim \"it makes no sense\" is not\n> linked to the conclusion \"we should error out\".  The behaviour of\n> the original code, and the behaviour of the code with this patch in\n> \"a better GNU patch\" mode is based on \"it makes no sense in such a\n> context, hence we just ignore (instead of erroring out)\", which\n> makes perfect sense as well.\n\nYeah, perhaps that's right. My thought was that \"the user said -N,\nthey must think they're in a repository\". On the other hand, however,\nit's a lot weaker than saying --index or --cached, which mean the\nuser definitely wants to manipulate a git repository; the user is\nstill /mainly/ trying to manipulate files if they use -N.\n"},{"id":"520986","messageId":"i5mft6pima4ft4gyhluoflktnr22dzl6vc2txqfwvhldecdk5b@rkfdkf2ryci7","threadId":"63706","inReplyTo":"xmqqbjq512y5.fsf@gitster.g","subject":"Re: [PATCH 2/5] apply: read in the index in --intent-to-add mode","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-01T05:32:39Z","receivedAt":"2025-07-01T05:32:44Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On 25/06/30 11:47AM, Junio C Hamano wrote:\n> \"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n> \n> > There are three main modes of operation for apply: applying only to the\n> > worktree, applying to the worktree and index (--index), and applying\n> > only to the index (--cached).\n> >\n> > The --intent-to-add flag modifies the first of these modes, applying\n> > only to the worktree, in a way which touches the index, because\n> > intents to add are special index entries. However, it has not ever\n> > worked correctly in any but the most trivial (empty repository)\n> > cases, because the index was never read in (in apply, this is done\n> > in read_apply_cache()) before writing to it.\n> \n> As the inventor of \"add -N\", I think what \"apply -N\" does may be\n> wrong (only judging from the above description; it's been a while\n> since I really read the code in apply.c).  It does not make any\n> sense to write a new index that has only the ITA entries.\n\nYeah, that's the bug; it writes a new index with just ITA entries (iow,\nthe index thinks every existing file has been deleted); the fix is to\ninstead write ITA entries to the existing index, not make a new one. And\nthe root cause is not having read the index, so it's starting from an\nempty tree.\n"},{"id":"520987","messageId":"vzkxbwj5khnlzx23p2jz2omf4nfzfaal25gwlovvwgzxi2mmjx@vijooguwndsj","threadId":"63706","inReplyTo":"xmqq7c0t12oz.fsf@gitster.g","subject":"Re: [PATCH 3/5] apply: only write intents to add for new files","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-01T05:44:00Z","receivedAt":"2025-07-01T05:44:13Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On 25/06/30 11:53AM, Junio C Hamano wrote:\n> \"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n> \n> > In the \"update only the worktree\" mode, the index should not be touched\n> > except to record intents to add when --intent-to-add is on. Because\n> > having --intent-to-add on sets update_index, to indicate that we are\n> > touching the index, we can't rely only on that flag to decide whether to\n> > write an index entry.\n> \n> Does that let us inspect state->ita_only alone and conclude that\n> state->update_index is set, though?  IOW ...\n> \n> >  \tif (patch->conflicted_threeway)\n> >  \t\treturn add_conflicted_stages_file(state, patch);\n> > -\telse if (state->update_index)\n> > +\telse if (state->check_index || (state->ita_only && patch->is_new > 0))\n> \n> ... I would have expected the new code to check not just ita_only but\n> check ita_only only when update_index is in effect.\n> \n> \n> >  \t\treturn add_index_file(state, path, mode, buf, size);\n> >  \treturn 0;\n> >  }\n\nWe're behind a more specific test on state->apply to enter this codepath\nat all (the only way in is the call to write_out_results around line\n4859). So we're already committed to applying the patch, which is one\nhalf of update_index, but the other half is check_index || ita_only and\nwe must behave differently depending on which of those is in effect and\ncausing us to touch the index.\n"},{"id":"521209","messageId":"20250702212814.1923253-1-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250628225819.1294068-2-ray@ameretat.dev","subject":"[PATCH v2 0/4] fix apply --intent-to-add","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-02T21:26:43Z","receivedAt":"2025-07-02T21:28:36Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"I've rerolled this taking comments into consideration; I've removed the\nfirst patch, because I'm convinced not erroring out is fine here, and\nexpanded on the log messages somewhat. I've also fixed an issue where I\nwas outsmarted by a text editor when writing the test patch.\n\nRaymond E. Pasco (4):\n  apply: read in the index in --intent-to-add mode\n  apply: only write intents to add for new files\n  t4140: test apply --intent-to-add interactions\n  apply docs: clarify wording for --intent-to-add\n\n Documentation/git-apply.adoc |  9 +++++----\n apply.c                      |  4 ++--\n t/t4140-apply-ita.sh         | 31 ++++++++++++++++++++++++++++++-\n 3 files changed, 37 insertions(+), 7 deletions(-)\n\nRange-diff against v1:\n1:  b1c9ea7cac < -:  ---------- apply: error on --intent-to-add outside gitdir\n2:  3a422c8124 ! 1:  71ec291fba apply: read in the index in --intent-to-add mode\n    @@ Commit message\n         cases, because the index was never read in (in apply, this is done\n         in read_apply_cache()) before writing to it.\n     \n    -    If we merely gate read_apply_cache() behind update_index, then it will\n    -    not be read when state->apply is false, even if it must be checked.\n    -    Therefore, we instead read the index if it will be either checked or\n    -    updated, because reading the index is a prerequisite to either.\n    +    This causes the operation to clobber the old, correct index with a\n    +    new empty-tree index before writing intent-to-add entries to this\n    +    empty index; the final result is that the index now records every\n    +    existing file in the repository as deleted, which is incorrect.\n    +\n    +    This error can be corrected by first reading the index. The\n    +    update_index flag is correctly set if ita_only is true, because\n    +    this mode updates the index. However, if we merely gate the call\n    +    to read_apply_cache() behind update_index, then it will not be read\n    +    when state->apply is false, even if it must be checked. Therefore,\n    +    we instead read the index if it will be either checked or updated,\n    +    because reading the index is a prerequisite to either.\n     \n         Reported-by: Ryan Hodges <rhodges@cisco.com>\n         Original-patch-by: Johannes Altmanninger <aclopte@gmail.com>\n3:  df28144f82 ! 2:  5f49ff9035 apply: only write intents to add for new files\n    @@ Commit message\n         touching the index, we can't rely only on that flag to decide whether to\n         write an index entry.\n     \n    -    Instead, we must test whether we are in a mode which updates the\n    -    index, or else are in worktree-only mode with --intent-to-add on and\n    -    the current file being an addition. We do not need to check\n    -    state->apply, because we only enter write_out_results() if state->apply\n    -    is already set.\n    +    Because we have already entered write_out_results() and are performing\n    +    writes, we know that state->apply is true. If state->check_index is\n    +    additionally true, we are in a mode which updates the index and should\n    +    always write, whereas if we are merely in ita_only mode we must only\n    +    write if the patch is a new file creation patch.\n     \n         Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>\n     \n4:  0bbae13b63 ! 3:  f8a6d8032f t4140: test apply --intent-to-add interactions\n    @@ t/t4140-apply-ita.sh: test_description='git apply of i-t-a file'\n      test_expect_success setup '\n      \ttest_write_lines 1 2 3 4 5 >blueprint &&\n      \n    -+  cat blueprint >committed-file &&\n    -+  git add committed-file &&\n    -+  git commit -m \"commit\" &&\n    ++\tcat blueprint >committed-file &&\n    ++\tgit add committed-file &&\n    ++\tgit commit -m \"commit\" &&\n     +\n      \tcat blueprint >test-file &&\n      \tgit add -N test-file &&\n    @@ t/t4140-apply-ita.sh: test_expect_success 'apply deletion patch to ita path (--i\n      '\n      \n     +test_expect_success 'apply creation patch to existing index with -N' '\n    -+  git rm -f test-file &&\n    -+  cat blueprint >index-file &&\n    -+  git add index-file &&\n    -+  git apply -N creation-patch &&\n    ++\tgit rm -f test-file &&\n    ++\tcat blueprint >index-file &&\n    ++\tgit add index-file &&\n    ++\tgit apply -N creation-patch &&\n     +\n    -+  git ls-files --stage --error-unmatch index-file &&\n    -+  git ls-files --stage --error-unmatch test-file\n    ++\tgit ls-files --stage --error-unmatch index-file &&\n    ++\tgit ls-files --stage --error-unmatch test-file\n     +'\n     +\n     +test_expect_success 'apply complex patch with -N' '\n    -+  git rm -f test-file index-file &&\n    -+  git apply -N complex-patch &&\n    ++\tgit rm -f test-file index-file &&\n    ++\tgit apply -N complex-patch &&\n     +\n    -+  git ls-files --stage --error-unmatch test-file &&\n    -+  git diff | grep \"a/committed-file\"\n    ++\tgit ls-files --stage --error-unmatch test-file &&\n    ++\tgit diff | grep \"a/committed-file\"\n     +'\n     +\n      test_done\n5:  970c739ca9 ! 4:  ad42992d03 apply docs: clarify wording for --intent-to-add\n    @@ Documentation/git-apply.adoc: OPTIONS\n     -\tNote that `--index` could be implied by other options such\n     -\tas `--cached` or `--3way`.\n     +\toption in linkgit:git-add[1]). This option is ignored if\n    -+\t`--index` or `--cached` are used. Note that `--index` could\n    -+\tbe implied by other options such as `--3way`.\n    ++\t`--index` or `--cached` are used, and has no effect outside a Git\n    ++\trepository. Note that `--index` could be implied by other options\n    ++\tsuch as `--3way`.\n      \n      -3::\n      --3way::\n-- \n2.50.0.201.gfeb04032fb\n\n"},{"id":"521210","messageId":"20250702212814.1923253-2-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250702212814.1923253-1-ray@ameretat.dev","subject":"[PATCH v2 1/4] apply: read in the index in --intent-to-add mode","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-02T21:26:44Z","receivedAt":"2025-07-02T21:28:37Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"There are three main modes of operation for apply: applying only to the\nworktree, applying to the worktree and index (--index), and applying\nonly to the index (--cached).\n\nThe --intent-to-add flag modifies the first of these modes, applying\nonly to the worktree, in a way which touches the index, because\nintents to add are special index entries. However, it has not ever\nworked correctly in any but the most trivial (empty repository)\ncases, because the index was never read in (in apply, this is done\nin read_apply_cache()) before writing to it.\n\nThis causes the operation to clobber the old, correct index with a\nnew empty-tree index before writing intent-to-add entries to this\nempty index; the final result is that the index now records every\nexisting file in the repository as deleted, which is incorrect.\n\nThis error can be corrected by first reading the index. The\nupdate_index flag is correctly set if ita_only is true, because\nthis mode updates the index. However, if we merely gate the call\nto read_apply_cache() behind update_index, then it will not be read\nwhen state->apply is false, even if it must be checked. Therefore,\nwe instead read the index if it will be either checked or updated,\nbecause reading the index is a prerequisite to either.\n\nReported-by: Ryan Hodges <rhodges@cisco.com>\nOriginal-patch-by: Johannes Altmanninger <aclopte@gmail.com>\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n apply.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex 8bbe6ed224..c8d4517c0a 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4833,7 +4833,7 @@ static int apply_patch(struct apply_state *state,\n \t\t\t\t\t       LOCK_DIE_ON_ERROR);\n \t}\n \n-\tif (state->check_index && read_apply_cache(state) < 0) {\n+\tif ((state->check_index || state->update_index) && read_apply_cache(state) < 0) {\n \t\terror(_(\"unable to read index file\"));\n \t\tres = -128;\n \t\tgoto end;\n-- \n2.50.0.201.gfeb04032fb\n\n"},{"id":"521211","messageId":"20250702212814.1923253-3-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250702212814.1923253-1-ray@ameretat.dev","subject":"[PATCH v2 2/4] apply: only write intents to add for new files","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-02T21:26:45Z","receivedAt":"2025-07-02T21:28:38Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"In the \"update only the worktree\" mode, the index should not be touched\nexcept to record intents to add when --intent-to-add is on. Because\nhaving --intent-to-add on sets update_index, to indicate that we are\ntouching the index, we can't rely only on that flag to decide whether to\nwrite an index entry.\n\nBecause we have already entered write_out_results() and are performing\nwrites, we know that state->apply is true. If state->check_index is\nadditionally true, we are in a mode which updates the index and should\nalways write, whereas if we are merely in ita_only mode we must only\nwrite if the patch is a new file creation patch.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n apply.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex c8d4517c0a..8637ad4c9f 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4565,7 +4565,7 @@ static int create_file(struct apply_state *state, struct patch *patch)\n \n \tif (patch->conflicted_threeway)\n \t\treturn add_conflicted_stages_file(state, patch);\n-\telse if (state->update_index)\n+\telse if (state->check_index || (state->ita_only && patch->is_new > 0))\n \t\treturn add_index_file(state, path, mode, buf, size);\n \treturn 0;\n }\n-- \n2.50.0.201.gfeb04032fb\n\n"},{"id":"521212","messageId":"20250702212814.1923253-4-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250702212814.1923253-1-ray@ameretat.dev","subject":"[PATCH v2 3/4] t4140: test apply --intent-to-add interactions","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-02T21:26:46Z","receivedAt":"2025-07-02T21:28:39Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"Test that applying a new file creation patch to an existing index works,\nand that applying a patch with both modifications and new file creations\nworks.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n t/t4140-apply-ita.sh | 31 ++++++++++++++++++++++++++++++-\n 1 file changed, 30 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh\nindex c614eaf04c..0b11a8aef4 100755\n--- a/t/t4140-apply-ita.sh\n+++ b/t/t4140-apply-ita.sh\n@@ -7,6 +7,10 @@ test_description='git apply of i-t-a file'\n test_expect_success setup '\n \ttest_write_lines 1 2 3 4 5 >blueprint &&\n \n+\tcat blueprint >committed-file &&\n+\tgit add committed-file &&\n+\tgit commit -m \"commit\" &&\n+\n \tcat blueprint >test-file &&\n \tgit add -N test-file &&\n \tgit diff >creation-patch &&\n@@ -14,7 +18,14 @@ test_expect_success setup '\n \n \trm -f test-file &&\n \tgit diff >deletion-patch &&\n-\tgrep \"deleted file mode 100644\" deletion-patch\n+\tgrep \"deleted file mode 100644\" deletion-patch &&\n+\n+\tgit rm -f test-file &&\n+\ttest_write_lines 6 >>committed-file &&\n+\tcat blueprint >test-file &&\n+\tgit add -N test-file &&\n+\tgit diff >complex-patch &&\n+\tgit restore committed-file\n '\n \n test_expect_success 'apply creation patch to ita path (--cached)' '\n@@ -53,4 +64,22 @@ test_expect_success 'apply deletion patch to ita path (--index)' '\n \tgit ls-files --stage --error-unmatch test-file\n '\n \n+test_expect_success 'apply creation patch to existing index with -N' '\n+\tgit rm -f test-file &&\n+\tcat blueprint >index-file &&\n+\tgit add index-file &&\n+\tgit apply -N creation-patch &&\n+\n+\tgit ls-files --stage --error-unmatch index-file &&\n+\tgit ls-files --stage --error-unmatch test-file\n+'\n+\n+test_expect_success 'apply complex patch with -N' '\n+\tgit rm -f test-file index-file &&\n+\tgit apply -N complex-patch &&\n+\n+\tgit ls-files --stage --error-unmatch test-file &&\n+\tgit diff | grep \"a/committed-file\"\n+'\n+\n test_done\n-- \n2.50.0.201.gfeb04032fb\n\n"},{"id":"521213","messageId":"20250702212814.1923253-5-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250702212814.1923253-1-ray@ameretat.dev","subject":"[PATCH v2 4/4] apply docs: clarify wording for --intent-to-add","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-02T21:26:47Z","receivedAt":"2025-07-02T21:28:41Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"Avoid using a double negative, and keep in mind that --index and\n--cached are distinct modes of operation.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n Documentation/git-apply.adoc | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-apply.adoc b/Documentation/git-apply.adoc\nindex 952518b8af..6c71ee69da 100644\n--- a/Documentation/git-apply.adoc\n+++ b/Documentation/git-apply.adoc\n@@ -75,13 +75,14 @@ OPTIONS\n \ttree. If `--check` is in effect, merely check that it would\n \tapply cleanly to the index entry.\n \n+-N::\n --intent-to-add::\n \tWhen applying the patch only to the working tree, mark new\n \tfiles to be added to the index later (see `--intent-to-add`\n-\toption in linkgit:git-add[1]). This option is ignored unless\n-\trunning in a Git repository and `--index` is not specified.\n-\tNote that `--index` could be implied by other options such\n-\tas `--cached` or `--3way`.\n+\toption in linkgit:git-add[1]). This option is ignored if\n+\t`--index` or `--cached` are used, and has no effect outside a Git\n+\trepository. Note that `--index` could be implied by other options\n+\tsuch as `--3way`.\n \n -3::\n --3way::\n-- \n2.50.0.201.gfeb04032fb\n\n"},{"id":"521430","messageId":"20250707121534.2933349-1-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250702212814.1923253-1-ray@ameretat.dev","subject":"[PATCH v3 0/4] fix apply --intent-to-add","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-07T12:12:29Z","receivedAt":"2025-07-07T12:15:49Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"I've rerolled this to considerably expand the commit messages, because\nit took me a while to learn apply.c and a while to relearn it as well,\nso the messages should be as specific as possible to aid the next\nperson.\n\nA note about this series: the code changes (first two patches) apply\nfine to cff5dc09ed, which introduced the feature in a broken state, and\nfix it there. However, that is from 2018, and t4140 did not exist yet at\nthat time (I wrote it in 2020) and the docs have moved around some in\nthe interim as well. If it were a year-old, and not a nearly\nseven-year-old, issue, I would prioritize making it all work there, but\nI'm not sure it's worth adding an entirely new test. Unsure what the\ncorrect thing to do here is.\n\nRaymond E. Pasco (4):\n  apply: read in the index in --intent-to-add mode\n  apply: only write intents to add for new files\n  t4140: test apply --intent-to-add interactions\n  apply docs: clarify wording for --intent-to-add\n\n Documentation/git-apply.adoc |  9 +++++----\n apply.c                      |  4 ++--\n t/t4140-apply-ita.sh         | 31 ++++++++++++++++++++++++++++++-\n 3 files changed, 37 insertions(+), 7 deletions(-)\n\nRange-diff against v2:\n1:  71ec291fba ! 1:  f3120189b3 apply: read in the index in --intent-to-add mode\n    @@ Commit message\n         only to the index (--cached).\n     \n         The --intent-to-add flag modifies the first of these modes, applying\n    -    only to the worktree, in a way which touches the index, because\n    -    intents to add are special index entries. However, it has not ever\n    +    only to the worktree, in a way which touches the index, because intents\n    +    to add are special index entries. However, since its introduction\n    +    in cff5dc09ed (apply: add --intent-to-add, 2018-05-26), it has not\n         worked correctly in any but the most trivial (empty repository)\n    -    cases, because the index was never read in (in apply, this is done\n    -    in read_apply_cache()) before writing to it.\n    +    cases, because the index is never read in (in apply, this is done in\n    +    read_apply_cache()) before writing to it.\n     \n         This causes the operation to clobber the old, correct index with a\n         new empty-tree index before writing intent-to-add entries to this\n    @@ Commit message\n         existing file in the repository as deleted, which is incorrect.\n     \n         This error can be corrected by first reading the index. The\n    -    update_index flag is correctly set if ita_only is true, because\n    -    this mode updates the index. However, if we merely gate the call\n    -    to read_apply_cache() behind update_index, then it will not be read\n    -    when state->apply is false, even if it must be checked. Therefore,\n    -    we instead read the index if it will be either checked or updated,\n    -    because reading the index is a prerequisite to either.\n    +    update_index flag is correctly set if ita_only is true, because this\n    +    flag causes the index to be updated. However, if we merely gate the\n    +    call to read_apply_cache() behind update_index, then it will not be\n    +    read when state->apply is false, even if it must be checked due to\n    +    being in --index or --cached mode. Therefore, we instead read the\n    +    index if it will be either checked or updated, because reading the\n    +    index is a prerequisite to either.\n     \n         Reported-by: Ryan Hodges <rhodges@cisco.com>\n         Original-patch-by: Johannes Altmanninger <aclopte@gmail.com>\n2:  5f49ff9035 ! 2:  907a90d849 apply: only write intents to add for new files\n    @@ Metadata\n      ## Commit message ##\n         apply: only write intents to add for new files\n     \n    -    In the \"update only the worktree\" mode, the index should not be touched\n    -    except to record intents to add when --intent-to-add is on. Because\n    -    having --intent-to-add on sets update_index, to indicate that we are\n    -    touching the index, we can't rely only on that flag to decide whether to\n    -    write an index entry.\n    +    In the \"apply only to files\" mode (i.e., neither --index nor --cached\n    +    mode), the index should not be touched except to record intents to\n    +    add when --intent-to-add is on. Because having --intent-to-add on sets\n    +    update_index, to indicate that we may touch the index, we can't rely\n    +    only on that flag in create_file() (which is called to write both new\n    +    files and updated files) to decide whether to write an index entry;\n    +    if we did, we would write an index entry for every file being patched\n    +    (which would moreover be an intent-to-add entry despite not being a\n    +    new file, because we are going to turn on the CE_INTENT_TO_ADD flag\n    +    in add_index_entry() if we enter it here and ita_only is true).\n     \n    -    Because we have already entered write_out_results() and are performing\n    -    writes, we know that state->apply is true. If state->check_index is\n    -    additionally true, we are in a mode which updates the index and should\n    +    To decide whether to touch the index, we need to check the\n    +    specific reason the index would be updated, rather than merely\n    +    their aggregate in the update_index flag. Because we have already\n    +    entered write_out_results() and are performing writes, we know that\n    +    state->apply is true. If state->check_index is additionally true, we\n    +    are in --index or --cached mode, which updates the index and should\n         always write, whereas if we are merely in ita_only mode we must only\n         write if the patch is a new file creation patch.\n     \n3:  f8a6d8032f ! 3:  b7603de201 t4140: test apply --intent-to-add interactions\n    @@ Metadata\n      ## Commit message ##\n         t4140: test apply --intent-to-add interactions\n     \n    -    Test that applying a new file creation patch to an existing index works,\n    -    and that applying a patch with both modifications and new file creations\n    -    works.\n    +    Test that applying a new file creation patch with --intent-to-add to\n    +    an existing index does not modify the index outside adding the correct\n    +    intents-to-add, and that applying a patch with both modifications\n    +    and new file creations with --intent-to-add correctly only adds\n    +    intents-to-add to the index.\n     \n         Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>\n     \n4:  ad42992d03 = 4:  4c786a77c9 apply docs: clarify wording for --intent-to-add\n-- \n2.50.0.229.gc167f4d905\n\n"},{"id":"521431","messageId":"20250707121534.2933349-2-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250707121534.2933349-1-ray@ameretat.dev","subject":"[PATCH v3 1/4] apply: read in the index in --intent-to-add mode","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-07T12:12:30Z","receivedAt":"2025-07-07T12:15:51Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"There are three main modes of operation for apply: applying only to the\nworktree, applying to the worktree and index (--index), and applying\nonly to the index (--cached).\n\nThe --intent-to-add flag modifies the first of these modes, applying\nonly to the worktree, in a way which touches the index, because intents\nto add are special index entries. However, since its introduction\nin cff5dc09ed (apply: add --intent-to-add, 2018-05-26), it has not\nworked correctly in any but the most trivial (empty repository)\ncases, because the index is never read in (in apply, this is done in\nread_apply_cache()) before writing to it.\n\nThis causes the operation to clobber the old, correct index with a\nnew empty-tree index before writing intent-to-add entries to this\nempty index; the final result is that the index now records every\nexisting file in the repository as deleted, which is incorrect.\n\nThis error can be corrected by first reading the index. The\nupdate_index flag is correctly set if ita_only is true, because this\nflag causes the index to be updated. However, if we merely gate the\ncall to read_apply_cache() behind update_index, then it will not be\nread when state->apply is false, even if it must be checked due to\nbeing in --index or --cached mode. Therefore, we instead read the\nindex if it will be either checked or updated, because reading the\nindex is a prerequisite to either.\n\nReported-by: Ryan Hodges <rhodges@cisco.com>\nOriginal-patch-by: Johannes Altmanninger <aclopte@gmail.com>\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n apply.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex 8bbe6ed224..c8d4517c0a 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4833,7 +4833,7 @@ static int apply_patch(struct apply_state *state,\n \t\t\t\t\t       LOCK_DIE_ON_ERROR);\n \t}\n \n-\tif (state->check_index && read_apply_cache(state) < 0) {\n+\tif ((state->check_index || state->update_index) && read_apply_cache(state) < 0) {\n \t\terror(_(\"unable to read index file\"));\n \t\tres = -128;\n \t\tgoto end;\n-- \n2.50.0.229.gc167f4d905\n\n"},{"id":"521432","messageId":"20250707121534.2933349-3-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250707121534.2933349-1-ray@ameretat.dev","subject":"[PATCH v3 2/4] apply: only write intents to add for new files","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-07T12:12:31Z","receivedAt":"2025-07-07T12:15:53Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"In the \"apply only to files\" mode (i.e., neither --index nor --cached\nmode), the index should not be touched except to record intents to\nadd when --intent-to-add is on. Because having --intent-to-add on sets\nupdate_index, to indicate that we may touch the index, we can't rely\nonly on that flag in create_file() (which is called to write both new\nfiles and updated files) to decide whether to write an index entry;\nif we did, we would write an index entry for every file being patched\n(which would moreover be an intent-to-add entry despite not being a\nnew file, because we are going to turn on the CE_INTENT_TO_ADD flag\nin add_index_entry() if we enter it here and ita_only is true).\n\nTo decide whether to touch the index, we need to check the\nspecific reason the index would be updated, rather than merely\ntheir aggregate in the update_index flag. Because we have already\nentered write_out_results() and are performing writes, we know that\nstate->apply is true. If state->check_index is additionally true, we\nare in --index or --cached mode, which updates the index and should\nalways write, whereas if we are merely in ita_only mode we must only\nwrite if the patch is a new file creation patch.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n apply.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex c8d4517c0a..8637ad4c9f 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4565,7 +4565,7 @@ static int create_file(struct apply_state *state, struct patch *patch)\n \n \tif (patch->conflicted_threeway)\n \t\treturn add_conflicted_stages_file(state, patch);\n-\telse if (state->update_index)\n+\telse if (state->check_index || (state->ita_only && patch->is_new > 0))\n \t\treturn add_index_file(state, path, mode, buf, size);\n \treturn 0;\n }\n-- \n2.50.0.229.gc167f4d905\n\n"},{"id":"521434","messageId":"20250707121534.2933349-4-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250707121534.2933349-1-ray@ameretat.dev","subject":"[PATCH v3 3/4] t4140: test apply --intent-to-add interactions","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-07T12:12:32Z","receivedAt":"2025-07-07T12:15:55Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"Test that applying a new file creation patch with --intent-to-add to\nan existing index does not modify the index outside adding the correct\nintents-to-add, and that applying a patch with both modifications\nand new file creations with --intent-to-add correctly only adds\nintents-to-add to the index.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n t/t4140-apply-ita.sh | 31 ++++++++++++++++++++++++++++++-\n 1 file changed, 30 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh\nindex c614eaf04c..0b11a8aef4 100755\n--- a/t/t4140-apply-ita.sh\n+++ b/t/t4140-apply-ita.sh\n@@ -7,6 +7,10 @@ test_description='git apply of i-t-a file'\n test_expect_success setup '\n \ttest_write_lines 1 2 3 4 5 >blueprint &&\n \n+\tcat blueprint >committed-file &&\n+\tgit add committed-file &&\n+\tgit commit -m \"commit\" &&\n+\n \tcat blueprint >test-file &&\n \tgit add -N test-file &&\n \tgit diff >creation-patch &&\n@@ -14,7 +18,14 @@ test_expect_success setup '\n \n \trm -f test-file &&\n \tgit diff >deletion-patch &&\n-\tgrep \"deleted file mode 100644\" deletion-patch\n+\tgrep \"deleted file mode 100644\" deletion-patch &&\n+\n+\tgit rm -f test-file &&\n+\ttest_write_lines 6 >>committed-file &&\n+\tcat blueprint >test-file &&\n+\tgit add -N test-file &&\n+\tgit diff >complex-patch &&\n+\tgit restore committed-file\n '\n \n test_expect_success 'apply creation patch to ita path (--cached)' '\n@@ -53,4 +64,22 @@ test_expect_success 'apply deletion patch to ita path (--index)' '\n \tgit ls-files --stage --error-unmatch test-file\n '\n \n+test_expect_success 'apply creation patch to existing index with -N' '\n+\tgit rm -f test-file &&\n+\tcat blueprint >index-file &&\n+\tgit add index-file &&\n+\tgit apply -N creation-patch &&\n+\n+\tgit ls-files --stage --error-unmatch index-file &&\n+\tgit ls-files --stage --error-unmatch test-file\n+'\n+\n+test_expect_success 'apply complex patch with -N' '\n+\tgit rm -f test-file index-file &&\n+\tgit apply -N complex-patch &&\n+\n+\tgit ls-files --stage --error-unmatch test-file &&\n+\tgit diff | grep \"a/committed-file\"\n+'\n+\n test_done\n-- \n2.50.0.229.gc167f4d905\n\n"},{"id":"521433","messageId":"20250707121534.2933349-5-ray@ameretat.dev","threadId":"63706","inReplyTo":"20250707121534.2933349-1-ray@ameretat.dev","subject":"[PATCH v3 4/4] apply docs: clarify wording for --intent-to-add","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2025-07-07T12:12:33Z","receivedAt":"2025-07-07T12:15:56Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"Avoid using a double negative, and keep in mind that --index and\n--cached are distinct modes of operation.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\n Documentation/git-apply.adoc | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-apply.adoc b/Documentation/git-apply.adoc\nindex 952518b8af..6c71ee69da 100644\n--- a/Documentation/git-apply.adoc\n+++ b/Documentation/git-apply.adoc\n@@ -75,13 +75,14 @@ OPTIONS\n \ttree. If `--check` is in effect, merely check that it would\n \tapply cleanly to the index entry.\n \n+-N::\n --intent-to-add::\n \tWhen applying the patch only to the working tree, mark new\n \tfiles to be added to the index later (see `--intent-to-add`\n-\toption in linkgit:git-add[1]). This option is ignored unless\n-\trunning in a Git repository and `--index` is not specified.\n-\tNote that `--index` could be implied by other options such\n-\tas `--cached` or `--3way`.\n+\toption in linkgit:git-add[1]). This option is ignored if\n+\t`--index` or `--cached` are used, and has no effect outside a Git\n+\trepository. Note that `--index` could be implied by other options\n+\tsuch as `--3way`.\n \n -3::\n --3way::\n-- \n2.50.0.229.gc167f4d905\n\n"},{"id":"521457","messageId":"xmqqcyab51pr.fsf@gitster.g","threadId":"63706","inReplyTo":"20250707121534.2933349-1-ray@ameretat.dev","subject":"Re: [PATCH v3 0/4] fix apply --intent-to-add","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-07T17:51:12Z","receivedAt":"2025-07-07T17:51:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n\n> I've rerolled this to considerably expand the commit messages, because\n> it took me a while to learn apply.c and a while to relearn it as well,\n> so the messages should be as specific as possible to aid the next\n> person.\n\nThanks.  Will queue.\n"}]}