From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 06 Jul 2026 10:08:18 GMT
Subject: Re: [PATCH] rebase -i: introduce `pick -x` to add "cherry picked from commit ..."
Message-ID: <5d238e0d-18ba-429a-a9a4-a3988b00e1e1@gmail.com>
In-Reply-To: <20260706002415.GC2301945@coredump.intra.peff.net>

On 06/07/2026 01:24, Jeff King wrote:
> On Sun, Jul 05, 2026 at 02:09:06PM +0000, Trevor Gross wrote:
> 
>> It is sometimes useful to do cherry picks via rebases when there is a
>> sequence of picks or other git operations to combine. However, there is
>> no interactive rebase equivalent to the cherry-pick `-x` flag, which
>> adds a line to the commit body indicating the original commit.
>>
>> Using `exec git cherry-pick ... -x` does work, but is not as nice
>> because it interrupts rebase flow; after resolving a conflict, both `git
>> cherry-pick --continue` and `git rebase --continue` must be run.
> 
> To me this feels like you're approaching the problem backwards. Mostly
> because rebase and cherry-pick are _kind of_ the same operation.
> 
> Usually a rebase is about rewriting the commits on a new base so that
> you can throw away the old ones. And that's why git-rebase generally
> rewrites the branch you're on, and replaces those old commits. So adding
> a "cherry-picked from..." annotation doesn't make sense there; nobody
> would have those old commits!

Exactly

> And so while cherry-pick is doing roughly the same thing under the hood,
> it has different defaults: you specify a read-only source from which to
> pick the commits (and "-x" may or may not make sense).
> 
> So I can see why you might use git-rebase to do what is essentially a
> cherry-pick, porting options from cherry-pick to rebase feels weird. Why
> can't we fix the problems in cherry-pick that make you want to use
> rebase instead?

I think that would be a better solution. Trevor - what is missing from 
"git cherry-pick" that means you end up using "git rebase" instead?

> So what I'm wondering specifically: have we done 99% of the work to have
> interactive cherry-pick, and we just need to add a "-i" option to let
> the user edit that todo file before we start executing it?
> 
> To be clear, I don't know the answer. It's been ages since I've looked
> at sequencer code, so there might be more gotchas. That's just my gut
> feeling from a high level after reading your message.

I don't think it would be much work. The code that edits the todo list 
is rebase specific because it deals with rebase.missingCommitsCheck but 
it shouldn't be too difficult to generalize it. I do wonder though if it 
makes sense to support all of the usual commands when cherry-picking 
especially with `-x`. In particular I'm not sure about adding support 
for `edit -x`, or for `pick -x` followed by `fixup` - what does the 
trailer mean when the commit has been edited or fixed up? (though if 
you're back-porting bug fixes I guess some degree of editing is inevitable)

On a slight tangent I've sometimes wanted to be able to do

	git cherry-pick --exec 'make test' some commits

>> To improve this, introduce `-x` to the pick, reword, and edit todo
>> rebase commands.  This uses the same logic as cherry-pick to add a
>> "(cherry picked from commit ...)" note to the commit body.
> 
> There is one thing that differs here from how cherry-pick works. Even
> though cherry-pick is using the sequencer under the hood, it does not
> allow individual "pick -x" commands, but instead records it as an option
> for the whole operation. So if you add "-x" to the conflicting
> cherry-pick above, you can see:
> 
>    $ cat .git/sequencer/opts
>    [options]
> 	record-origin = true
> 
> That's less flexible, since you can't have per-pick "-x" behavior. If
> that's important to you, I think it might be reasonable to support the
> "-x" option for those sequencer commands, and have "cherry-pick -x" just
> add it automatically to each line (rather than record the global
> option).

Yes, if we're adding a per-commit flag to record the origin it would be 
much nicer just to set that flag when we build the todo list rather than 
having to do

	if (opt->record_origin || (item->flags &  TODO_RECORD_ORIGIN))

to see whether we need to add the trailer.

>> Of note is that rebase will fastforward wherever possible, meaning the
>> check for TODO_RECORD_ORIGIN doesn't get hit and the message will not
>> get amended. This differs from the cherry-pick logic, which will add
>> "cherry picked from ..." even if a rewrite isn't otherwise necessary.
> 
> This sounds like another case where cherry-pick and rebase have subtly
> different behaviors, even though the core functionality is still "pick
> these commits". So being able to stick to the cherry-pick command for
> cherry-picking may be preferable.

I think that is a consequence of the way this patch is implemented - it 
adds the new per-commit flag but does not change the conditions for 
preventing a fast-forward in do_pick_commit() or skip_unnecessary_picks().

Thanks

Phillip