From: "Raymond E. Pasco" <ray@ameretat.dev>
To: ray@ameretat.dev
Cc: aclopte@gmail.com, git@vger.kernel.org, gitster@pobox.com,
jason11choca@proton.me, kristofferhaugsbakk@fastmail.com,
rhodges@cisco.com
Subject: [PATCH v3 0/4] fix apply --intent-to-add
Date: Mon, 7 Jul 2025 08:12:29 -0400 [thread overview]
Message-ID: <20250707121534.2933349-1-ray@ameretat.dev> (raw)
In-Reply-To: <20250702212814.1923253-1-ray@ameretat.dev>
I've rerolled this to considerably expand the commit messages, because
it took me a while to learn apply.c and a while to relearn it as well,
so the messages should be as specific as possible to aid the next
person.
A note about this series: the code changes (first two patches) apply
fine to cff5dc09ed, which introduced the feature in a broken state, and
fix it there. However, that is from 2018, and t4140 did not exist yet at
that time (I wrote it in 2020) and the docs have moved around some in
the interim as well. If it were a year-old, and not a nearly
seven-year-old, issue, I would prioritize making it all work there, but
I'm not sure it's worth adding an entirely new test. Unsure what the
correct thing to do here is.
Raymond E. Pasco (4):
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 | 9 +++++----
apply.c | 4 ++--
t/t4140-apply-ita.sh | 31 ++++++++++++++++++++++++++++++-
3 files changed, 37 insertions(+), 7 deletions(-)
Range-diff against v2:
1: 71ec291fba ! 1: f3120189b3 apply: read in the index in --intent-to-add mode
@@ Commit message
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
+ only to the worktree, in a way which touches the index, because intents
+ to add are special index entries. However, since its introduction
+ in cff5dc09ed (apply: add --intent-to-add, 2018-05-26), it has not
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.
+ cases, because the index is never read in (in apply, this is done in
+ read_apply_cache()) before writing to it.
This causes the operation to clobber the old, correct index with a
new empty-tree index before writing intent-to-add entries to this
@@ Commit message
existing file in the repository as deleted, which is incorrect.
This error can be corrected by first reading the index. The
- update_index flag is correctly set if ita_only is true, because
- this mode updates the index. However, if we merely gate the call
- to read_apply_cache() behind update_index, then it will not be read
- when state->apply is false, even if it must be checked. Therefore,
- we instead read the index if it will be either checked or updated,
- because reading the index is a prerequisite to either.
+ update_index flag is correctly set if ita_only is true, because this
+ flag causes the index to be updated. However, if we merely gate the
+ call to read_apply_cache() behind update_index, then it will not be
+ read when state->apply is false, even if it must be checked due to
+ being in --index or --cached mode. Therefore, we instead read the
+ index if it will be either checked or updated, because reading the
+ index is a prerequisite to either.
Reported-by: Ryan Hodges <rhodges@cisco.com>
Original-patch-by: Johannes Altmanninger <aclopte@gmail.com>
2: 5f49ff9035 ! 2: 907a90d849 apply: only write intents to add for new files
@@ Metadata
## Commit message ##
apply: only write intents to add for new files
- 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.
+ In the "apply only to files" mode (i.e., neither --index nor --cached
+ 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 may touch the index, we can't rely
+ only on that flag in create_file() (which is called to write both new
+ files and updated files) to decide whether to write an index entry;
+ if we did, we would write an index entry for every file being patched
+ (which would moreover be an intent-to-add entry despite not being a
+ new file, because we are going to turn on the CE_INTENT_TO_ADD flag
+ in add_index_entry() if we enter it here and ita_only is true).
- Because we have already entered write_out_results() and are performing
- writes, we know that state->apply is true. If state->check_index is
- additionally true, we are in a mode which updates the index and should
+ To decide whether to touch the index, we need to check the
+ specific reason the index would be updated, rather than merely
+ their aggregate in the update_index flag. Because we have already
+ entered write_out_results() and are performing writes, we know that
+ state->apply is true. If state->check_index is additionally true, we
+ are in --index or --cached mode, which updates the index and should
always write, whereas if we are merely in ita_only mode we must only
write if the patch is a new file creation patch.
3: f8a6d8032f ! 3: b7603de201 t4140: test apply --intent-to-add interactions
@@ Metadata
## Commit message ##
t4140: test apply --intent-to-add interactions
- 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.
+ Test that applying a new file creation patch with --intent-to-add to
+ an existing index does not modify the index outside adding the correct
+ intents-to-add, and that applying a patch with both modifications
+ and new file creations with --intent-to-add correctly only adds
+ intents-to-add to the index.
Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
4: ad42992d03 = 4: 4c786a77c9 apply docs: clarify wording for --intent-to-add
--
2.50.0.229.gc167f4d905
next prev parent reply other threads:[~2025-07-07 12:15 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-28 22:52 [PATCH 0/5] fix apply --intent-to-add Raymond E. Pasco
2025-06-28 22:52 ` [PATCH 1/5] apply: error on --intent-to-add outside gitdir Raymond E. Pasco
2025-06-30 18:34 ` Junio C Hamano
2025-07-01 5:26 ` Raymond E. Pasco
2025-06-28 22:52 ` [PATCH 2/5] apply: read in the index in --intent-to-add mode Raymond E. Pasco
2025-06-30 18:47 ` Junio C Hamano
2025-07-01 5:32 ` Raymond E. Pasco
2025-06-28 22:52 ` [PATCH 3/5] apply: only write intents to add for new files Raymond E. Pasco
2025-06-30 18:53 ` Junio C Hamano
2025-07-01 5:44 ` Raymond E. Pasco
2025-06-28 22:52 ` [PATCH 4/5] t4140: test apply --intent-to-add interactions Raymond E. Pasco
2025-06-28 22:52 ` [PATCH 5/5] apply docs: clarify wording for --intent-to-add Raymond E. Pasco
2025-06-29 3:10 ` [PATCH 0/5] fix apply --intent-to-add Lidong Yan
2025-06-30 0:56 ` Raymond E. Pasco
2025-07-02 21:26 ` [PATCH v2 0/4] " Raymond E. Pasco
2025-07-02 21:26 ` [PATCH v2 1/4] apply: read in the index in --intent-to-add mode Raymond E. Pasco
2025-07-02 21:26 ` [PATCH v2 2/4] apply: only write intents to add for new files Raymond E. Pasco
2025-07-02 21:26 ` [PATCH v2 3/4] t4140: test apply --intent-to-add interactions Raymond E. Pasco
2025-07-02 21:26 ` [PATCH v2 4/4] apply docs: clarify wording for --intent-to-add Raymond E. Pasco
2025-07-07 12:12 ` Raymond E. Pasco [this message]
2025-07-07 12:12 ` [PATCH v3 1/4] apply: read in the index in --intent-to-add mode Raymond E. Pasco
2025-07-07 12:12 ` [PATCH v3 2/4] apply: only write intents to add for new files Raymond E. Pasco
2025-07-07 12:12 ` [PATCH v3 3/4] t4140: test apply --intent-to-add interactions Raymond E. Pasco
2025-07-07 12:12 ` [PATCH v3 4/4] apply docs: clarify wording for --intent-to-add Raymond E. Pasco
2025-07-07 17:51 ` [PATCH v3 0/4] fix apply --intent-to-add Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250707121534.2933349-1-ray@ameretat.dev \
--to=ray@ameretat.dev \
--cc=aclopte@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jason11choca@proton.me \
--cc=kristofferhaugsbakk@fastmail.com \
--cc=rhodges@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).