Re: [PATCH 1/2] t3507: pin CHERRY_PICK_HEAD absence for a conflicting --no-commit
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 3, 2026, 21:32 UTC
- Message-ID
- <xmqq7bl29g2p.fsf@gitster.g>
- In-Reply-To
- <20260903125524.67889-1-f@lex.la>
Aleksei Sviridkin <f@lex.la> writes:
Show 9 quoted lines
> The tests here check the ref after a conflicting pick, after a clean > pick and after a clean pick under --no-commit, but not after a > conflicting one under --no-commit. That is the combination a user > runs into by accident: the pick stops with conflicts, and the ref > "git commit" would take the authorship from is not there. > > Pin it next to its siblings. Letting the ref be written under > --no-commit when the pick conflicts leaves the rest of the cherry-pick > tests green, so nothing else guards that path.
It is not apparent what problem, if any, the description above claims the commit addresses. Nor is it clear why checking these combinations is relevant.
I also fail to parse what the second paragraph intends to say. What does "it" refer to in "Pin it next to its siblings"? A new test? Any test inserted into a sequence will naturally sit adjacent to its neighbors, so calling them "its siblings" offers little clue to help the reader understand the change.
Can you help me understand the above two paragraphs a bit better?
Thanks.
P.S.
I shamelessly asked an AI agent I had nearby to guess what your log message might have meant and got the following. I am not sure if that matches what you wanted to say, or if it is totally off the mark, but at least I can follow what it is trying to say, even though I do not think if that matches reality (for example, when "--no-commit" is in effect, we probably do not want CHERRY_PICK_HEAD, even though the version of the text given by Gemini below claims it is needed).
When a cherry-pick is run with the --no-commit option and halts
due to conflicts, Git must still write the CHERRY_PICK_HEAD ref.
This ref is necessary because a subsequent "git commit" relies
on it to preserve the authorship metadata of the original
commit. Add a new test alongside the existing cherry-pick tests to
verify this behavior. The test suite currently checks for
CHERRY_PICK_HEAD after a conflicting pick, after a clean pick,
and after a clean pick with --no-commit. However, it lacks
coverage for a conflicting pick with --no-commit. Indeed, if
Git is modified to stop writing the ref in this specific
scenario, all existing tests still pass. This new test closes
the coverage gap.Show 26 quoted lines
> > Assisted-by: LLM > Signed-off-by: Aleksei Sviridkin <f@lex.la> > --- > t/t3507-cherry-pick-conflict.sh | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh > index 44596cb1e8..2ce2e88184 100755 > --- a/t/t3507-cherry-pick-conflict.sh > +++ b/t/t3507-cherry-pick-conflict.sh > @@ -100,6 +100,12 @@ test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' ' > test_must_fail git rev-parse --verify CHERRY_PICK_HEAD > ' > > +test_expect_success 'failed cherry-pick --no-commit does not set CHERRY_PICK_HEAD' ' > + pristine_detach initial && > + test_must_fail git cherry-pick --no-commit picked && > + test_must_fail git rev-parse --verify CHERRY_PICK_HEAD > +' > + > test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' ' > pristine_detach initial && > echo foo >foo && > > base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc