On Fri, Aug 10, 2012 at 2:38 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Martin von Zweigbergk <martin.von.zweigbergk@xxxxxxxxx> writes:
>
>> There is also cherry-pick/revert, which I _think_ does not really want
>> the revisions sorted.
>
> Yes, I think sequencer.c::prepare_revs() is wrong to unconditoinally
> call prepare_revision_walk().
>
> It instead should first check the revs->pending.objects list to see
> if what was given by the caller is a mere collection of individual
> objects or a range expression (i.e. check if any of them is marked
> with UNINTERESTING), and refrain from going into the body of the
> preparation steps, which has to involve sorting.
Do you mean "has to involve sorting" as in "has to involve sorting in
order not to break current users of e.g. 'git log --no-walk
--branches'" or "revision walking inherently involves sorting"? My
current working assumption is that it is the former.
I will make rev_info.no_walk a tri-state {walk, no-walk-sorted,
no-walk-unsorted}. The third state would be used from
cherry-pick/revert (and maybe git-show, although it should make no
difference). I would also expose the third state to rev-list's command
line, maybe as --no-walk=unsorted.
Actually, all but command-line parsing is done now and test seem fine,
with quite a small patch:
$ git diff --stat
builtin/log.c | 2 +-
builtin/revert.c | 2 +-
revision.c | 5 +++--
revision.h | 6 +++++-
4 files changed, 10 insertions(+), 5 deletions(-)
Did you see a problem with this approach, since you said that
sequencer shouldn't unconditionally call prepare_revision_walk()? I
can see that git-show needs to go through revs->pending.objects
because it handles tags and stuff, but cherry-pick/revert only seem to
need the revisions.
Martin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html