Re: [PATCH v4 1/4] gitformat-commit-graph: describe version 2 of BDAT

[Derrick Stolee <derrickstolee@github.com>]

On 6/13/2023 5:58 PM, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:

>> +	in Probabilistic Verification". Version 1 bloom filters have a bug that appears
> 
> "bloom" -> "Bloom", probably, as the name comes from the name of its
> inventor (just like we spell "Boolean", not "boolean").

The ultimate recognition comes from when the term named after you
becomes lower-case ("boolean" is sometimes in this category, but
definitely "abelian" is an example).

In this case, you are right that we should capitalize Bloom.

>> +	when char is signed and the repository has path names that have characters >=
>> +	0x80; Git supports reading and writing them, but this ability will be removed
>> +	in a future version of Git.
> 
> Makes sense.

I also like how you organized this: "We support version 2. 1 is
still around but not for long."
 
> I wonder if we want to mention what the undesired misbehaviour the
> "bug" causes and what we do to avoid getting affected by the bug
> here.  If we can say something like "When querying for a pathname
> with a byte with high-bit set, the buggy filter may produce false
> negative, making the filter unusable, but asking for a pathname
> without such a byte produces no false negatives (even though we may
> get false positives).  When Git reads version 1 filter data, it
> refrains from using it for processing paths with high-bit set to
> avoid triggering the bug", then it would be ideal.  Or "When the
> repository has even a single pathname with high-bit set anywhere in
> its history, version 1 Bloom can give false negative when querying
> any paths and becomes unusable.  You can use $THIS configuration
> variable to disable use of Bloom filter data in such a case" would
> also be fine.  The point is to give actionable piece of information
> to the readers.

This is definitely helpful, but if someone is having issues we
would say "try version 2 and see if it still happens" and not
over-index on the underlying reason.

That's to say, I'm OK with the shorter description of the problem.
Feel free to expand if you're interested, though.

Thanks,
-Stolee