Re: [PATCH 00/11] sequencer: do not record dropped commits as rewritten

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 30, 2026, 19:57 UTC
Message-ID: <xmqqpl17rec3.fsf@gitster.g>
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 41 quoted lines

> On 19/06/2026 11:13, Phillip Wood wrote:
>> I'm happy to take this forward and try and fix at least some of the
>> other bugs I've listed above. Uwe - if I don't cc you on some patches
>> within the next couple of weeks please feel free to send a reminder.
>
> Here is the first batch that fixes the same problem as Uwe's patch. I've
> taken a slightly different approach that uses the return value from
> do_pick_commit() to signal that a commit was dropped rather than
> adding another function argument. That involves a number of preparatory
> patches, but they are hopefully reasonably small and easy to follow.
>
> If a commit gets dropped because its changes are already upstream
> then we should not record it as rewritten. As well as confusing any
> post-rewrite hooks this means we end up copying the notes from the
> dropped commit to the commit that was picked immediately before the
> one that was dropped.
>
> This series is structured as follows:
>
> Patch 1 restores some test coverage that was lost when the default
> rebase backend was changed.
>
> Patch 2 moves a function so it can be called without a forward
> declaration in Patch 11.
>
> Patches 3 & 4 fix the return value of do_pick_commit() when an external
> command fails (this is in preparation for patch 10).
>
> Patches 5-9 try and simplify the control flow in pick_one_commit()
> in preparation for patch 10.
>
> Patch 10 changes the return type of do_pick_commit() to an enum.
>
> Patch 11 adds a new member to the enum from patch 10 for commits that
> are dropped when they become empty and uses that to stop them from
> being recorded as rewritten.
>
> Base-Commit: 6c3d7b73556db708feb3b16232fab1efc4353428
> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Frebase-drop-notes-with-commit%2Fv1
> View-Changes-At: https://github.com/phillipwood/git/compare/6c3d7b735...26551f268
> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/rebase-drop-notes-with-commit/v1

Thanks.

A tangent (I Cc'ed Konstantin for this), but

    $ b4 am -o- '<cover.1782833268.git.phillip.wood@dunelm.org.uk>' >b4am.mbx

failed to produce a usable mailbox. It somehow did not think [2/11] existed. I manually examined the References and In-Reply-To headers of that particular message and compared them with those from other messages but did not find anything suspicious X-<.

I have a bunch of typofixes queued on top of these 11 patches (made with "git commit --fixup reword:<sha1>"); please double check when you reroll after seeing more substantial reviews than mere typofixes, possibly from others.

Thanks.

Here is the transcript of failed b4 am invocation.
---- >8 ----
Looking up https://lore.kernel.org/all/cover.1782833268.git.phillip.wood@dunelm.org.uk/
Grabbing thread from lore.kernel.org/all/cover.1782833268.git.phillip.wood@dunelm.org.uk/t.mbox.gz
Analyzing 17 messages in the thread
WARNING: duplicate messages found at index 1
   Subject 1: sequencer: Skip copying notes for commits that disappear during rebase
   Subject 2: t3400: restore coverage for note copying with apply backend
  2 is not a reply... assume additional patch
Looking for additional code-review trailers on lore.kernel.org
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
  ✗ [PATCH] sequencer: Skip copying notes for commits that disappear during rebase
    ✗ No key: openpgp/u.kleine-koenig@baylibre.com
    ✗ BADSIG: DKIM/baylibre.com
  ✓ [PATCH 1/11] t3400: restore coverage for note copying with apply backend
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 3/11] sequencer: be more careful with external merge
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 4/11] sequencer: never reschedule on failed commit
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 5/11] sequencer: remove unnecessary "or" in pick_one_commit()
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 6/11] sequencer: simplify handing of fixup with conflicts
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 7/11] sequencer: remove unnecessary condition in pick_one_commit()
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 8/11] sequencer: simplify pick_one_commit()
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 9/11] sequencer: return early from pick_one_commit() on success
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 10/11] sequencer: use an enum to represent result of picking a commit
    ✓ Signed: DKIM/gmail.com
  ✓ [PATCH 11/11] sequencer: do not record dropped commits as rewritten
    ✓ Signed: DKIM/gmail.com
  ERROR: missing [12/2]!
---
Total patches: 11
---
WARNING: Thread incomplete!
 Link: https://patch.msgid.link/cover.1782833268.git.phillip.wood@dunelm.org.uk
: