Lidong Yan <502024330056@xxxxxxxxxxxxxxxx> writes: >> You'd need to check in forbid_bloom_filters() that none of the >> pathspec items have magic (other than literal), not just the first >> one, no? > > Yeah, I never notice that. I would add checks in forbid_bloom_filters(). > And add test to ensure we don’t use bloom filter if any pathspec item is > not literal. Sounds great. I was wondering why your tests did not catch it. >> Totally outside the topic, but I wonder if we can further optimize >> by adding an early rejection using .nowildcard_len? Instead of >> allowing a wildcarded "dir/*" pathspec element from disabling the >> Bloom filter altogether, we could say "dir/ is not possibly altered, >> so there may be dir/A, dir/B, etc., in the directory, nothing that >> would match dir/* wildcard would have been modified", couldn't we? > > I think it's feasible. In that case, we would need to add a condition > .nowildcard_len > 0 to forbid_bloom_filter. I'm happy to write a new > patch to address this issue. Let's leave it outside the topic and concentrate on the problem at hand first.
