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