On 5/3/2023 5:48 PM, Taylor Blau wrote: > 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. 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
