Re: [PATCH 1/4] sequencer: Do not require `allow_empty` for redundant commit options

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jan 24, 2024, 11:01 UTC
Message-ID: <ec05c0e9-20ad-42ab-8858-cc4ae0847d5a@gmail.com>
In-Reply-To: <7bf5036b-9f55-4451-a13c-8a2c815dfbb7@gmail.com>

Hi Brian

On 23/01/2024 14:23, Phillip Wood wrote:

Show 9 quoted lines

> rebase always sets "opts->allow_empty = 1" in 
> builtin/rebase.c:get_replay_opts() and if the user passes 
> --no-keep-empty drops commits that start empty from the list of commits 
> to be picked. This is slightly confusing but is more efficient as we 
> don't do waste time trying to pick a commit we're going to drop. Can we 
> do something similar for "git cherry-pick"? When cherry-picking a 
> sequence of commits I think it should just work because the code is 
> shared with rebase, for a single commit we'd need to add a test to see 
> if it is empty in single_pick() before calling pick_commits().

Having thought about this again I don't think we can reuse the same approach as rebase because cherry-pick and rebase have different behaviors. "git rebase --no-keep-empty" drops empty commits whereas "git cherry-pick" wants to error out if it sees an empty commit. So I think your approach of reworking allow_empty() in the sequencer is the right approach.

Sorry for the confusion

Phillip