Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
> I did not implement the mitigation of not using the Bloom filters when
> a high-bit path is sought because, as Stolee says, this is useful only
> when mixing Git implementations and will slow down operations (without
> any increase in correctness) in the absence of such a mix [1].
Sensible, I guess.
> @@ Commit message
> This commit does not change the behavior of writing (Git writes changed
> path filters when explicitly instructed regardless of any config
> variable), but a subsequent commit will restrict Git such that it will
> - only write when commitgraph.changedPathsVersion is 0, 1, or 2.
> + only write when commitgraph.changedPathsVersion is a recognized value.
This is nicer.
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> @@ Documentation/config/commitgraph.txt: commitGraph.maxNewFilters::
> - 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
> ++ Deprecated. Equivalent to changedPathsVersion=-1 if true, and
> + changedPathsVersion=0 if false.
I forgot to comment on this part earlier, but does the context make
it clear enough that these `changedPathsVersion` references are
about `commitGraph.changedPathsVersion` configuration variable
without fully spelled out? They sit next to each other right now,
so it may not be too bad. If they appeared across more distance,
I would be worried, though.
> +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
> ++ write. May be -1, 0 or 1. Any changed-path Bloom filters on disk that do not
> + match the version set in this config variable will be ignored.
So, any time the user configures this to a different value, we will
start to ignore the existing changed-path-filters data in the
repository, and when we are told to write commit-graph, we will
construct changed-path-filters data using the new version?
> ++
> -+Defaults to 1.
> ++Defaults to -1.
> +++
> ++If -1, Git will use the version of the changed-path Bloom filters in the
> ++repository, defaulting to 1 if there are none.
OK, that was misleading. The configuration can say "-1" and it does
not mean "I'll ignore anything other than version -1"---it means
"I'll read anything". The earlier statement should be toned down so
that we do not surprise readers, perhaps
When set to a positive integer value, any changed-path Bloom
filters on disk whose version is different from the value are
ignored.
to signal that 0 and negative are special. Then the readers can
anticipate that special cases are described next.
When set to -1, then ...
When set to 0, then ...
Defaults to -1.
When set to the special value -1, what version will we write?
> +If 0, git will write version 1 Bloom filters when instructed to write.
And we will only read 0 and refuse to read 1? Or we will read both
0 and 1?
Thanks.
