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

From: Taylor Blau <[email-masked]>
Date: Wed, 3 May 2023 17:48:49 -0400

On Tue, Apr 25, 2023 at 02:15:49PM -0400, Derrick Stolee wrote:
> 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