Hi Stolee, On Sat, Apr 11, 2020 at 01:02:59AM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > > The changed-path Bloom filters work only when we can compute an > explicit Bloom filter key in advance. When a pathspec is given > that allows case-insensitive checks or wildcard matching, we > must disable the Bloom filter performance checks. > > By checking the pathspec in prepare_to_use_bloom_filters(), we > avoid setting up the Bloom filter data and thus revert to the > usual logic. All makes sense to me, and this seems like the only reasonable thing *to* do in this situation. That's fine, since we're not regressing, we're just not using Bloom filters. > Before this change, the following tests would fail*: > > t6004-rev-list-path-optim.sh (Tests 6-7) > t6130-pathspec-noglob.sh (Tests 3-6) > t6131-pathspec-icase.sh (Tests 3-5) > > *These tests would fail when using GIT_TEST_COMMIT_GRAPH and > GIT_TEST_COMMIT_GRAPH_BLOOM_FILTERS except that the latter > environment variable was not set up correctly to write the changed- > path Bloom filters in the test suite. That will be fixed in the > next change. Nicely done. > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > revision.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/revision.c b/revision.c > index 2b06ee739c8..e37b5b06108 100644 > --- a/revision.c > +++ b/revision.c > @@ -661,6 +661,15 @@ static void prepare_to_use_bloom_filter(struct rev_info *revs) > if (!revs->commits) > return; > I certainly wouldn't complain about a comment here explaining these three checks, but I suppose that the rationale is only a 'git blame' away (and I guess that is faster now after this series ;-)). > + if (revs->prune_data.has_wildcard) > + return; > + if (revs->prune_data.nr > 1) > + return; > + if (revs->prune_data.magic || > + (revs->prune_data.nr && > + revs->prune_data.items[0].magic)) > + return; > + > repo_parse_commit(revs->repo, revs->commits->item); > > if (!revs->repo->objects->commit_graph) > -- > gitgitgadget Reviewed-by: Taylor Blau <me@xxxxxxxxxxxx> Thanks, Taylor
