Re: [PATCH 0/2] bloom: use bloom filter given multiple pathspec

> 
> Lidong Yan <yldhome2d2@gmail.com> writes:
> 
>> git won't use bloom filter for multiple pathspec, which makes the command
> 
> Let's get the terminology straight.  A pathspec consists of one or
> more pathspec elements (or pathspec items).

> Also, "git won't" is overly general.  The series title shares the
> same issue ("given multiple pathspec" does not even hint that this
> is about revision traversal---you are not making filter used with
> pathspec with more than one element in other code paths).
> 
> Perhaps like:
> 
>    The revision traversal limited by pathspec has optimization when
>    the pathspec has only one element, it does not use any pathspec
>    magic (other than literal), and there is no wildcard.
> 
>    While it is much harder to lift the latter two limitations,
>    supporting a pathspec with multiple elements is relatively easy.
>    Just make sure we hash each of them separately and ask the bloom
>    filter about them, and if we see none of them can possibly be
>    affected by the commit, we can skip without tree comparison.
> 
> or something along that line?
> 

> Can we have a set of real tests to make sure that the updated filter
> code still identifies commits that touch the files without false
> negatives?  False positives are OK as we will follow them with real
> tree comparison to determine what exactly got changed, but false
> negatives are absolute no-no.
> 
> Testing to see that the filter code path is activated is much less
> interesting than the filter code path still functions correctly with
> these changes presented here.  I have a feeling that with the
> changes to the test in this series, you wouldn't even find a bug
> where you simply added subpaths for all pathspec elements into a
> single array and use the original "bloom has to say 'possibly yes'
> to all array elements" logic (which would incorrectly require that
> both file1 and file2 must be modified).