Re: [PATCH 0/8] ahead-behind: new builtin for counting multiple commit ranges

From: Jeff King <peff@peff.net>
Date: Mar 9, 2023, 09:20 UTC
Message-ID: <ZAmk9V54c/jIbJ65@coredump.intra.peff.net>
In-Reply-To: <ZAaHB7XTz8HX75LD@nand.local>

On Mon, Mar 06, 2023 at 07:36:23PM -0500, Taylor Blau wrote:

Show 15 quoted lines

> > This makes readers wonder if "git rev-list --count B...C" should be
> > the end-user facing UI for this new feature, perhaps?
> >
> > Of course if you are checking how C0, C1, C2,... relate to a single
> > B, the existing rev-list syntax would not work, and makes a totally
> > new subcommand a possibilty.
> 
> Yeah. You could imagine that `rev-list --count` might do something
> fancy like coalescing
> 
>     git rev-list --count B...C1 B...C2 B...C3
> 
> into a single walk. But I am not sure that just because `rev-list
> --count` provides similar functionality that we should fold in the
> proposed `ahead-behind` interface into that flag.

It does coalesce all of that into a single walk. The problem is somewhat the opposite: it only has a notion of two "sides" for a symmetric traversal: left and right. But in your example there are many sides, and we have to remember which is which.

I think getting the answer from one walk would require an arbitrary number of bits to paint down each path. Certainly the ahead-behind that Vicent and I wrote long ago didn't do that (IIRC it mostly relied on doing multiple traversals in the same process, which amortized the cost of commit parsing; that's not really an issue these days with commit graphs).

Peeking at patch 7 of Stolee's series...yep. That's exactly what it does. :)

I wondered how much it would matter on top of a naive loop of single-traversals, now that we have commit graphs. It looks like there's still quite a nice speedup from the numbers in patch 7 (though the totally naive "loop of rev-list" is incurring extra startup overhead, too).

> My personal feeling is that we ought to avoid (further) overloading
> `rev-list` absent of a compelling reason to do so. But I am definitely
> open to other thoughts here.

So I think this actually is what "git rev-list --left-right --count old...new" does now. But extending it to multiple sets in one traversal means you need:

  - being able to ask for individual left-right markers for each pair,
    not treating all lefts and all rights together

  - don't stop traversing when you hit an UNINTERESTING commit if there
    are still bits to paint. In a single-pair traversal, those two are
    the same thing (we stop at the merge base), but with multiple pairs
    you may have to keep walking past a commit that is excluded from one
    pair, but not another. This _might_ be doable if you assume all of
    the left-hand bases are the same, but I didn't think hard enough to
    feel confident in that. But even so, that only solves cases like
    "how do these branches compare to HEAD" (which I think is what
    GitHub does). But it doesn't allow "how do these branches compare to
    to their respective @{upstream} refs".

So I don't think it would be impossible to make this a mode of rev-list. And that mode might even provide flexibility for other similar operations, like a mass "git rev-list --cherry-mark"[1]. But it is a pretty big departure from the current rev-list traversal (to my mind, especially the "keep walking past UNINTERESTING part). I don't mind it as its own command.

-Peff

[1] The reason you might want a mass cherry-mark is basically doing
    something like the "branches" page, but in a workflow where upstream
    applies patches, like git.git. There you may want to ask about
    "origin/next...$branch" for all of your branches to see which ones
    have been merged where.