Re: [BUG] multi-commit cherry-pick messes up the order of commits

[Jeff King <peff@peff.net>]

On Fri, Jan 13, 2012 at 12:55:58AM +0530, Ramkumar Ramachandra wrote:

> Junio C Hamano wrote:
> > You should be able to look at revs->cmdline and tell if you need to let
> > cherry-pick walk (i.e. "cherry-pick master..next"), or if the user wants
> > individual commits (i.e. "cherry-pick A B C").
> >
> > And you do prepare_revision_walk() only when you need to walk; otherwise
> > you use the contents of revs->pending in order.

I am tempted to suggest that cherry-pick should not feed its arguments
to the revision machinery in the first place, but instead accept a set
of arguments, each argument of which is either a single commit
(interpreted by get_sha1) or a range specifier (which can be fed to
setup-revisions). And then get a linearized set of commits for _each_
argument independently and concatenate them (possibly eliminating
duplicates). That would make all of these work as most people would
expect:

  git cherry-pick A B C
  git cherry-pick A..B
  git cherry-pick A..B B..C

but would be a regression for:

  git cherry-pick B ^A

versus the current code. I suspect that the latter form is not all that
commonly used, though, and certainly I would accept it as a casualty of
making the "A B C" form work. My only hesitation is that it is in fact a
regression.

> Okay, just to make sure I understand this correctly: if more than one
> argument is literally specified, I should not set up the revision
> walker and pick the commits listed in revs->pending, correct?  Then,
> when I encounter the following command,
> 
>   $ git cherry-pick maint ^master
> 
> I should just pick two commits: maint, and ^master.

But ^master is not a commit, it is a negation. So it is nonsensical if
the arguments are considered independent of each other (you _could_
use a heuristic to guess that they are not independent, but I'd rather
not go there). So you'd probably end up just rejecting the arguments.

-Peff