Subject: Re: [PATCH v4 3/4] repo-settings: introduce commitgraph.changedPathsVersion

From: Taylor Blau <[email-masked]>
Date: Wed, 21 Jun 2023 08:14:38 -0400

On Tue, Jun 13, 2023 at 10:39:57AM -0700, Jonathan Tan wrote:
> diff --git a/Documentation/config/commitgraph.txt b/Documentation/config/commitgraph.txt
> index 30604e4a4c..eaa10bf232 100644
> --- a/Documentation/config/commitgraph.txt
> +++ b/Documentation/config/commitgraph.txt
> @@ -9,6 +9,16 @@ commitGraph.maxNewFilters::
>  	commit-graph write` (c.f., linkgit:git-commit-graph[1]).
>
>  commitGraph.readChangedPaths::
> -	If true, then git will use the changed-path Bloom filters in the
> -	commit-graph file (if it exists, and they are present). Defaults to
> -	true. See linkgit:git-commit-graph[1] for more information.
> +	Deprecated. Equivalent to changedPathsVersion=1 if true, and
> +	changedPathsVersion=0 if false.
> +
> +commitGraph.changedPathsVersion::
> +	Specifies the version of the changed-path Bloom filters that Git will read and
> +	write. May be 0 or 1. Any changed-path Bloom filters on disk that do not
> +	match the version set in this config variable will be ignored.
> ++
> +Defaults to 1.
> ++
> +If 0, git will write version 1 Bloom filters when instructed to write.
> ++
> +See linkgit:git-commit-graph[1] for more information.

Hmm. I'm a little confused: we should still be able to use the old
broken filters if (and only if) the paths we're querying don't have any
bytes with their high-order bit set, no?

That should be true with the caveat that querying such a path would need
to result in our querying function returning "maybe" instead of
"definitely not" to protect against the false-negatives described
earlier.

As I read this, it seems to imply that as soon as this change lands that
we'll stop reading old Bloom filters altogether. Is that the case?

If so, I wonder if we can do this with without needing this
configuration setting at all (by writing the newest version of Bloom
filters possible, and working around the existing ones with the
aforementioned workaround).

Thanks,
Taylor