Does extending `--empty` to git-cherry-pick make sense?

The `--empty=(keep|drop|ask)` option added to git-rebase in e98c4269c86019bfe057a91b4305f784365b6f0b seems like it would be applicable to git-cherry-pick (and maybe git-revert for completeness?). While the shared sequencer code likely would already handle this fairly well (at a cursory glance from someone with very little knowledge of C or git's codebase, admittedly), it's only exposed via git-rebase.

The use case in which this came up involves a semi-automated workflow for moving commits between branches. At a high level, a tool is:

- Identifying commits to be picked based on a specific trailer value
- Using git-cherry-pick with `-x` to pick those commits
- Relying on the presence of the `(cherry picked from commit ...)`
  trailer to avoid re-picking commits that have already been picked

If the picked commits are then rewritten in the upstream such that there are squashes or fixups, that trailer can end up missing in the upstream. The next time the tool runs, it will end up trying to re-pick commits that are already represented there. As a result, those commits become empty upon being picked a second time and the cherry-pick ends up breaking for the user to resolve:

    $ git cherry-pick main
    On branch feature
    You are currently cherry-picking commit cfd7cd9.
      (all conflicts fixed: run "git cherry-pick --continue")
      (use "git cherry-pick --skip" to skip this patch)
      (use "git cherry-pick --abort" to cancel the cherry-pick operation)

    nothing to commit, working tree clean
    The previous cherry-pick is now empty, possibly due to conflict resolution.
    If you wish to commit it anyway, use:

        git commit --allow-empty

    Otherwise, please use 'git cherry-pick --skip'

I'll spare the details, but we do expect this to happen with enough frequency that we'd really like to be able to prevent it. The `--empty=drop` option sounds like exactly what we want here:

    --empty=(drop|keep|ask)
    How to handle commits that are not empty to start and are not
    clean cherry-picks of any upstream commit, but which become
    empty after rebasing (because they contain a subset of already
    upstream changes). With drop (the default), commits that become
    empty are dropped.

Is there any real barrier to exposing that option to git-cherry-pick as well? Was this an oversight, or intentionally left out? The corresponding commit message doesn't seem to indicate any specific reason for limiting it to git-rebase.

Thanks, -Brian Lyles

[+cc Elijah]

On Thu, Jan 04, 2024 at 12:57:18AM -0600, Brian Lyles wrote:

I am not nearly as familiar with this code as Elijah is, but this certainly appears possible by setting the `drop_redundant_commits` and `keep_redundant_commits` flags in the replay_opts struct.

I don't see any fundamental reason why cherry-pick shouldn't have the same functionality.

Thanks, Taylor

On Thu, Jan 4, 2024 at 11:33 AM Taylor Blau <me@ttaylorr.com> wrote:

I was indeed focused on simply getting the multiple rebase backends to have consistent behavior (we had like 4-5 backends at the time, right?) and just didn't consider cherry-pick at the time. Now that those are more consistent (though there's still work to be done on that end too), cherry-pick and rebase really ought to share a lot more between each other, from command line flags, to temporary control files written, to more shared code paths. Adding an --empty flag to cherry-pick as a step along the way makes sense.

Note that git-am also gained a similar flag in the meantime, but changed the names slightly: --empty=(stop,drop,keep). I think 'stop' is a better name than 'ask', and we really should make rebase suggest 'stop' instead (but keep 'ask' as a synonym for 'stop', for backwards compatibility). Also, I kind of want to replace 'keep' with 'roll' in the --empty option for both git-am and git-rebase, while keeping 'keep' as a synonym for 'roll'. But I'm not sure if others on the list would go along with it...

On Thu, Jan 4, 2024 at 8:28 PM Elijah Newren <newren@gmail.com> wrote:

I appreciate the insight from both of you.

It sounds like I'll be diving into some C code for the first time in over a decade, then! I'll plan to take a crack at this soon.

-Brian Lyles