> > +If `--keep-cherry-pick is given`, all commits (including these) will be
> > +re-applied. This allows rebase to forgo reading all upstream commits,
> > +potentially improving performance.
>
> I'm slightly worried that "keep" is setting up an incorrect
> expectation for users; in most cases, a reapplied cherry-pick will
> result in the merge machinery applying no new changes (they were
> already applied) and then rebase's default of dropping commits which
> become empty will kick in and drop the commit.
>
> Maybe the name is fine and we just need to be more clear in the text
> on the expected behavior and advantages and disadvantages of this
> option:
>
> If `--keep-cherry-picks` is given, all commits (including these) will be
> re-applied. Note that cherry picks are likely to result in no changes
> when being reapplied and thus are likely to be dropped anyway (assuming
> the default --empty=drop behavior). The advantage of this option, is it
> allows rebase to forgo reading all upstream commits, potentially
> improving performance. The disadvantage of this option is that in some
> cases, the code has drifted such that reapplying a cherry-pick is not
> detectable as a no-op, and instead results in conflicts for the user to
> manually resolve (usually via `git rebase --skip`).
>
> It may also be helpful to prevent users from making a false inference
> by renaming these options to --[no-]reapply-cherry-pick[s]. Sorry to
> bring this up so late after earlier saying --[no-]keep-cherry-pick[s]
> was fine; didn't occur to me then. If you want to keep the name, the
> extended paragraph should be good enough.
Sorry for getting back to this so late. After some thought, I'm liking
--reapply-cherry-picks too. Perhaps documented like this:
Reapply all clean cherry-picks of any upstream commit instead of
dropping them. (If these commits then become empty after rebasing,
because they contain a subset of already upstream changes, the
behavior towards them is controlled by the `--empty` flag.)
By default (or if `--noreapply-cherry-picks` is given), these commits
will be automatically dropped. Because this necessitates reading all
upstream commits, this can be expensive in repos with a large number
of upstream commits that need to be read.
`--reapply-cherry-picks` allows rebase to forgo reading all upstream
commits, potentially improving performance.
See also INCOMPATIBLE OPTIONS below.
This also makes me realize that we probably need to change the "--empty"
documentation too. Maybe:
--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
+ How to handle commits that become
empty after rebasing (because they contain a subset of already
upstream changes). With drop (the default), commits that
become empty are dropped. With keep, such commits are kept.
With ask (implied by --interactive), the rebase will halt when
an empty commit is applied allowing you to choose whether to
drop it, edit files more, or just commit the empty changes.
Other options, like --exec, will use the default of drop unless
-i/--interactive is explicitly specified.
+
-Note that commits which start empty are kept, and commits which are
-clean cherry-picks (as determined by `git log --cherry-mark ...`) are
-always dropped.
+Commits that start empty are always kept.
++
+Commits that are clean cherry-picks of any upstream commit (as determined by
+`git log --cherry-mark ...`) are always dropped, unless
+`--reapply-cherry-picks`, is set, in which case they are reapplied. If they
+become empty after rebasing, `--empty` determines what happens to them.
+
See also INCOMPATIBLE OPTIONS below.
If this works, I'll send out a new version containing Elijah's patches
and mine in whatever branch my patch shows up in [1].
[1] https://lore.kernel.org/git/xmqqd08fhvx5.fsf@xxxxxxxxxxxxxxxxxxxxxx/
> > @@ -568,6 +583,9 @@ In addition, the following pairs of options are incompatible:
> > * --keep-base and --onto
> > * --keep-base and --root
> >
> > +Also, the --keep-cherry-pick option requires the use of the merge backend
> > +(e.g., through --merge).
>
> Why not just list --keep-cherry-pick[s] in the list of options that
> require use of the merge backend (i.e. the list containing '--merge')
> instead of adding another sentence here?
My reading of the list containing "--merge" is that they *trigger* the
merge backend, not require the merge backend. My new option requires but
does not trigger it (unless we want to change it to do so, which I'm
fine with).
