Junio C Hamano <gitster@xxxxxxxxx> writes: > Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: > >> When fetching into a repository, a connectivity check is first made by >> check_exist_and_connected() in builtin/fetch.c that runs: >> ... >> Another way to accomplish this effect would be to modify parse_object() >> to use the commit graph if possible; however, I did not want to change >> parse_object()'s current behavior of always checking the object >> signature of the returned object. > > Sounds good. > >> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx> >> --- >> This patch is now on master. > > OK. > > Obviously that won't apply to the base for v1 without conflicts, and > it of course applies cleanly on 'master', but the result of doing so > will cause the same conflicts when sb/more-repo-in-api is merged on > top, which means that the same conflicts need to be resolved if this > wants to be merged to 'next' (or 'pu', FWIW). So,... as I had to do the reverse rebase anyway, here is the difference since the previous round, which I came up with by comparing these two: (A) merge 'sb/more-repo-in-api' to 'master' and then merge v1 of this topic to the result. (B) apply your patch to 'master', and then merge 'sb/more-repo-in-api' to the result. diff --git a/commit-graph.c b/commit-graph.c index f78a8e96b5..74a17789f8 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -286,7 +286,8 @@ void close_commit_graph(struct repository *r) r->objects->commit_graph = NULL; } -static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t *pos) +static int bsearch_graph(struct commit_graph *g, const struct object_id *oid, + uint32_t *pos) { return bsearch_hash(oid->hash, g->chunk_oid_fanout, g->chunk_oid_lookup, g->hash_len, pos); @@ -377,26 +378,41 @@ static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uin } } -static int parse_commit_in_graph_one(struct repository *r, - struct commit_graph *g, - struct commit *item) +static struct commit *parse_commit_in_graph_one(struct repository *r, + struct commit_graph *g, + struct commit *shell, + const struct object_id *oid) { uint32_t pos; - if (item->object.parsed) - return 1; + if (shell && shell->object.parsed) + return shell; - if (find_commit_in_graph(item, g, &pos)) - return fill_commit_in_graph(r, item, g, pos); + if (shell && shell->graph_pos != COMMIT_NOT_FROM_GRAPH) { + pos = shell->graph_pos; + } else if (bsearch_graph(g, shell ? &shell->object.oid : oid, &pos)) { + /* bsearch_graph sets pos */ + } else { + return NULL; + } - return 0; + if (!shell) { + shell = lookup_commit(r, oid); + if (!shell) + return NULL; + } + + fill_commit_in_graph(r, shell, g, pos); + return shell; } -int parse_commit_in_graph(struct repository *r, struct commit *item) +struct commit *parse_commit_in_graph(struct repository *r, struct commit *shell, + const struct object_id *oid) { 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, r->objects->commit_graph, shell, + oid); } void load_commit_graph_info(struct repository *r, struct commit *item) @@ -1033,7 +1049,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g) } graph_commit = lookup_commit(r, &cur_oid); - if (!parse_commit_in_graph_one(r, g, graph_commit)) + if (!parse_commit_in_graph_one(r, g, graph_commit, NULL)) graph_report("failed to parse %s from commit-graph", oid_to_hex(&cur_oid)); } diff --git a/commit-graph.h b/commit-graph.h index 9db40b4d3a..8b7b5985dc 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -13,16 +13,20 @@ struct commit; char *get_commit_graph_filename(const char *obj_dir); /* - * Given a commit struct, try to fill the commit struct info, including: + * If the given commit (identified by shell->object.oid or oid) is in the + * commit graph, returns a commit struct (reusing shell if it is not NULL) + * including the following info: * 1. tree object * 2. date * 3. parents. * - * Returns 1 if and only if the commit was found in the packed graph. + * If not, returns NULL. See parse_commit_buffer() for the fallback after this + * call. * - * See parse_commit_buffer() for the fallback after this call. + * Either shell or oid must be non-NULL. If both are non-NULL, oid is ignored. */ -int parse_commit_in_graph(struct repository *r, struct commit *item); +struct commit *parse_commit_in_graph(struct repository *r, struct commit *shell, + const struct object_id *oid); /* * It is possible that we loaded commit contents from the commit buffer, diff --git a/commit.c b/commit.c index a5333c7ac6..da7a1d3262 100644 --- a/commit.c +++ b/commit.c @@ -462,7 +462,7 @@ int repo_parse_commit_internal(struct repository *r, return -1; if (item->object.parsed) return 0; - if (use_commit_graph && parse_commit_in_graph(r, item)) + if (use_commit_graph && parse_commit_in_graph(r, item, NULL)) return 0; buffer = repo_read_object_file(r, &item->object.oid, &type, &size); if (!buffer) diff --git a/revision.c b/revision.c index 22aa109c14..05fddb5880 100644 --- a/revision.c +++ b/revision.c @@ -213,19 +213,9 @@ static struct object *get_reference(struct rev_info *revs, const char *name, { struct object *object; - /* - * If the repository has commit graphs, repo_parse_commit() avoids - * reading the object buffer, so use it whenever possible. - */ - if (oid_object_info(revs->repo, oid, NULL) == OBJ_COMMIT) { - struct commit *c = lookup_commit(revs->repo, oid); - if (!repo_parse_commit(revs->repo, c)) - object = (struct object *) c; - else - object = NULL; - } else { + object = (struct object *) parse_commit_in_graph(revs->repo, NULL, oid); + if (!object) object = parse_object(revs->repo, oid); - } if (!object) { if (revs->ignore_missing) diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c index f7f8618445..689a0b652e 100644 --- a/t/helper/test-repository.c +++ b/t/helper/test-repository.c @@ -27,7 +27,7 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree, c = lookup_commit(&r, commit_oid); - if (!parse_commit_in_graph(&r, c)) + if (!parse_commit_in_graph(&r, c, NULL)) die("Couldn't parse commit"); printf("%"PRItime, c->date); @@ -62,7 +62,7 @@ static void test_get_commit_tree_in_graph(const char *gitdir, * get_commit_tree_in_graph does not automatically parse the commit, so * parse it first. */ - if (!parse_commit_in_graph(&r, c)) + if (!parse_commit_in_graph(&r, c, NULL)) die("Couldn't parse commit"); tree = get_commit_tree_in_graph(&r, c); if (!tree)