From 199d452758605a3ff15ac7d900653b4de7455e24 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:57 +0200 Subject: commit-graph: return the prepared commit graph from `prepare_commit_graph()` When making use of commit graphs, one needs to first prepare them by calling `prepare_commit_graph()`. Once that function was called and the commit graph was prepared successfully, the caller is now expected to access the graph directly via `struct object_database::commit_graph`. In a subsequent change, we're going to move the commit graph pointer from `struct object_database` into `struct odb_source`. With this change, semantics will change so that we use the commit graph of the first source that has one. Consequently, all callers that currently deference the `commit_graph` pointer would now have to loop around the list of sources to find the commit graph. This would become quite unwieldy. So instead of shifting the burden onto such callers, adapt `prepare_commit_graph()` to return the prepared commit graph, if any. Like this, callers are expected to call that function and then use the returned commit graph. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-graph.c | 82 +++++++++++++++++++++++----------------------------------- 1 file changed, 32 insertions(+), 50 deletions(-) (limited to 'commit-graph.c') diff --git a/commit-graph.c b/commit-graph.c index 3cd9e73e2a..62260a2026 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -735,7 +735,7 @@ struct commit_graph *read_commit_graph_one(struct odb_source *source) * On the first invocation, this function attempts to load the commit * graph if the repository is configured to have one. */ -static int prepare_commit_graph(struct repository *r) +static struct commit_graph *prepare_commit_graph(struct repository *r) { struct odb_source *source; @@ -747,10 +747,10 @@ static int prepare_commit_graph(struct repository *r) * we want to disable even an already-loaded graph file. */ if (!r->gitdir || r->commit_graph_disabled) - return 0; + return NULL; if (r->objects->commit_graph_attempted) - return !!r->objects->commit_graph; + return r->objects->commit_graph; r->objects->commit_graph_attempted = 1; prepare_repo_settings(r); @@ -763,10 +763,10 @@ static int prepare_commit_graph(struct repository *r) * so that commit graph loading is not attempted again for this * repository.) */ - return 0; + return NULL; if (!commit_graph_compatible(r)) - return 0; + return NULL; odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { @@ -775,20 +775,17 @@ static int prepare_commit_graph(struct repository *r) break; } - return !!r->objects->commit_graph; + return r->objects->commit_graph; } int generation_numbers_enabled(struct repository *r) { uint32_t first_generation; struct commit_graph *g; - if (!prepare_commit_graph(r)) - return 0; - g = r->objects->commit_graph; - - if (!g->num_commits) - return 0; + g = prepare_commit_graph(r); + if (!g || !g->num_commits) + return 0; first_generation = get_be32(g->chunk_commit_data + g->hash_algo->rawsz + 8) >> 2; @@ -799,12 +796,9 @@ int generation_numbers_enabled(struct repository *r) int corrected_commit_dates_enabled(struct repository *r) { struct commit_graph *g; - if (!prepare_commit_graph(r)) - return 0; - g = r->objects->commit_graph; - - if (!g->num_commits) + g = prepare_commit_graph(r); + if (!g || !g->num_commits) return 0; return g->read_generation_data; @@ -1012,23 +1006,26 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g, int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c, uint32_t *pos) { - if (!prepare_commit_graph(r)) + struct commit_graph *g = prepare_commit_graph(r); + if (!g) return 0; - return find_commit_pos_in_graph(c, r->objects->commit_graph, pos); + return find_commit_pos_in_graph(c, g, pos); } struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id) { static int commit_graph_paranoia = -1; + struct commit_graph *g; struct commit *commit; uint32_t pos; if (commit_graph_paranoia == -1) commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 0); - if (!prepare_commit_graph(repo)) + g = prepare_commit_graph(repo); + if (!g) return NULL; - if (!search_commit_pos_in_graph(id, repo->objects->commit_graph, &pos)) + if (!search_commit_pos_in_graph(id, g, &pos)) return NULL; if (commit_graph_paranoia && !odb_has_object(repo->objects, id, 0)) return NULL; @@ -1039,7 +1036,7 @@ struct commit *lookup_commit_in_graph(struct repository *repo, const struct obje if (commit->object.parsed) return commit; - if (!fill_commit_in_graph(commit, repo->objects->commit_graph, pos)) + if (!fill_commit_in_graph(commit, g, pos)) return NULL; return commit; @@ -1062,6 +1059,7 @@ static int parse_commit_in_graph_one(struct commit_graph *g, int parse_commit_in_graph(struct repository *r, struct commit *item) { static int checked_env = 0; + struct commit_graph *g; if (!checked_env && git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE, 0)) @@ -1069,9 +1067,10 @@ int parse_commit_in_graph(struct repository *r, struct commit *item) GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE); checked_env = 1; - if (!prepare_commit_graph(r)) + g = prepare_commit_graph(r); + if (!g) return 0; - return parse_commit_in_graph_one(r->objects->commit_graph, item); + return parse_commit_in_graph_one(g, item); } void load_commit_graph_info(struct repository *r, struct commit *item) @@ -2519,6 +2518,7 @@ int write_commit_graph(struct odb_source *source, int replace = 0; struct bloom_filter_settings bloom_settings = DEFAULT_BLOOM_FILTER_SETTINGS; struct topo_level_slab topo_levels; + struct commit_graph *g; prepare_repo_settings(r); if (!r->settings.core_commit_graph) { @@ -2547,23 +2547,13 @@ int write_commit_graph(struct odb_source *source, init_topo_level_slab(&topo_levels); ctx.topo_levels = &topo_levels; - prepare_commit_graph(ctx.r); - if (ctx.r->objects->commit_graph) { - struct commit_graph *g = ctx.r->objects->commit_graph; - - while (g) { - g->topo_levels = &topo_levels; - g = g->base_graph; - } - } + g = prepare_commit_graph(ctx.r); + for (struct commit_graph *chain = g; chain; chain = chain->base_graph) + g->topo_levels = &topo_levels; if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS) ctx.changed_paths = 1; if (!(flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) { - struct commit_graph *g; - - g = ctx.r->objects->commit_graph; - /* We have changed-paths already. Keep them in the next graph */ if (g && g->bloom_filter_settings) { ctx.changed_paths = 1; @@ -2580,22 +2570,15 @@ int write_commit_graph(struct odb_source *source, bloom_settings.hash_version = bloom_settings.hash_version == 2 ? 2 : 1; if (ctx.split) { - struct commit_graph *g = ctx.r->objects->commit_graph; - - while (g) { + for (struct commit_graph *chain = g; chain; chain = chain->base_graph) ctx.num_commit_graphs_before++; - g = g->base_graph; - } if (ctx.num_commit_graphs_before) { ALLOC_ARRAY(ctx.commit_graph_filenames_before, ctx.num_commit_graphs_before); i = ctx.num_commit_graphs_before; - g = ctx.r->objects->commit_graph; - while (g) { - ctx.commit_graph_filenames_before[--i] = xstrdup(g->filename); - g = g->base_graph; - } + for (struct commit_graph *chain = g; chain; chain = chain->base_graph) + ctx.commit_graph_filenames_before[--i] = xstrdup(chain->filename); } if (ctx.opts) @@ -2604,8 +2587,7 @@ int write_commit_graph(struct odb_source *source, 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; + if (ctx.append && g) { 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), @@ -2651,7 +2633,7 @@ int write_commit_graph(struct odb_source *source, } else ctx.num_commit_graphs_after = 1; - ctx.trust_generation_numbers = validate_mixed_generation_chain(ctx.r->objects->commit_graph); + ctx.trust_generation_numbers = validate_mixed_generation_chain(g); compute_topological_levels(&ctx); if (ctx.write_generation_data) -- cgit v1.2.3 From 88bc3500e5be888c13757d12c4a5cb16e39ec673 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:58 +0200 Subject: commit-graph: return commit graph from `repo_find_commit_pos_in_graph()` The function `repo_find_commit_pos_in_graph()` takes a commit as input and tries to figure out whether the given repository has a commit graph that contains that specific commit. If so, it returns the corresponding position of that commit inside the graph. Right now though we only return the position, but not the actual graph that the commit has been found in. This is sensible as repositories always have the graph in `struct repository::objects::commit_graph`. Consequently, the caller always knows where to find it. But in a subsequent change we're going to move the graph into the object sources. This would require callers of the function to loop through all sources to find the relevant commit graph. Refactor the code so that we instead return the commit-graph that the commit has been found with. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- bloom.c | 8 +++++--- commit-graph.c | 18 ++++++++++++------ commit-graph.h | 12 ++++++------ 3 files changed, 23 insertions(+), 15 deletions(-) (limited to 'commit-graph.c') diff --git a/bloom.c b/bloom.c index b86015f6d1..2d7b951e5b 100644 --- a/bloom.c +++ b/bloom.c @@ -452,10 +452,12 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r, filter = bloom_filter_slab_at(&bloom_filters, c); if (!filter->data) { + struct commit_graph *g; uint32_t graph_pos; - if (repo_find_commit_pos_in_graph(r, c, &graph_pos)) - load_bloom_filter_from_graph(r->objects->commit_graph, - filter, graph_pos); + + g = repo_find_commit_pos_in_graph(r, c, &graph_pos); + if (g) + load_bloom_filter_from_graph(g, filter, graph_pos); } if (filter->data && filter->len) { diff --git a/commit-graph.c b/commit-graph.c index 62260a2026..16dfe58229 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1003,13 +1003,16 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g, } } -int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c, - uint32_t *pos) +struct commit_graph *repo_find_commit_pos_in_graph(struct repository *r, + struct commit *c, + uint32_t *pos) { struct commit_graph *g = prepare_commit_graph(r); if (!g) - return 0; - return find_commit_pos_in_graph(c, g, pos); + return NULL; + if (!find_commit_pos_in_graph(c, g, pos)) + return NULL; + return g; } struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id) @@ -1075,9 +1078,12 @@ int parse_commit_in_graph(struct repository *r, struct commit *item) void load_commit_graph_info(struct repository *r, struct commit *item) { + struct commit_graph *g; uint32_t pos; - if (repo_find_commit_pos_in_graph(r, item, &pos)) - fill_commit_graph_info(item, r->objects->commit_graph, pos); + + g = repo_find_commit_pos_in_graph(r, item, &pos); + if (g) + fill_commit_graph_info(item, g, pos); } static struct tree *load_tree_for_commit(struct commit_graph *g, diff --git a/commit-graph.h b/commit-graph.h index 4899b54ef8..f6a5433641 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -48,10 +48,9 @@ int open_commit_graph_chain(const char *chain_file, int *fd, struct stat *st, int parse_commit_in_graph(struct repository *r, struct commit *item); /* - * Fills `*pos` with the graph position of `c`, and returns 1 if `c` is - * found in the commit-graph belonging to `r`, or 0 otherwise. - * Initializes the commit-graph belonging to `r` if it hasn't been - * already. + * Fills `*pos` with the graph position of `c`, and returns the graph `c` is + * found in, or NULL otherwise. Initializes the commit-graphs belonging to + * `r` if it hasn't been already. * * Note: this is a low-level helper that does not alter any slab data * associated with `c`. Useful in circumstances where the slab data is @@ -59,8 +58,9 @@ int parse_commit_in_graph(struct repository *r, struct commit *item); * * In most cases, callers should use `parse_commit_in_graph()` instead. */ -int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c, - uint32_t *pos); +struct commit_graph *repo_find_commit_pos_in_graph(struct repository *r, + struct commit *c, + uint32_t *pos); /* * Look up the given commit ID in the commit-graph. This will only return a -- cgit v1.2.3 From 62490b6d85882e6a0ba434ab436640e31352ffee Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:59 +0200 Subject: commit-graph: pass graphs that are to be merged as parameter When determining whether or not we want to merge a commit graph chain we retrieve the graph that is to be merged via the context's repository. With an upcoming change though it will become a bit more complex to figure out the commit graph, which would lead to code duplication. Prepare for this change by passing the graph that is to be merged as a parameter. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-graph.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'commit-graph.c') diff --git a/commit-graph.c b/commit-graph.c index 16dfe58229..0e25b14076 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2226,7 +2226,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) return 0; } -static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) +static void split_graph_merge_strategy(struct write_commit_graph_context *ctx, + struct commit_graph *graph_to_merge) { struct commit_graph *g; uint32_t num_commits; @@ -2245,7 +2246,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) flags = ctx->opts->split_flags; } - g = ctx->r->objects->commit_graph; + g = graph_to_merge; num_commits = ctx->commits.nr; if (flags == COMMIT_GRAPH_SPLIT_REPLACE) ctx->num_commit_graphs_after = 1; @@ -2297,7 +2298,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) ctx->commit_graph_filenames_after[i] = xstrdup(ctx->commit_graph_filenames_before[i]); i = ctx->num_commit_graphs_before - 1; - g = ctx->r->objects->commit_graph; + g = graph_to_merge; while (g) { if (i < ctx->num_commit_graphs_after) @@ -2395,9 +2396,9 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx) stop_progress(&ctx->progress); } -static void merge_commit_graphs(struct write_commit_graph_context *ctx) +static void merge_commit_graphs(struct write_commit_graph_context *ctx, + struct commit_graph *g) { - struct commit_graph *g = ctx->r->objects->commit_graph; uint32_t current_graph_number = ctx->num_commit_graphs_before; while (g && current_graph_number >= ctx->num_commit_graphs_after) { @@ -2632,12 +2633,13 @@ int write_commit_graph(struct odb_source *source, goto cleanup; if (ctx.split) { - split_graph_merge_strategy(&ctx); + split_graph_merge_strategy(&ctx, g); if (!replace) - merge_commit_graphs(&ctx); - } else + merge_commit_graphs(&ctx, g); + } else { ctx.num_commit_graphs_after = 1; + } ctx.trust_generation_numbers = validate_mixed_generation_chain(g); -- cgit v1.2.3