[PATCH 6/7] commit-graph.h: replace 'commit_hex' with 'commits'
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Apr 14, 2020, 04:04 UTC
- Message-ID
- <811a2ece05f2d576bf2b3d6429c537800649b27b.1586836700.git.me@ttaylorr.com>
- In-Reply-To
- <cover.1586836700.git.me@ttaylorr.com>
The 'write_commit_graph()' function takes in either a string list of pack indices, or a string list of hexadecimal commit OIDs. These correspond to the '--stdin-packs' and '--stdin-commits' mode(s) from 'git commit-graph write'.
Using a string_list of hexadecimal commit IDs is not the most efficient use of memory, since we can instead use the 'struct oidset', which is more well-suited for this case.
This has another benefit which will become apparent in the following commit. This is that we are about to disambiguate the kinds of errors we produce with '--stdin-commits' into "non-hex input" and "hex-input, but referring to a non-commit object". By having 'write_commit_graph' take in a 'struct oidset *' of commits, we place the burden on the caller (in this case, the builtin) to handle the first case, and the commit-graph machinery can handle the second case.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- builtin/commit-graph.c | 19 ++++++++++--- commit-graph.c | 59 +++++++++++++++++++++++------------------ commit-graph.h | 3 ++- t/t5318-commit-graph.sh | 2 +- 4 files changed, 52 insertions(+), 31 deletions(-)
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 922f876bfa..c69716aa7e 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -140,7 +140,7 @@ static int write_option_parse_split(const struct option *opt, const char *arg, static int graph_write(int argc, const char **argv) { struct string_list *pack_indexes = NULL; - struct string_list *commit_hex = NULL; + struct oidset commits = OIDSET_INIT; struct object_directory *odb = NULL; struct string_list lines; int result = 0; @@ -213,7 +213,20 @@ static int graph_write(int argc, const char **argv) if (opts.stdin_packs) pack_indexes = &lines; if (opts.stdin_commits) { - commit_hex = &lines; + struct string_list_item *item; + oidset_init(&commits, lines.nr); + for_each_string_list_item(item, &lines) { + struct object_id oid; + const char *end; + + if (parse_oid_hex(item->string, &oid, &end)) { + error(_("unexpected non-hex object ID: " + "%s"), item->string); + return 1; + } + + oidset_insert(&commits, &oid); + } flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS; } @@ -222,7 +235,7 @@ static int graph_write(int argc, const char **argv) if (write_commit_graph(odb, pack_indexes, - commit_hex, + opts.stdin_commits ? &commits : NULL, flags, &split_opts)) result = 1; diff --git a/commit-graph.c b/commit-graph.c index c598508d7f..f60346baee 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1136,13 +1136,13 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx) stop_progress(&ctx->progress); } -static int add_ref_to_list(const char *refname, - const struct object_id *oid, - int flags, void *cb_data) +static int add_ref_to_set(const char *refname, + const struct object_id *oid, + int flags, void *cb_data) { - struct string_list *list = (struct string_list *)cb_data; + struct oidset *commits = (struct oidset *)cb_data; - string_list_append(list, oid_to_hex(oid)); + oidset_insert(commits, oid); return 0; } @@ -1150,14 +1150,14 @@ int write_commit_graph_reachable(struct object_directory *odb, enum commit_graph_write_flags flags, const struct split_commit_graph_opts *split_opts) { - struct string_list list = STRING_LIST_INIT_DUP; + struct oidset commits = OIDSET_INIT; int result; - for_each_ref(add_ref_to_list, &list); - result = write_commit_graph(odb, NULL, &list, + for_each_ref(add_ref_to_set, &commits); + result = write_commit_graph(odb, NULL, &commits, flags, split_opts); - string_list_clear(&list, 0); + oidset_clear(&commits); return result; } @@ -1206,39 +1206,46 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx, return 0; } -static int fill_oids_from_commit_hex(struct write_commit_graph_context *ctx, - struct string_list *commit_hex) +static int fill_oids_from_commits(struct write_commit_graph_context *ctx, + struct oidset *commits) { - uint32_t i; + uint32_t i = 0; struct strbuf progress_title = STRBUF_INIT; + struct oidset_iter iter; + struct object_id *oid; + + if (!oidset_size(commits)) + return 0; if (ctx->report_progress) { strbuf_addf(&progress_title, Q_("Finding commits for commit graph from %d ref", "Finding commits for commit graph from %d refs", - commit_hex->nr), - commit_hex->nr); + oidset_size(commits)), + oidset_size(commits)); ctx->progress = start_delayed_progress( progress_title.buf, - commit_hex->nr); + oidset_size(commits)); } - for (i = 0; i < commit_hex->nr; i++) { - const char *end; - struct object_id oid; + + oidset_iter_init(commits, &iter); + while ((oid = oidset_iter_next(&iter))) { struct commit *result; - display_progress(ctx->progress, i + 1); - if (!parse_oid_hex(commit_hex->items[i].string, &oid, &end) && - (result = lookup_commit_reference_gently(ctx->r, &oid, 1))) { + display_progress(ctx->progress, ++i); + + result = lookup_commit_reference_gently(ctx->r, oid, 1); + if (result) { ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc); oidcpy(&ctx->oids.list[ctx->oids.nr], &(result->object.oid)); ctx->oids.nr++; } else if (ctx->check_oids) { error(_("invalid commit object id: %s"), - commit_hex->items[i].string); + oid_to_hex(oid)); return -1; } } + stop_progress(&ctx->progress); strbuf_release(&progress_title); @@ -1781,7 +1788,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx) int write_commit_graph(struct object_directory *odb, struct string_list *pack_indexes, - struct string_list *commit_hex, + struct oidset *commits, enum commit_graph_write_flags flags, const struct split_commit_graph_opts *split_opts) { @@ -1857,12 +1864,12 @@ int write_commit_graph(struct object_directory *odb, goto cleanup; } - if (commit_hex) { - if ((res = fill_oids_from_commit_hex(ctx, commit_hex))) + if (commits) { + if ((res = fill_oids_from_commits(ctx, commits))) goto cleanup; } - if (!pack_indexes && !commit_hex) + if (!pack_indexes && !commits) fill_oids_from_all_packs(ctx); close_reachable(ctx); diff --git a/commit-graph.h b/commit-graph.h index 718433d79b..98ef121924 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -6,6 +6,7 @@ #include "string-list.h" #include "cache.h" #include "object-store.h" +#include "oidset.h" #define GIT_TEST_COMMIT_GRAPH "GIT_TEST_COMMIT_GRAPH" #define GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD "GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD" @@ -106,7 +107,7 @@ int write_commit_graph_reachable(struct object_directory *odb, const struct split_commit_graph_opts *split_opts); int write_commit_graph(struct object_directory *odb, struct string_list *pack_indexes, - struct string_list *commit_hex, + struct oidset *commits, enum commit_graph_write_flags flags, const struct split_commit_graph_opts *split_opts); diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 9bf920ae17..e874a12696 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -43,7 +43,7 @@ test_expect_success 'create commits and repack' ' test_expect_success 'exit with correct error on bad input to --stdin-commits' ' cd "$TRASH_DIRECTORY/full" && echo HEAD | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr && - test_i18ngrep "invalid commit object id" stderr && + test_i18ngrep "unexpected non-hex object ID: HEAD" stderr && # valid tree OID, but not a commit OID git rev-parse HEAD^{tree} | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr && test_i18ngrep "invalid commit object id" stderr
-- 2.26.0.106.g9fadedd637