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

From: Jeff King <peff@peff.net>
Date: Jan 12, 2012, 17:15 UTC
Message-ID: <20120112171536.GA18102@sigill.intra.peff.net>
In-Reply-To: <CALkWK0=Mv_tzNw-hN_9fAr+vABappndEK5iSWQHDk8Yk6Z-stw@mail.gmail.com>

On Thu, Jan 12, 2012 at 10:39:48PM +0530, Ramkumar Ramachandra wrote:

Show 11 quoted lines

> Jeff King wrote:
> >                  I agree it would be nice to make:
> >  git cherry-pick commit1 commit3 commit2
> >
> > work in the order specified, but how does that interact with existing
> > cases that provide more traditional revision arguments?
> 
> What are your thoughts on making it a flag in the revision API to be
> activated with "cherry-pick --literal-order commit1 commit3 commit2"
> or similar?  I'm not sure how to get it to reconcile with the more
> traditional revision arguments yet. My current worktree (WIP):

I think that is a sensible first-cut. It may even be possible to use heuristics to identify when --literal-order is needed, and eventually it could go away. But that is a much riskier feature that can be built on top of the much safer proposal you are making.

Show 16 quoted lines

> diff --git a/revision.c b/revision.c
> index 064e351..301ef58 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
>                 if (commit) {
>                         if (!(commit->object.flags & SEEN)) {
>                                 commit->object.flags |= SEEN;
> -                               commit_list_insert_by_date(commit,
> &revs->commits
> +                               if (revs->literal_order)
> +                                       commit_list_insert(commit,
> &revs->commits
> +                               else
> +
> commit_list_insert_by_date(commit, &revs-

My only concern is that there are other parts of the revision machinery that depend on the date-ordering of the commit list. What would happen, for example, with:

  git rev-list --literal-order --do-walk foo

It probably doesn't make sense to allow literal-order without no-walk, anyway (which of course is the default in cherry-pick anyway, so it's not a big deal here).

I'm also not sure what:

  git rev-list --literal-order foo..bar

would or should do.

-Peff