Re: [PATCH 2/2] bloom: enable multiple pathspec bloom keys

[Junio C Hamano <gitster@pobox.com>]

Lidong Yan <502024330056@smail.nju.edu.cn> 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.