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

From: Junio C Hamano <[email-masked]>
Date: Fri, 27 Jun 2025 13:39:04 -0700

Lidong Yan <[email-masked]> writes:

This is a tangent, but I have to say that whoever wrote the original
test does not understand shells very well.  When you have files A
and B in your working tree, to your $command, the following two does
not make any difference:

	$command ?
	$command A B

In fact it cannot even tell which form was used when composing the
command line.  So this original test ...

> -test_expect_success 'git log with wildcard that resolves to a multiple paths does not uses Bloom filters' '
> -	test_bloom_filters_not_used "-- *" &&
> -	test_bloom_filters_not_used "-- file*"

... is misleading to say the least.

> +test_expect_success 'git log with wildcard that resolves to a multiple paths uses Bloom filters' '
> +	test_bloom_filters_used "-- *" &&
> +	test_bloom_filters_used "-- file*"
>  '

I think you should just retitle this to say

	git log with multiple literal paths use Bloom filter

or something.

Also the setup helper test_bloom_filters_{not_,}used helpers call is
written in a way to make it impossible to pass a real wildcard and
see how "$git log" would behave, because it does this:

	git -c core.commitGraph=false log --pretty="format:%s" $1 >log_wo_bloom &&

It probably should use 'eval' so that the caller can pass a quoted
wildcard, perhaps like

    eval git -c core.commitgraph=false \
	     log --pretty=format:%s "$1" >log_wo_bloom &&

Then a test we can add to see how wildcards prevent Bloom from
kicking in would look like

	test_bloom_filters_used "-- file*" &&
	test_bloom_filters_not_used "-- file4 file\*" &&

The former lets the shell expand file* when the above "eval"
evaluates its (concatenated) strings, while the latter leaves the
backslash before the asterisk in the strings fed to "eval", so the
"log" will see a pathspec with wildcard.

If we were to fix that setup() thing, we of course need to be
a bit careful about existing tests.

Thanks.