commit-graph: pass repo_settings instead of repository

The parse_commit_graph() function takes a 'struct repository *' pointer,
but it only ever accesses config settings (either directly or through
the .settings field of the repo struct). Move all relevant config
settings into the repo_settings struct, and update parse_commit_graph()
and its existing callers so that it takes 'struct repo_settings *'
instead.

Callers of parse_commit_graph() will now need to call
prepare_repo_settings() themselves, or initialize a 'struct
repo_settings' directly.

Prior to ab14d0676c (commit-graph: pass a 'struct repository *' in more
places, 2020-09-09), parsing a commit-graph was a pure function
depending only on the contents of the commit-graph itself. Commit
ab14d0676c introduced a dependency on a `struct repository` pointer, and
later commits such as b66d84756f (commit-graph: respect
'commitGraph.readChangedPaths', 2020-09-09) added dependencies on config
settings, which were accessed through the `settings` field of the
repository pointer. This field was initialized via a call to
`prepare_repo_settings()`.

Additionally, this fixes an issue in fuzz-commit-graph: In 44c7e62
(2021-12-06, repo-settings:prepare_repo_settings only in git repos),
prepare_repo_settings was changed to issue a BUG() if it is called by a
process whose CWD is not a Git repository.

The combination of commits mentioned above broke fuzz-commit-graph,
which attempts to parse arbitrary fuzzing-engine-provided bytes as a
commit graph file. Prior to this change, parse_commit_graph() called
prepare_repo_settings(), but since we run the fuzz tests without a valid
repository, we are hitting the BUG() from 44c7e62 for every test case.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/commit-graph.c b/commit-graph.c
index 92d450333664d4298eb9cbcda332fe857fd22458..5a6280290f8303a03ecbf61ab6e0bdb56bb6c0f7 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -252,7 +252,8 @@ struct commit_graph *load_commit_graph_one_fd_st(struct repository *r,
        }
        graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
        close(fd);
-       ret = parse_commit_graph(r, graph_map, graph_size);
+       prepare_repo_settings(r);
+       ret = parse_commit_graph(&r->settings, graph_map, graph_size);
 
        if (ret)
                ret->odb = odb;
@@ -321,7 +322,7 @@ static int graph_read_bloom_data(const unsigned char *chunk_start,
        return 0;
 }
 
-struct commit_graph *parse_commit_graph(struct repository *r,
+struct commit_graph *parse_commit_graph(struct repo_settings *s,
                                        void *graph_map, size_t graph_size)
 {
        const unsigned char *data;
@@ -359,8 +360,6 @@ struct commit_graph *parse_commit_graph(struct repository *r,
                return NULL;
        }
 
-       prepare_repo_settings(r);
-
        graph = alloc_commit_graph();
 
        graph->hash_len = the_hash_algo->rawsz;
@@ -390,7 +389,7 @@ struct commit_graph *parse_commit_graph(struct repository *r,
        pair_chunk(cf, GRAPH_CHUNKID_EXTRAEDGES, &graph->chunk_extra_edges);
        pair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs);
 
-       if (get_configured_generation_version(r) >= 2) {
+       if (s->commit_graph_generation_version >= 2) {
                pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA,
                        &graph->chunk_generation_data);
                pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW,
@@ -400,7 +399,7 @@ struct commit_graph *parse_commit_graph(struct repository *r,
                        graph->read_generation_data = 1;
        }
 
-       if (r->settings.commit_graph_read_changed_paths) {
+       if (s->commit_graph_read_changed_paths) {
                pair_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES,
                           &graph->chunk_bloom_indexes);
                read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA,

diff --git a/commit-graph.h b/commit-graph.h
index 2e3ac35237e919790555bae317bd580cf065b727..25cdec127aca098ed4bd35bdfcf8e630d8758e01 100644 (file)
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -93,7 +93,12 @@ struct commit_graph *load_commit_graph_one_fd_st(struct repository *r,
                                                 struct object_directory *odb);
 struct commit_graph *read_commit_graph_one(struct repository *r,
                                           struct object_directory *odb);
-struct commit_graph *parse_commit_graph(struct repository *r,
+
+/*
+ * Callers should initialize the repo_settings with prepare_repo_settings()
+ * prior to calling parse_commit_graph().
+ */
+struct commit_graph *parse_commit_graph(struct repo_settings *s,
                                        void *graph_map, size_t graph_size);
 
 /*

diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c
index e7cf6d5b0facb53d3e5f35d9b54ac34814d0491a..914026f5d80f876c8a7c28a914a58295a5a72346 100644 (file)
--- a/fuzz-commit-graph.c
+++ b/fuzz-commit-graph.c
@@ -1,7 +1,7 @@
 #include "commit-graph.h"
 #include "repository.h"
 
-struct commit_graph *parse_commit_graph(struct repository *r,
+struct commit_graph *parse_commit_graph(struct repo_settings *s,
                                        void *graph_map, size_t graph_size);
 
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
@@ -11,7 +11,15 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
        struct commit_graph *g;
 
        initialize_the_repository();
-       g = parse_commit_graph(the_repository, (void *)data, size);
+       /*
+        * Initialize the_repository with commit-graph settings that would
+        * normally be read from the repository's gitdir. We want to avoid
+        * touching the disk to keep the individual fuzz-test cases as fast as
+        * possible.
+        */
+       the_repository->settings.commit_graph_generation_version = 2;
+       the_repository->settings.commit_graph_read_changed_paths = 1;
+       g = parse_commit_graph(&the_repository->settings, (void *)data, size);
        repo_clear(the_repository);
        free_commit_graph(g);
 

diff --git a/repo-settings.c b/repo-settings.c
index 2dfcb2b6542f2cbe75d4575e8b707cf1676679a3..43bc881bfc90ca1db92cdbf917fc09182a04a798 100644 (file)
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -11,6 +11,13 @@ static void repo_cfg_bool(struct repository *r, const char *key, int *dest,
                *dest = def;
 }
 
+static void repo_cfg_int(struct repository *r, const char *key, int *dest,
+                        int def)
+{
+       if (repo_config_get_int(r, key, dest))
+               *dest = def;
+}
+
 void prepare_repo_settings(struct repository *r)
 {
        int experimental;
@@ -42,11 +49,14 @@ void prepare_repo_settings(struct repository *r)
                r->settings.core_untracked_cache = UNTRACKED_CACHE_WRITE;
        }
 
-       /* Boolean config or default, does not cascade (simple)  */
+       /* 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, "gc.writecommitgraph", &r->settings.gc_write_commit_graph, 1);
        repo_cfg_bool(r, "fetch.writecommitgraph", &r->settings.fetch_write_commit_graph, 0);
+
+       /* Boolean config or default, does not cascade (simple)  */
        repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1);
        repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1);
        repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0);

diff --git a/repository.h b/repository.h
index 6cc661e5a43b82022ca171a43c015db2f278116e..797f471cce9a5501ead146933d71f33bd45827e4 100644 (file)
--- a/repository.h
+++ b/repository.h
@@ -30,6 +30,7 @@ struct repo_settings {
        int initialized;
 
        int core_commit_graph;
+       int commit_graph_generation_version;
        int commit_graph_read_changed_paths;
        int gc_write_commit_graph;
        int fetch_write_commit_graph;