commit-graph.h: replace 'commit_hex' with 'commits'

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index e0dd2876a15366371f636fca3aacafcdedb20279..075f8f6928317ac510c662ddcd58ac6c1881c082 100644 (file)
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -137,7 +137,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;
@@ -210,7 +210,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;
                }
 
@@ -219,7 +232,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 36e1d6f9b36cd5566d7830b51330033a61ff1c28..bf86e4a92b6f2a0f66f1b48332c74f7ff47d0f32 100644 (file)
--- 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);
 
@@ -1785,7 +1792,7 @@ out:
 
 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)
 {
@@ -1861,12 +1868,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 718433d79b772a437885141e44fc760e3720f7db..98ef121924486dc9442a1fa5b70cea5efd623e2d 100644 (file)
--- 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 0bf98b56ecbad79be20514fe0c4cccd787cfb0c6..69599cea7f9f370722d50978830a07fd00618ae4 100755 (executable)
--- 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