Re: [PATCH 1/3] revision: support tracking uninteresting commits

From: Taylor Blau <me@ttaylorr.com>
Date: May 3, 2023, 21:48 UTC
Message-ID: <ZFLWwTAIxRnNUKDp@nand.local>
In-Reply-To: <0b8884ea-f37b-b7d4-6edb-825ca935a893@github.com>

On Tue, Apr 25, 2023 at 02:15:49PM -0400, Derrick Stolee wrote:

Show 12 quoted lines

> On 4/24/2023 8:00 PM, Taylor Blau wrote:
> > The boundary-based bitmap walk will want to know which commits were
> > marked UNINTERESTING in the walk used to discover the boundary.
> >
> > Track which commits are marked as such during list limitation as well as
> > the topo walk step, though the latter is not used.
>
> I was surprised to see this as an addition to revision.c, and
> only specific to limit_list() (and its iterative copy). I would
> expect this to work for non-topo-order, too. I suppose we couldn't
> completely trust that the first visit to a commit includes the
> UNINTERESTING flag if there is clock skew in the commit history.

Yeah, the distinction between limit_list() and the --topo-order code makes things a little funky here. But I think that's OK, since `--topo-order` is not likely to be used here, since this is all bitmap-based traversal. IOW, I think that it would be reasonable to ban the revision args combination of --use-bitmap-index with --topo-order.

> But could we also do this at the caller's end by passing a
> callback that checks each walked commit if it is UNINTERESTING
> and adds it to a set?

I think I remember trying this when I originally wrote this code, and ended up discarding the idea because it walked over more commits than we needed to consider.

Thanks, Taylor