Subject: Re: [PATCH 1/3] revision: complicated pathspecs disable filters

From: Junio C Hamano <[email-masked]>
Date: Sat, 11 Apr 2020 14:40:04 -0700

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> From: Derrick Stolee <[email-masked]>
>
> The changed-path Bloom filters work only when we can compute an
> explicit Bloom filter key in advance. When a pathspec is given
> that allows case-insensitive checks or wildcard matching, we
> must disable the Bloom filter performance checks.

How often do we want to do case-insensitive path matching, I wonder.

As somebody who never touches case-insensitive filesystem, I would
be a bad judge, and I would imagine that I would be looking for a
pathspec "[Mm]akefile" rather than ":(icase)makefile" myself if
there are projects that may have either/both, so I do not mind
disabling the bloom filter for case insensitivity myself.

But if users may use icase pathspec very often, it may be worth
considering to build the bloom filter after downcasing the paths,
perhaps?  Given that many projects extract their source code to a
case insensitive filesystem, I would imagine that downcasing paths
would map two originally different paths into the same thing only
rarely, if ever, so there may not be much downside to do so.

> By checking the pathspec in prepare_to_use_bloom_filters(), we
> avoid setting up the Bloom filter data and thus revert to the
> usual logic.
>
> Before this change, the following tests would fail*:
>
> 	t6004-rev-list-path-optim.sh (Tests 6-7)
> 	t6130-pathspec-noglob.sh (Tests 3-6)
> 	t6131-pathspec-icase.sh (Tests 3-5)
>
> *These tests would fail when using GIT_TEST_COMMIT_GRAPH and
> GIT_TEST_COMMIT_GRAPH_BLOOM_FILTERS except that the latter
> environment variable was not set up correctly to write the changed-
> path Bloom filters in the test suite. That will be fixed in the
> next change.

Thanks.  This is exciting.