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 --- commit-graph.c | 120 ++++++++++++++++++++++++--------------------------------- 1 file changed, 50 insertions(+), 70 deletions(-) (limited to 'commit-graph.c') 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; } -- cgit v1.2.3