On 6/13/2023 1:39 PM, Jonathan Tan wrote:
> A subsequent commit will introduce another version of the changed-path
> filter in the commit graph file. In order to control which version is
> to be accepted when read (and which version to write), a config variable
> is needed.
>
> Therefore, introduce this config variable. For forwards compatibility,
> teach Git to not read commit graphs when the config variable
> is set to an unsupported version. Because we teach Git this,
> commitgraph.readChangedPaths is now redundant, so deprecate it and
> define its behavior in terms of the config variable we introduce.
I'm late to the party, but I support this change. The safety valve
of "I want to turn this off if something goes wrong" is long overdue
for deletion (it would help someone in this high-bit situation).
Having a new replacement is a good way to preserve the safety valve
behavior while also promoting the new versioning scheme.
> 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.
> 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.
This defaulted to true before...
> +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.
So this version defaults to 1. Good.
> ++
> +If 0, git will write version 1 Bloom filters when instructed to write.
> - if (s->commit_graph_read_changed_paths) {
> + if (s->commit_graph_changed_paths_version == 1) {
> pair_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES,
> &graph->chunk_bloom_indexes);
> read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA,
> /* Commit graph config or default, does not cascade (simple) */
> repo_cfg_bool(r, "core.commitgraph", &r->settings.core_commit_graph, 1);
> repo_cfg_int(r, "commitgraph.generationversion", &r->settings.commit_graph_generation_version, 2);
> - repo_cfg_bool(r, "commitgraph.readchangedpaths", &r->settings.commit_graph_read_changed_paths, 1);
> + repo_cfg_bool(r, "commitgraph.readchangedpaths", &readChangedPaths, 1);
> + repo_cfg_int(r, "commitgraph.changedpathsversion",
> + &r->settings.commit_graph_changed_paths_version,
> + readChangedPaths ? 1 : 0);
And here we default to 'true' and '1', respectively. This allows
'commitGraph.readChangedPaths=false' to override
'commitGraph.changedPathsVersion=1'. Should that implication be
documented?
Thanks,
-Stolee
