On 5/3/2023 5:48 PM, Taylor Blau wrote:

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

quoted

On 4/24/2023 8:00 PM, Taylor Blau wrote:

quoted

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.

 
I think there is a miscommunication here, since the limit_list()
code will only be run if there is a need to "walk to the end" before
outputting results. --topo-order is an example of something that
triggers this need, but it is disabled by default.

It seems that revs->collect_uninteresting would be a condition that
should signal to enable revs->limited so this code is actually
executed.

Taking a look at how this works with your current patch, the only
thing I can think is that since your initial commits include some
UNINTERESTING commits, that actually triggers revs->limited = 1
in handle_commit(). It's an indirect use, and without such a
commit the boundary would be empty, anyway.

Thanks,
-Stolee