From e45402bb199e869c223f575bf2f44db679d00279 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 15 Aug 2025 07:49:48 +0200 Subject: commit-graph: store the hash algorithm instead of its length The commit-graph stores the length of the hash algorithm it uses. In subsequent commits we'll need to pass the whole hash algorithm around though, which we currently don't have access to. Refactor the code so that we store the hash algorithm instead of only its size. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-graph.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'commit-graph.h') diff --git a/commit-graph.h b/commit-graph.h index 78ab7b875b..7dc1f2b22b 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -84,7 +84,7 @@ struct commit_graph { const unsigned char *data; size_t data_len; - unsigned char hash_len; + const struct git_hash_algo *hash_algo; unsigned char num_chunks; uint32_t num_commits; struct object_id oid; -- cgit v1.2.3 From f1141b43911441f3c5d0fda49d05a6433da372a3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 15 Aug 2025 07:49:49 +0200 Subject: commit-graph: refactor `parse_commit_graph()` to take a repository Refactor `parse_commit_graph()` so that it takes a repository instead of taking repository settings. On the one hand this allows us to get rid of instances where we access `the_hash_algo` by using the repository's hash algorithm instead. On the other hand it also allows us to move the call of `prepare_repo_settings()` into the function itself. Note that there's one small catch, as the commit-graph fuzzer calls this function directly without having a fully functional repository at hand. And while the fuzzer already initializes `the_repository` with relevant info, the call to `prepare_repo_settings()` would fail because we don't have a fully-initialized repository. Work around the issue by also settings `settings.initialized` to pretend that we've already read the settings. While at it, remove the redundant `parse_commit_graph()` declaration in the fuzzer. It was added together with aa658574bf (commit-graph, fuzz: add fuzzer for commit-graph, 2019-01-15), but as we also declared the same function in "commit-graph.h" it wasn't ever needed. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-graph.c | 23 ++++++++++++----------- commit-graph.h | 2 +- oss-fuzz/fuzz-commit-graph.c | 6 ++---- 3 files changed, 15 insertions(+), 16 deletions(-) (limited to 'commit-graph.h') diff --git a/commit-graph.c b/commit-graph.c index 6cdaff26c2..55a8ce0739 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -272,9 +272,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); - prepare_repo_settings(r); - ret = parse_commit_graph(&r->settings, graph_map, graph_size); + ret = parse_commit_graph(r, graph_map, graph_size); if (ret) ret->odb_source = source; else @@ -374,7 +373,7 @@ static int graph_read_bloom_data(const unsigned char *chunk_start, return 0; } -struct commit_graph *parse_commit_graph(struct repo_settings *s, +struct commit_graph *parse_commit_graph(struct repository *r, void *graph_map, size_t graph_size) { const unsigned char *data; @@ -386,7 +385,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s, if (!graph_map) return NULL; - if (graph_size < graph_min_size(the_hash_algo)) + if (graph_size < graph_min_size(r->hash_algo)) return NULL; data = (const unsigned char *)graph_map; @@ -406,22 +405,22 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s, } hash_version = *(unsigned char*)(data + 5); - if (hash_version != oid_version(the_hash_algo)) { + if (hash_version != oid_version(r->hash_algo)) { error(_("commit-graph hash version %X does not match version %X"), - hash_version, oid_version(the_hash_algo)); + hash_version, oid_version(r->hash_algo)); return NULL; } graph = alloc_commit_graph(); - graph->hash_algo = the_hash_algo; + graph->hash_algo = r->hash_algo; graph->num_chunks = *(unsigned char*)(data + 6); graph->data = graph_map; graph->data_len = graph_size; if (graph_size < GRAPH_HEADER_SIZE + (graph->num_chunks + 1) * CHUNK_TOC_ENTRY_SIZE + - GRAPH_FANOUT_SIZE + the_hash_algo->rawsz) { + GRAPH_FANOUT_SIZE + r->hash_algo->rawsz) { error(_("commit-graph file is too small to hold %u chunks"), graph->num_chunks); free(graph); @@ -452,7 +451,9 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s, pair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs, &graph->chunk_base_graphs_size); - if (s->commit_graph_generation_version >= 2) { + prepare_repo_settings(r); + + if (r->settings.commit_graph_generation_version >= 2) { read_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA, graph_read_generation_data, graph); pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW, @@ -463,7 +464,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s, graph->read_generation_data = 1; } - if (s->commit_graph_changed_paths_version) { + if (r->settings.commit_graph_changed_paths_version) { read_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES, graph_read_bloom_index, graph); read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA, @@ -480,7 +481,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s, } oidread(&graph->oid, graph->data + graph->data_len - graph->hash_algo->rawsz, - the_repository->hash_algo); + r->hash_algo); free_chunkfile(cf); return graph; diff --git a/commit-graph.h b/commit-graph.h index 7dc1f2b22b..7bbc69989c 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -128,7 +128,7 @@ struct repo_settings; * 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, +struct commit_graph *parse_commit_graph(struct repository *r, void *graph_map, size_t graph_size); /* diff --git a/oss-fuzz/fuzz-commit-graph.c b/oss-fuzz/fuzz-commit-graph.c index fbb77fec19..fb8b8787a4 100644 --- a/oss-fuzz/fuzz-commit-graph.c +++ b/oss-fuzz/fuzz-commit-graph.c @@ -4,9 +4,6 @@ #include "commit-graph.h" #include "repository.h" -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); int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) @@ -22,9 +19,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) * possible. */ repo_set_hash_algo(the_repository, GIT_HASH_SHA1); + the_repository->settings.initialized = 1; the_repository->settings.commit_graph_generation_version = 2; the_repository->settings.commit_graph_changed_paths_version = 1; - g = parse_commit_graph(&the_repository->settings, (void *)data, size); + g = parse_commit_graph(the_repository, (void *)data, size); repo_clear(the_repository); free_commit_graph(g); -- cgit v1.2.3 From 89cc9b9adf31729c91aa94c178b44b45febd260f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 15 Aug 2025 07:49:50 +0200 Subject: commit-graph: stop using `the_hash_algo` Stop using `the_hash_algo` as it implicitly relies on `the_repository`. Instead, we either use the hash algo provided via the context or, if there is no such hash algo, we use `the_repository` explicitly. Such uses will be removed in subsequent commits. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/commit-graph.c | 3 ++- commit-graph.c | 27 ++++++++++++++------------- commit-graph.h | 3 ++- 3 files changed, 18 insertions(+), 15 deletions(-) (limited to 'commit-graph.h') diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 4992ac146e..f5c6f863a5 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -109,7 +109,8 @@ static int graph_verify(int argc, const char **argv, const char *prefix, opened = OPENED_GRAPH; else if (errno != ENOENT) die_errno(_("Could not open commit-graph '%s'"), graph_name); - else if (open_commit_graph_chain(chain_name, &fd, &st)) + else if (open_commit_graph_chain(chain_name, &fd, &st, + the_repository->hash_algo)) opened = OPENED_CHAIN; else if (errno != ENOENT) die_errno(_("could not open commit-graph chain '%s'"), chain_name); diff --git a/commit-graph.c b/commit-graph.c index 55a8ce0739..fbb351d9ee 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -265,7 +265,7 @@ struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, graph_size = xsize_t(st->st_size); - if (graph_size < graph_min_size(the_hash_algo)) { + if (graph_size < graph_min_size(r->hash_algo)) { close(fd); error(_("commit-graph file is too small")); return NULL; @@ -320,7 +320,7 @@ static int graph_read_commit_data(const unsigned char *chunk_start, size_t chunk_size, void *data) { struct commit_graph *g = data; - if (chunk_size / graph_data_width(the_hash_algo) != g->num_commits) + if (chunk_size / graph_data_width(g->hash_algo) != g->num_commits) return error(_("commit-graph commit data chunk is wrong size")); g->chunk_commit_data = chunk_start; return 0; @@ -621,7 +621,8 @@ static int add_graph_to_chain(struct commit_graph *g, } int open_commit_graph_chain(const char *chain_file, - int *fd, struct stat *st) + int *fd, struct stat *st, + const struct git_hash_algo *hash_algo) { *fd = git_open(chain_file); if (*fd < 0) @@ -630,7 +631,7 @@ int open_commit_graph_chain(const char *chain_file, close(*fd); return 0; } - if (st->st_size < the_hash_algo->hexsz) { + if (st->st_size < hash_algo->hexsz) { close(*fd); if (!st->st_size) { /* treat empty files the same as missing */ @@ -654,7 +655,7 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, int i = 0, valid = 1, count; FILE *fp = xfdopen(fd, "r"); - count = st->st_size / (the_hash_algo->hexsz + 1); + count = st->st_size / (r->hash_algo->hexsz + 1); CALLOC_ARRAY(oids, count); odb_prepare_alternates(r->objects); @@ -716,7 +717,7 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, int fd; struct commit_graph *g = NULL; - if (open_commit_graph_chain(chain_file, &fd, &st)) { + if (open_commit_graph_chain(chain_file, &fd, &st, r->hash_algo)) { int incomplete; /* ownership of fd is taken over by load function */ g = load_commit_graph_chain_fd_st(r, fd, &st, &incomplete); @@ -908,7 +909,7 @@ static void fill_commit_graph_info(struct commit *item, struct commit_graph *g, die(_("invalid commit position. commit-graph is likely corrupt")); lex_index = pos - g->num_commits_in_base; - commit_data = g->chunk_commit_data + st_mult(graph_data_width(the_hash_algo), lex_index); + commit_data = g->chunk_commit_data + st_mult(graph_data_width(g->hash_algo), lex_index); graph_data = commit_graph_data_at(item); graph_data->graph_pos = pos; @@ -1112,7 +1113,7 @@ static struct tree *load_tree_for_commit(struct repository *r, g = g->base_graph; commit_data = g->chunk_commit_data + - st_mult(graph_data_width(the_hash_algo), + st_mult(graph_data_width(g->hash_algo), graph_pos - g->num_commits_in_base); oidread(&oid, commit_data, the_repository->hash_algo); @@ -1221,7 +1222,7 @@ static int write_graph_chunk_oids(struct hashfile *f, int count; for (count = 0; count < ctx->commits.nr; count++, list++) { display_progress(ctx->progress, ++ctx->progress_cnt); - hashwrite(f, (*list)->object.oid.hash, the_hash_algo->rawsz); + hashwrite(f, (*list)->object.oid.hash, f->algop->rawsz); } return 0; @@ -1252,7 +1253,7 @@ static int write_graph_chunk_data(struct hashfile *f, die(_("unable to parse commit %s"), oid_to_hex(&(*list)->object.oid)); tree = get_commit_tree_oid(*list); - hashwrite(f, tree->hash, the_hash_algo->rawsz); + hashwrite(f, tree->hash, ctx->r->hash_algo->rawsz); parent = (*list)->parents; @@ -2035,7 +2036,7 @@ static int write_graph_chunk_base_1(struct hashfile *f, return 0; num = write_graph_chunk_base_1(f, g->base_graph); - hashwrite(f, g->oid.hash, the_hash_algo->rawsz); + hashwrite(f, g->oid.hash, g->hash_algo->rawsz); return num + 1; } @@ -2059,7 +2060,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) struct hashfile *f; struct tempfile *graph_layer; /* when ctx->split is non-zero */ struct lock_file lk = LOCK_INIT; - const unsigned hashsz = the_hash_algo->rawsz; + const unsigned hashsz = ctx->r->hash_algo->rawsz; struct strbuf progress_title = STRBUF_INIT; struct chunkfile *cf; unsigned char file_hash[GIT_MAX_RAWSZ]; @@ -2147,7 +2148,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) hashwrite_be32(f, GRAPH_SIGNATURE); hashwrite_u8(f, GRAPH_VERSION); - hashwrite_u8(f, oid_version(the_hash_algo)); + hashwrite_u8(f, oid_version(ctx->r->hash_algo)); hashwrite_u8(f, get_num_chunks(cf)); hashwrite_u8(f, ctx->num_commit_graphs_after - 1); diff --git a/commit-graph.h b/commit-graph.h index 7bbc69989c..df10daf01c 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -32,7 +32,8 @@ struct string_list; char *get_commit_graph_filename(struct odb_source *source); char *get_commit_graph_chain_filename(struct odb_source *source); int open_commit_graph(const char *graph_file, int *fd, struct stat *st); -int open_commit_graph_chain(const char *chain_file, int *fd, struct stat *st); +int open_commit_graph_chain(const char *chain_file, int *fd, struct stat *st, + const struct git_hash_algo *hash_algo); /* * Given a commit struct, try to fill the commit struct info, including: -- cgit v1.2.3 From ddacfc7466707cbe462594052261647b43868825 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 15 Aug 2025 07:49:51 +0200 Subject: commit-graph: stop using `the_repository` There's still a bunch of uses of `the_repository` in "commit-graph.c", which we want to stop using due to it being a global variable. Refactor the code to stop using `the_repository` in favor of the repository provided via the calling context. This allows us to drop the `USE_THE_REPOSITORY_VARIABLE` macro. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/commit.c | 2 +- builtin/merge.c | 2 +- commit-graph.c | 76 +++++++++++++++++++++++++++++--------------------------- commit-graph.h | 2 +- 4 files changed, 42 insertions(+), 40 deletions(-) (limited to 'commit-graph.h') diff --git a/builtin/commit.c b/builtin/commit.c index b5b9608813..8a5dee384d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1947,7 +1947,7 @@ int cmd_commit(int argc, "new index file. Check that disk is not full and quota is\n" "not exceeded, and then \"git restore --staged :/\" to recover.")); - git_test_write_commit_graph_or_die(); + git_test_write_commit_graph_or_die(the_repository->objects->sources); repo_rerere(the_repository, 0); run_auto_maintenance(quiet); diff --git a/builtin/merge.c b/builtin/merge.c index dc4cb8fb14..a62317f4fd 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1862,7 +1862,7 @@ int cmd_merge(int argc, if (squash) { finish(head_commit, remoteheads, NULL, NULL); - git_test_write_commit_graph_or_die(); + git_test_write_commit_graph_or_die(the_repository->objects->sources); } else write_merge_state(remoteheads); diff --git a/commit-graph.c b/commit-graph.c index fbb351d9ee..d6f0bf5e88 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1,4 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" @@ -29,7 +28,7 @@ #include "tree.h" #include "chunk-format.h" -void git_test_write_commit_graph_or_die(void) +void git_test_write_commit_graph_or_die(struct odb_source *source) { int flags = 0; if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0)) @@ -38,8 +37,7 @@ void git_test_write_commit_graph_or_die(void) if (git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0)) flags = COMMIT_GRAPH_WRITE_BLOOM_FILTERS; - if (write_commit_graph_reachable(the_repository->objects->sources, - flags, NULL)) + if (write_commit_graph_reachable(source, flags, NULL)) die("failed to write commit-graph under GIT_TEST_COMMIT_GRAPH"); } @@ -597,7 +595,7 @@ static int add_graph_to_chain(struct commit_graph *g, if (!cur_g || !oideq(&oids[n], &cur_g->oid) || !hasheq(oids[n].hash, g->chunk_base_graphs + st_mult(g->hash_algo->rawsz, n), - the_repository->hash_algo)) { + g->hash_algo)) { warning(_("commit-graph chain does not match")); return 0; } @@ -666,7 +664,7 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, if (strbuf_getline_lf(&line, fp) == EOF) break; - if (get_oid_hex(line.buf, &oids[i])) { + if (get_oid_hex_algop(line.buf, &oids[i], r->hash_algo)) { warning(_("invalid commit-graph chain: line '%s' not a hash"), line.buf); valid = 0; @@ -752,7 +750,7 @@ static void prepare_commit_graph_one(struct repository *r, * Return 1 if commit_graph is non-NULL, and 0 otherwise. * * On the first invocation, this function attempts to load the commit - * graph if the_repository is configured to have one. + * graph if the repository is configured to have one. */ static int prepare_commit_graph(struct repository *r) { @@ -873,7 +871,7 @@ static void load_oid_from_graph(struct commit_graph *g, lex_index = pos - g->num_commits_in_base; oidread(oid, g->chunk_oid_lookup + st_mult(g->hash_algo->rawsz, lex_index), - the_repository->hash_algo); + g->hash_algo); } static struct commit_list **insert_parent_or_die(struct repository *r, @@ -1116,7 +1114,7 @@ static struct tree *load_tree_for_commit(struct repository *r, st_mult(graph_data_width(g->hash_algo), graph_pos - g->num_commits_in_base); - oidread(&oid, commit_data, the_repository->hash_algo); + oidread(&oid, commit_data, g->hash_algo); set_commit_tree(c, lookup_tree(r, &oid)); return c->maybe_tree; @@ -1543,7 +1541,7 @@ static void close_reachable(struct write_commit_graph_context *ctx) if (ctx->report_progress) ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Loading known commits in commit graph"), ctx->oids.nr); for (i = 0; i < ctx->oids.nr; i++) { @@ -1561,7 +1559,7 @@ static void close_reachable(struct write_commit_graph_context *ctx) */ if (ctx->report_progress) ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Expanding reachable commits in commit graph"), 0); for (i = 0; i < ctx->oids.nr; i++) { @@ -1582,7 +1580,7 @@ static void close_reachable(struct write_commit_graph_context *ctx) if (ctx->report_progress) ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Clearing commit marks in commit graph"), ctx->oids.nr); for (i = 0; i < ctx->oids.nr; i++) { @@ -1700,7 +1698,7 @@ static void compute_topological_levels(struct write_commit_graph_context *ctx) if (ctx->report_progress) info.progress = ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Computing commit graph topological levels"), ctx->commits.nr); @@ -1735,7 +1733,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx) if (ctx->report_progress) info.progress = ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Computing commit graph generation numbers"), ctx->commits.nr); @@ -1812,7 +1810,7 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx) if (ctx->report_progress) progress = start_delayed_progress( - the_repository, + ctx->r, _("Computing commit changed paths Bloom filters"), ctx->commits.nr); @@ -1858,6 +1856,7 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx) } struct refs_cb_data { + struct repository *repo; struct oidset *commits; struct progress *progress; }; @@ -1870,9 +1869,9 @@ static int add_ref_to_set(const char *refname UNUSED, struct object_id peeled; struct refs_cb_data *data = (struct refs_cb_data *)cb_data; - if (!peel_iterated_oid(the_repository, oid, &peeled)) + if (!peel_iterated_oid(data->repo, oid, &peeled)) oid = &peeled; - if (odb_read_object_info(the_repository->objects, oid, NULL) == OBJ_COMMIT) + if (odb_read_object_info(data->repo->objects, oid, NULL) == OBJ_COMMIT) oidset_insert(data->commits, oid); display_progress(data->progress, oidset_size(data->commits)); @@ -1889,13 +1888,15 @@ int write_commit_graph_reachable(struct odb_source *source, int result; memset(&data, 0, sizeof(data)); + data.repo = source->odb->repo; data.commits = &commits; + if (flags & COMMIT_GRAPH_WRITE_PROGRESS) data.progress = start_delayed_progress( - the_repository, + source->odb->repo, _("Collecting referenced commits"), 0); - refs_for_each_ref(get_main_ref_store(the_repository), add_ref_to_set, + refs_for_each_ref(get_main_ref_store(source->odb->repo), add_ref_to_set, &data); stop_progress(&data.progress); @@ -1924,7 +1925,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx, "Finding commits for commit graph in %"PRIuMAX" packs", pack_indexes->nr), (uintmax_t)pack_indexes->nr); - ctx->progress = start_delayed_progress(the_repository, + ctx->progress = start_delayed_progress(ctx->r, progress_title.buf, 0); ctx->progress_done = 0; } @@ -1978,7 +1979,7 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx) { if (ctx->report_progress) ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Finding commits for commit graph among packed objects"), ctx->approx_nr_objects); for_each_packed_object(ctx->r, add_packed_commits, ctx, @@ -1997,7 +1998,7 @@ static void copy_oids_to_commits(struct write_commit_graph_context *ctx) ctx->num_extra_edges = 0; if (ctx->report_progress) ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Finding extra edges in commit graph"), ctx->oids.nr); oid_array_sort(&ctx->oids); @@ -2076,7 +2077,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) ctx->graph_name = get_commit_graph_filename(ctx->odb_source); } - if (safe_create_leading_directories(the_repository, ctx->graph_name)) { + if (safe_create_leading_directories(ctx->r, ctx->graph_name)) { error(_("unable to create leading directories of %s"), ctx->graph_name); return -1; @@ -2095,18 +2096,18 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) return -1; } - if (adjust_shared_perm(the_repository, get_tempfile_path(graph_layer))) { + if (adjust_shared_perm(ctx->r, get_tempfile_path(graph_layer))) { error(_("unable to adjust shared permissions for '%s'"), get_tempfile_path(graph_layer)); return -1; } - f = hashfd(the_repository->hash_algo, + f = hashfd(ctx->r->hash_algo, get_tempfile_fd(graph_layer), get_tempfile_path(graph_layer)); } else { hold_lock_file_for_update_mode(&lk, ctx->graph_name, LOCK_DIE_ON_ERROR, 0444); - f = hashfd(the_repository->hash_algo, + f = hashfd(ctx->r->hash_algo, get_lock_file_fd(&lk), get_lock_file_path(&lk)); } @@ -2159,7 +2160,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) get_num_chunks(cf)), get_num_chunks(cf)); ctx->progress = start_delayed_progress( - the_repository, + ctx->r, progress_title.buf, st_mult(get_num_chunks(cf), ctx->commits.nr)); } @@ -2217,7 +2218,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) } free(ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); - ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(hash_to_hex(file_hash)); + ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = + xstrdup(hash_to_hex_algop(file_hash, ctx->r->hash_algo)); final_graph_name = get_split_graph_filename(ctx->odb_source, ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); free(ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1]); @@ -2372,7 +2374,7 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx) if (ctx->report_progress) ctx->progress = start_delayed_progress( - the_repository, + ctx->r, _("Scanning merged commits"), ctx->commits.nr); @@ -2417,7 +2419,7 @@ static void merge_commit_graphs(struct write_commit_graph_context *ctx) current_graph_number--; if (ctx->report_progress) - ctx->progress = start_delayed_progress(the_repository, + ctx->progress = start_delayed_progress(ctx->r, _("Merging commit-graph"), 0); merge_commit_graph(ctx, g); @@ -2520,7 +2522,7 @@ int write_commit_graph(struct odb_source *source, enum commit_graph_write_flags flags, const struct commit_graph_opts *opts) { - struct repository *r = the_repository; + struct repository *r = source->odb->repo; struct write_commit_graph_context ctx = { .r = r, .odb_source = source, @@ -2620,14 +2622,14 @@ int write_commit_graph(struct odb_source *source, replace = ctx.opts->split_flags & COMMIT_GRAPH_SPLIT_REPLACE; } - ctx.approx_nr_objects = repo_approximate_object_count(the_repository); + ctx.approx_nr_objects = repo_approximate_object_count(r); if (ctx.append && ctx.r->objects->commit_graph) { struct commit_graph *g = ctx.r->objects->commit_graph; for (i = 0; i < g->num_commits; i++) { struct object_id oid; oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_algo->rawsz, i), - the_repository->hash_algo); + r->hash_algo); oid_array_append(&ctx.oids, &oid); } } @@ -2735,7 +2737,7 @@ static void graph_report(const char *fmt, ...) static int commit_graph_checksum_valid(struct commit_graph *g) { - return hashfile_checksum_valid(the_repository->hash_algo, + return hashfile_checksum_valid(g->hash_algo, g->data, g->data_len); } @@ -2758,7 +2760,7 @@ static int verify_one_commit_graph(struct repository *r, struct commit *graph_commit; oidread(&cur_oid, g->chunk_oid_lookup + st_mult(g->hash_algo->rawsz, i), - the_repository->hash_algo); + g->hash_algo); if (i && oidcmp(&prev_oid, &cur_oid) >= 0) graph_report(_("commit-graph has incorrect OID order: %s then %s"), @@ -2803,7 +2805,7 @@ static int verify_one_commit_graph(struct repository *r, display_progress(progress, ++(*seen)); oidread(&cur_oid, g->chunk_oid_lookup + st_mult(g->hash_algo->rawsz, i), - the_repository->hash_algo); + g->hash_algo); graph_commit = lookup_commit(r, &cur_oid); odb_commit = (struct commit *)create_object(r, &cur_oid, alloc_commit_node(r)); @@ -2907,7 +2909,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW)) total += g->num_commits_in_base; - progress = start_progress(the_repository, + progress = start_progress(r, _("Verifying commits in commit graph"), total); } diff --git a/commit-graph.h b/commit-graph.h index df10daf01c..0a67ac9280 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -21,7 +21,7 @@ * call this method oustide of a builtin, and only if you know what * you are doing! */ -void git_test_write_commit_graph_or_die(void); +void git_test_write_commit_graph_or_die(struct odb_source *source); struct commit; struct bloom_filter_settings; -- cgit v1.2.3 From 7be9e410b22b3544e01d32f7bef8e6aa9516e152 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 15 Aug 2025 07:49:52 +0200 Subject: commit-graph: stop passing in redundant repository Many of the commit-graph related functions take in both a repository and the object database source (directly or via `struct commit_graph`) for which we are supposed to load such a commit-graph. In the best case this information is simply redundant as the source already contains a reference to its owning object database, which in turn has a reference to its repository. In the worst case this information could even mismatch when passing in a source that doesn't belong to the same repository. Refactor the code so that we only pass in the object database source in those cases. There is one exception though, namely `load_commit_graph_chain_fd_st()`, which is responsible for loading a commit-graph chain. It is expected that parts of the commit-graph chain aren't located in the same object source as the chain file itself, but in a different one. Consequently, this function doesn't work on the source level but on the database level instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/commit-graph.c | 6 +-- commit-graph.c | 120 +++++++++++++++++++-------------------------- commit-graph.h | 12 ++--- t/helper/test-read-graph.c | 2 +- 4 files changed, 59 insertions(+), 81 deletions(-) (limited to 'commit-graph.h') diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index f5c6f863a5..6656187f90 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -122,15 +122,15 @@ static int graph_verify(int argc, const char **argv, const char *prefix, if (opened == OPENED_NONE) return 0; else if (opened == OPENED_GRAPH) - graph = load_commit_graph_one_fd_st(the_repository, fd, &st, source); + graph = load_commit_graph_one_fd_st(source, fd, &st); else - graph = load_commit_graph_chain_fd_st(the_repository, fd, &st, + graph = load_commit_graph_chain_fd_st(the_repository->objects, fd, &st, &incomplete_chain); if (!graph) return 1; - ret = verify_commit_graph(the_repository, graph, flags); + ret = verify_commit_graph(graph, flags); free_commit_graph(graph); if (incomplete_chain) { diff --git a/commit-graph.c b/commit-graph.c index d6f0bf5e88..3cd9e73e2a 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -253,9 +253,8 @@ int open_commit_graph(const char *graph_file, int *fd, struct stat *st) return 1; } -struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, - int fd, struct stat *st, - struct odb_source *source) +struct commit_graph *load_commit_graph_one_fd_st(struct odb_source *source, + int fd, struct stat *st) { void *graph_map; size_t graph_size; @@ -263,7 +262,7 @@ struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, graph_size = xsize_t(st->st_size); - if (graph_size < graph_min_size(r->hash_algo)) { + if (graph_size < graph_min_size(source->odb->repo->hash_algo)) { close(fd); error(_("commit-graph file is too small")); return NULL; @@ -271,7 +270,7 @@ 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); + ret = parse_commit_graph(source->odb->repo, graph_map, graph_size); if (ret) ret->odb_source = source; else @@ -491,11 +490,9 @@ free_and_return: return NULL; } -static struct commit_graph *load_commit_graph_one(struct repository *r, - const char *graph_file, - struct odb_source *source) +static struct commit_graph *load_commit_graph_one(struct odb_source *source, + const char *graph_file) { - struct stat st; int fd; struct commit_graph *g; @@ -504,19 +501,17 @@ static struct commit_graph *load_commit_graph_one(struct repository *r, if (!open_ok) return NULL; - g = load_commit_graph_one_fd_st(r, fd, &st, source); - + g = load_commit_graph_one_fd_st(source, fd, &st); if (g) g->filename = xstrdup(graph_file); return g; } -static struct commit_graph *load_commit_graph_v1(struct repository *r, - struct odb_source *source) +static struct commit_graph *load_commit_graph_v1(struct odb_source *source) { char *graph_name = get_commit_graph_filename(source); - struct commit_graph *g = load_commit_graph_one(r, graph_name, source); + struct commit_graph *g = load_commit_graph_one(source, graph_name); free(graph_name); return g; @@ -643,7 +638,7 @@ int open_commit_graph_chain(const char *chain_file, return 1; } -struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, +struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb, int fd, struct stat *st, int *incomplete_chain) { @@ -653,10 +648,10 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, int i = 0, valid = 1, count; FILE *fp = xfdopen(fd, "r"); - count = st->st_size / (r->hash_algo->hexsz + 1); + count = st->st_size / (odb->repo->hash_algo->hexsz + 1); CALLOC_ARRAY(oids, count); - odb_prepare_alternates(r->objects); + odb_prepare_alternates(odb); for (i = 0; i < count; i++) { struct odb_source *source; @@ -664,7 +659,7 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, if (strbuf_getline_lf(&line, fp) == EOF) break; - if (get_oid_hex_algop(line.buf, &oids[i], r->hash_algo)) { + if (get_oid_hex_algop(line.buf, &oids[i], odb->repo->hash_algo)) { warning(_("invalid commit-graph chain: line '%s' not a hash"), line.buf); valid = 0; @@ -672,9 +667,9 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, } valid = 0; - for (source = r->objects->sources; source; source = source->next) { + for (source = odb->sources; source; source = source->next) { char *graph_name = get_split_graph_filename(source, line.buf); - struct commit_graph *g = load_commit_graph_one(r, graph_name, source); + struct commit_graph *g = load_commit_graph_one(source, graph_name); free(graph_name); @@ -707,45 +702,33 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, return graph_chain; } -static struct commit_graph *load_commit_graph_chain(struct repository *r, - struct odb_source *source) +static struct commit_graph *load_commit_graph_chain(struct odb_source *source) { char *chain_file = get_commit_graph_chain_filename(source); struct stat st; int fd; struct commit_graph *g = NULL; - if (open_commit_graph_chain(chain_file, &fd, &st, r->hash_algo)) { + if (open_commit_graph_chain(chain_file, &fd, &st, source->odb->repo->hash_algo)) { int incomplete; /* ownership of fd is taken over by load function */ - g = load_commit_graph_chain_fd_st(r, fd, &st, &incomplete); + g = load_commit_graph_chain_fd_st(source->odb, fd, &st, &incomplete); } free(chain_file); return g; } -struct commit_graph *read_commit_graph_one(struct repository *r, - struct odb_source *source) +struct commit_graph *read_commit_graph_one(struct odb_source *source) { - struct commit_graph *g = load_commit_graph_v1(r, source); + struct commit_graph *g = load_commit_graph_v1(source); if (!g) - g = load_commit_graph_chain(r, source); + g = load_commit_graph_chain(source); return g; } -static void prepare_commit_graph_one(struct repository *r, - struct odb_source *source) -{ - - if (r->objects->commit_graph) - return; - - r->objects->commit_graph = read_commit_graph_one(r, source); -} - /* * Return 1 if commit_graph is non-NULL, and 0 otherwise. * @@ -786,10 +769,12 @@ static int prepare_commit_graph(struct repository *r) return 0; odb_prepare_alternates(r->objects); - for (source = r->objects->sources; - !r->objects->commit_graph && source; - source = source->next) - prepare_commit_graph_one(r, source); + for (source = r->objects->sources; source; source = source->next) { + r->objects->commit_graph = read_commit_graph_one(source); + if (r->objects->commit_graph) + break; + } + return !!r->objects->commit_graph; } @@ -874,8 +859,7 @@ static void load_oid_from_graph(struct commit_graph *g, g->hash_algo); } -static struct commit_list **insert_parent_or_die(struct repository *r, - struct commit_graph *g, +static struct commit_list **insert_parent_or_die(struct commit_graph *g, uint32_t pos, struct commit_list **pptr) { @@ -886,7 +870,7 @@ static struct commit_list **insert_parent_or_die(struct repository *r, die("invalid parent position %"PRIu32, pos); load_oid_from_graph(g, pos, &oid); - c = lookup_commit(r, &oid); + c = lookup_commit(g->odb_source->odb->repo, &oid); if (!c) die(_("could not find commit %s"), oid_to_hex(&oid)); commit_graph_data_at(c)->graph_pos = pos; @@ -942,8 +926,7 @@ static inline void set_commit_tree(struct commit *c, struct tree *t) c->maybe_tree = t; } -static int fill_commit_in_graph(struct repository *r, - struct commit *item, +static int fill_commit_in_graph(struct commit *item, struct commit_graph *g, uint32_t pos) { uint32_t edge_value; @@ -969,13 +952,13 @@ static int fill_commit_in_graph(struct repository *r, edge_value = get_be32(commit_data + g->hash_algo->rawsz); if (edge_value == GRAPH_PARENT_NONE) return 1; - pptr = insert_parent_or_die(r, g, edge_value, pptr); + pptr = insert_parent_or_die(g, edge_value, pptr); edge_value = get_be32(commit_data + g->hash_algo->rawsz + 4); if (edge_value == GRAPH_PARENT_NONE) return 1; if (!(edge_value & GRAPH_EXTRA_EDGES_NEEDED)) { - pptr = insert_parent_or_die(r, g, edge_value, pptr); + pptr = insert_parent_or_die(g, edge_value, pptr); return 1; } @@ -990,7 +973,7 @@ static int fill_commit_in_graph(struct repository *r, } edge_value = get_be32(g->chunk_extra_edges + sizeof(uint32_t) * parent_data_pos); - pptr = insert_parent_or_die(r, g, + pptr = insert_parent_or_die(g, edge_value & GRAPH_EDGE_LAST_MASK, pptr); parent_data_pos++; @@ -1056,14 +1039,13 @@ struct commit *lookup_commit_in_graph(struct repository *repo, const struct obje if (commit->object.parsed) return commit; - if (!fill_commit_in_graph(repo, commit, repo->objects->commit_graph, pos)) + if (!fill_commit_in_graph(commit, repo->objects->commit_graph, pos)) return NULL; return commit; } -static int parse_commit_in_graph_one(struct repository *r, - struct commit_graph *g, +static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item) { uint32_t pos; @@ -1072,7 +1054,7 @@ static int parse_commit_in_graph_one(struct repository *r, return 1; if (find_commit_pos_in_graph(item, g, &pos)) - return fill_commit_in_graph(r, item, g, pos); + return fill_commit_in_graph(item, g, pos); return 0; } @@ -1089,7 +1071,7 @@ int parse_commit_in_graph(struct repository *r, struct commit *item) if (!prepare_commit_graph(r)) return 0; - return parse_commit_in_graph_one(r, r->objects->commit_graph, item); + return parse_commit_in_graph_one(r->objects->commit_graph, item); } void load_commit_graph_info(struct repository *r, struct commit *item) @@ -1099,8 +1081,7 @@ void load_commit_graph_info(struct repository *r, struct commit *item) fill_commit_graph_info(item, r->objects->commit_graph, pos); } -static struct tree *load_tree_for_commit(struct repository *r, - struct commit_graph *g, +static struct tree *load_tree_for_commit(struct commit_graph *g, struct commit *c) { struct object_id oid; @@ -1115,13 +1096,12 @@ static struct tree *load_tree_for_commit(struct repository *r, graph_pos - g->num_commits_in_base); oidread(&oid, commit_data, g->hash_algo); - set_commit_tree(c, lookup_tree(r, &oid)); + set_commit_tree(c, lookup_tree(g->odb_source->odb->repo, &oid)); return c->maybe_tree; } -static struct tree *get_commit_tree_in_graph_one(struct repository *r, - struct commit_graph *g, +static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g, const struct commit *c) { if (c->maybe_tree) @@ -1129,12 +1109,12 @@ static struct tree *get_commit_tree_in_graph_one(struct repository *r, if (commit_graph_position(c) == COMMIT_NOT_FROM_GRAPH) BUG("get_commit_tree_in_graph_one called from non-commit-graph commit"); - return load_tree_for_commit(r, g, (struct commit *)c); + return load_tree_for_commit(g, (struct commit *)c); } struct tree *get_commit_tree_in_graph(struct repository *r, const struct commit *c) { - return get_commit_tree_in_graph_one(r, r->objects->commit_graph, c); + return get_commit_tree_in_graph_one(r->objects->commit_graph, c); } struct packed_commit_list { @@ -2741,11 +2721,11 @@ static int commit_graph_checksum_valid(struct commit_graph *g) g->data, g->data_len); } -static int verify_one_commit_graph(struct repository *r, - struct commit_graph *g, +static int verify_one_commit_graph(struct commit_graph *g, struct progress *progress, uint64_t *seen) { + struct repository *r = g->odb_source->odb->repo; uint32_t i, cur_fanout_pos = 0; struct object_id prev_oid, cur_oid; struct commit *seen_gen_zero = NULL; @@ -2779,7 +2759,7 @@ static int verify_one_commit_graph(struct repository *r, } graph_commit = lookup_commit(r, &cur_oid); - if (!parse_commit_in_graph_one(r, g, graph_commit)) + if (!parse_commit_in_graph_one(g, graph_commit)) graph_report(_("failed to parse commit %s from commit-graph"), oid_to_hex(&cur_oid)); } @@ -2815,7 +2795,7 @@ static int verify_one_commit_graph(struct repository *r, continue; } - if (!oideq(&get_commit_tree_in_graph_one(r, g, graph_commit)->object.oid, + if (!oideq(&get_commit_tree_in_graph_one(g, graph_commit)->object.oid, get_commit_tree_oid(odb_commit))) graph_report(_("root tree OID for commit %s in commit-graph is %s != %s"), oid_to_hex(&cur_oid), @@ -2833,7 +2813,7 @@ static int verify_one_commit_graph(struct repository *r, } /* parse parent in case it is in a base graph */ - parse_commit_in_graph_one(r, g, graph_parents->item); + parse_commit_in_graph_one(g, graph_parents->item); if (!oideq(&graph_parents->item->object.oid, &odb_parents->item->object.oid)) graph_report(_("commit-graph parent for %s is %s != %s"), @@ -2893,7 +2873,7 @@ static int verify_one_commit_graph(struct repository *r, return verify_commit_graph_error; } -int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) +int verify_commit_graph(struct commit_graph *g, int flags) { struct progress *progress = NULL; int local_error = 0; @@ -2909,13 +2889,13 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW)) total += g->num_commits_in_base; - progress = start_progress(r, + progress = start_progress(g->odb_source->odb->repo, _("Verifying commits in commit graph"), total); } for (; g; g = g->base_graph) { - local_error |= verify_one_commit_graph(r, g, progress, &seen); + local_error |= verify_one_commit_graph(g, progress, &seen); if (flags & COMMIT_GRAPH_VERIFY_SHALLOW) break; } diff --git a/commit-graph.h b/commit-graph.h index 0a67ac9280..4899b54ef8 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -114,14 +114,12 @@ struct commit_graph { struct bloom_filter_settings *bloom_filter_settings; }; -struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, - int fd, struct stat *st, - struct odb_source *source); -struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, +struct commit_graph *load_commit_graph_one_fd_st(struct odb_source *source, + int fd, struct stat *st); +struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb, int fd, struct stat *st, int *incomplete_chain); -struct commit_graph *read_commit_graph_one(struct repository *r, - struct odb_source *source); +struct commit_graph *read_commit_graph_one(struct odb_source *source); struct repo_settings; @@ -185,7 +183,7 @@ int write_commit_graph(struct odb_source *source, #define COMMIT_GRAPH_VERIFY_SHALLOW (1 << 0) -int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags); +int verify_commit_graph(struct commit_graph *g, int flags); void close_commit_graph(struct object_database *); void free_commit_graph(struct commit_graph *); diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c index ef5339bbee..6a5f64e473 100644 --- a/t/helper/test-read-graph.c +++ b/t/helper/test-read-graph.c @@ -81,7 +81,7 @@ int cmd__read_graph(int argc, const char **argv) prepare_repo_settings(the_repository); - graph = read_commit_graph_one(the_repository, source); + graph = read_commit_graph_one(source); if (!graph) { ret = 1; goto done; -- cgit v1.2.3