From: Lidong Yan <502024330056@smail.nju.edu.cn>
Date: Sat, 28 Jun 2025 02:54:09 GMT
Subject: Re: [PATCH 2/2] bloom: enable multiple pathspec bloom keys
Message-ID: <1D8CE39A-D6F5-4AB7-8613-6C8DF1302907@smail.nju.edu.cn>
In-Reply-To: <xmqqqzz47wd3.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:
> 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 &&

Yeah, if $1 contains * and because $1 is not quotes, * would trigger file name
expansion.

> 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.

Wow, this solution is very clever.

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

Though the uses of test_bloom_filters_(not_)used are not too much,
I think replace
  git -c core.commitGraph=false log --pretty="format:%s" $1 >log_wo_bloom &&
with
  git -c core.commitGraph=false log --pretty="format:%s” “$*" >log_wo_bloom &&

is not better than add the “eval …” solution, I will just use eval

Thanks,
Lidong