Merge branch 'jt/commit-graph-per-object-store'
authorJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:47 +0000 (15:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:47 +0000 (15:30 -0700)
The singleton commit-graph in-core instance is made per in-core
repository instance.

* jt/commit-graph-per-object-store:
commit-graph: add repo arg to graph readers
commit-graph: store graph in struct object_store
commit-graph: add free_commit_graph
commit-graph: add missing forward declaration
object-store: add missing include
commit-graph: refactor preparing commit graph

16 files changed:
Makefile
builtin/commit-graph.c
builtin/fsck.c
cache.h
commit-graph.c
commit-graph.h
commit.c
config.c
environment.c
object-store.h
object.c
ref-filter.c
t/helper/test-repository.c [new file with mode: 0644]
t/helper/test-tool.c
t/helper/test-tool.h
t/t5318-commit-graph.sh
index 74588d3ddaa0d1dd8365bbf441845db4f08cb166..bc4fc8eeab21862887c0b19560b07646e0c4848f 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -719,6 +719,7 @@ TEST_BUILTINS_OBJS += test-prio-queue.o
 TEST_BUILTINS_OBJS += test-read-cache.o
 TEST_BUILTINS_OBJS += test-ref-store.o
 TEST_BUILTINS_OBJS += test-regex.o
+TEST_BUILTINS_OBJS += test-repository.o
 TEST_BUILTINS_OBJS += test-revision-walking.o
 TEST_BUILTINS_OBJS += test-run-command.o
 TEST_BUILTINS_OBJS += test-scrap-cache-tree.o
index c7d0db5ab4b668618720eb30cffad904f0c57b14..0bf0c486575a8557b729658ce05be02d28a1a8b8 100644 (file)
@@ -115,6 +115,8 @@ static int graph_read(int argc, const char **argv)
                printf(" large_edges");
        printf("\n");
 
+       free_commit_graph(graph);
+
        return 0;
 }
 
index ea5e2a03e6c882c674dbef9b3f6e99e93b552f64..c96f3f4fccea0bbd78e4fe1b09e5d8c51e302bb0 100644 (file)
@@ -830,7 +830,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
        check_connectivity();
 
-       if (core_commit_graph) {
+       if (!git_config_get_bool("core.commitgraph", &i) && i) {
                struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
                const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL };
 
diff --git a/cache.h b/cache.h
index 87785fc83218cff355f02e2e92a7f819594e5b15..8dc7134f002e0f8bfe693d2679cda6a95fc7c118 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -877,7 +877,6 @@ extern char *git_replace_ref_base;
 
 extern int fsync_object_files;
 extern int core_preload_index;
-extern int core_commit_graph;
 extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
 extern int protect_hfs;
index 41a0133ff7506131d4966bd6bdee9cf6ea7614a2..b0a55ad128fbcaa2e34c25b8487856d20d0ad3d5 100644 (file)
@@ -183,53 +183,60 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
        exit(1);
 }
 
-/* global storage */
-static struct commit_graph *commit_graph = NULL;
-
-static void prepare_commit_graph_one(const char *obj_dir)
+static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
 {
        char *graph_name;
 
-       if (commit_graph)
+       if (r->objects->commit_graph)
                return;
 
        graph_name = get_commit_graph_filename(obj_dir);
-       commit_graph = load_commit_graph_one(graph_name);
+       r->objects->commit_graph =
+               load_commit_graph_one(graph_name);
 
        FREE_AND_NULL(graph_name);
 }
 
-static int prepare_commit_graph_run_once = 0;
-static void prepare_commit_graph(void)
+/*
+ * Return 1 if commit_graph is non-NULL, and 0 otherwise.
+ *
+ * On the first invocation, this function attemps to load the commit
+ * graph if the_repository is configured to have one.
+ */
+static int prepare_commit_graph(struct repository *r)
 {
        struct alternate_object_database *alt;
        char *obj_dir;
+       int config_value;
 
-       if (prepare_commit_graph_run_once)
-               return;
-       prepare_commit_graph_run_once = 1;
+       if (r->objects->commit_graph_attempted)
+               return !!r->objects->commit_graph;
+       r->objects->commit_graph_attempted = 1;
 
-       obj_dir = get_object_directory();
-       prepare_commit_graph_one(obj_dir);
-       prepare_alt_odb(the_repository);
-       for (alt = the_repository->objects->alt_odb_list;
-            !commit_graph && alt;
+       if (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
+           !config_value)
+               /*
+                * This repository is not configured to use commit graphs, so
+                * do not load one. (But report commit_graph_attempted anyway
+                * so that commit graph loading is not attempted again for this
+                * repository.)
+                */
+               return 0;
+
+       obj_dir = r->objects->objectdir;
+       prepare_commit_graph_one(r, obj_dir);
+       prepare_alt_odb(r);
+       for (alt = r->objects->alt_odb_list;
+            !r->objects->commit_graph && alt;
             alt = alt->next)
-               prepare_commit_graph_one(alt->path);
+               prepare_commit_graph_one(r, alt->path);
+       return !!r->objects->commit_graph;
 }
 
 static void close_commit_graph(void)
 {
-       if (!commit_graph)
-               return;
-
-       if (commit_graph->graph_fd >= 0) {
-               munmap((void *)commit_graph->data, commit_graph->data_len);
-               commit_graph->data = NULL;
-               close(commit_graph->graph_fd);
-       }
-
-       FREE_AND_NULL(commit_graph);
+       free_commit_graph(the_repository->objects->commit_graph);
+       the_repository->objects->commit_graph = NULL;
 }
 
 static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t *pos)
@@ -324,8 +331,6 @@ static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item
 {
        uint32_t pos;
 
-       if (!core_commit_graph)
-               return 0;
        if (item->object.parsed)
                return 1;
 
@@ -335,25 +340,20 @@ static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item
        return 0;
 }
 
-int parse_commit_in_graph(struct commit *item)
+int parse_commit_in_graph(struct repository *r, struct commit *item)
 {
-       if (!core_commit_graph)
+       if (!prepare_commit_graph(r))
                return 0;
-
-       prepare_commit_graph();
-       if (commit_graph)
-               return parse_commit_in_graph_one(commit_graph, item);
-       return 0;
+       return parse_commit_in_graph_one(r->objects->commit_graph, item);
 }
 
-void load_commit_graph_info(struct commit *item)
+void load_commit_graph_info(struct repository *r, struct commit *item)
 {
        uint32_t pos;
-       if (!core_commit_graph)
+       if (!prepare_commit_graph(r))
                return;
-       prepare_commit_graph();
-       if (commit_graph && find_commit_in_graph(item, commit_graph, &pos))
-               fill_commit_graph_info(item, commit_graph, pos);
+       if (find_commit_in_graph(item, r->objects->commit_graph, &pos))
+               fill_commit_graph_info(item, r->objects->commit_graph, pos);
 }
 
 static struct tree *load_tree_for_commit(struct commit_graph *g, struct commit *c)
@@ -379,9 +379,9 @@ static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g,
        return load_tree_for_commit(g, (struct commit *)c);
 }
 
-struct tree *get_commit_tree_in_graph(const struct commit *c)
+struct tree *get_commit_tree_in_graph(struct repository *r, const struct commit *c)
 {
-       return get_commit_tree_in_graph_one(commit_graph, c);
+       return get_commit_tree_in_graph_one(r->objects->commit_graph, c);
 }
 
 static void write_graph_chunk_fanout(struct hashfile *f,
@@ -697,16 +697,18 @@ void write_commit_graph(const char *obj_dir,
        oids.alloc = approximate_object_count() / 4;
 
        if (append) {
-               prepare_commit_graph_one(obj_dir);
-               if (commit_graph)
-                       oids.alloc += commit_graph->num_commits;
+               prepare_commit_graph_one(the_repository, obj_dir);
+               if (the_repository->objects->commit_graph)
+                       oids.alloc += the_repository->objects->commit_graph->num_commits;
        }
 
        if (oids.alloc < 1024)
                oids.alloc = 1024;
        ALLOC_ARRAY(oids.list, oids.alloc);
 
-       if (append && commit_graph) {
+       if (append && the_repository->objects->commit_graph) {
+               struct commit_graph *commit_graph =
+                       the_repository->objects->commit_graph;
                for (i = 0; i < commit_graph->num_commits; i++) {
                        const unsigned char *hash = commit_graph->chunk_oid_lookup +
                                commit_graph->hash_len * i;
@@ -1027,3 +1029,15 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
 
        return verify_commit_graph_error;
 }
+
+void free_commit_graph(struct commit_graph *g)
+{
+       if (!g)
+               return;
+       if (g->graph_fd >= 0) {
+               munmap((void *)g->data, g->data_len);
+               g->data = NULL;
+               close(g->graph_fd);
+       }
+       free(g);
+}
index 506cb45fb1178256b2d6c9752c28e300fc0791b2..76e098934a7f6740b52a479baa5c68fe605d1ea6 100644 (file)
@@ -5,6 +5,8 @@
 #include "repository.h"
 #include "string-list.h"
 
+struct commit;
+
 char *get_commit_graph_filename(const char *obj_dir);
 
 /*
@@ -17,7 +19,7 @@ char *get_commit_graph_filename(const char *obj_dir);
  *
  * See parse_commit_buffer() for the fallback after this call.
  */
-int parse_commit_in_graph(struct commit *item);
+int parse_commit_in_graph(struct repository *r, struct commit *item);
 
 /*
  * It is possible that we loaded commit contents from the commit buffer,
@@ -25,9 +27,10 @@ int parse_commit_in_graph(struct commit *item);
  * checked and filled. Fill the graph_pos and generation members of
  * the given commit.
  */
-void load_commit_graph_info(struct commit *item);
+void load_commit_graph_info(struct repository *r, struct commit *item);
 
-struct tree *get_commit_tree_in_graph(const struct commit *c);
+struct tree *get_commit_tree_in_graph(struct repository *r,
+                                     const struct commit *c);
 
 struct commit_graph {
        int graph_fd;
@@ -56,4 +59,6 @@ void write_commit_graph(const char *obj_dir,
 
 int verify_commit_graph(struct repository *r, struct commit_graph *g);
 
+void free_commit_graph(struct commit_graph *);
+
 #endif
index add310d4231d4cc71068b64827dc041aae0bb011..30d1af2b20660de99bed06486c11d1aef3763849 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -342,7 +342,7 @@ struct tree *get_commit_tree(const struct commit *commit)
        if (commit->graph_pos == COMMIT_NOT_FROM_GRAPH)
                BUG("commit has NULL tree, but was not loaded from commit-graph");
 
-       return get_commit_tree_in_graph(commit);
+       return get_commit_tree_in_graph(the_repository, commit);
 }
 
 struct object_id *get_commit_tree_oid(const struct commit *commit)
@@ -438,7 +438,7 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b
        item->date = parse_commit_date(bufptr, tail);
 
        if (check_graph)
-               load_commit_graph_info(item);
+               load_commit_graph_info(the_repository, item);
 
        return 0;
 }
@@ -454,7 +454,7 @@ int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_com
                return -1;
        if (item->object.parsed)
                return 0;
-       if (use_commit_graph && parse_commit_in_graph(item))
+       if (use_commit_graph && parse_commit_in_graph(the_repository, item))
                return 0;
        buffer = read_object_file(&item->object.oid, &type, &size);
        if (!buffer)
index 35a9bc79555ade317e965b031a7a8a402bddf43f..66645047eb3f9f84f185e0339807548fd562ea7d 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1320,11 +1320,6 @@ static int git_default_core_config(const char *var, const char *value)
                return 0;
        }
 
-       if (!strcmp(var, "core.commitgraph")) {
-               core_commit_graph = git_config_bool(var, value);
-               return 0;
-       }
-
        if (!strcmp(var, "core.sparsecheckout")) {
                core_apply_sparse_checkout = git_config_bool(var, value);
                return 0;
index 013e845235ea88e977c3181258b3e77e6ecb4c2f..6cf00793894f75230e5d66d39d56fe2e5388c8a6 100644 (file)
@@ -66,7 +66,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 char *notes_ref_name;
 int grafts_replace_parents = 1;
-int core_commit_graph;
 int core_apply_sparse_checkout;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
index a3db17bbf5a70c9920181e26eeba5c348ad87671..e481f7ad41bd876df3fb98f1578c38f55fc288e5 100644 (file)
@@ -2,6 +2,9 @@
 #define OBJECT_STORE_H
 
 #include "oidmap.h"
+#include "list.h"
+#include "sha1-array.h"
+#include "strbuf.h"
 
 struct alternate_object_database {
        struct alternate_object_database *next;
@@ -103,6 +106,9 @@ struct raw_object_store {
         */
        struct oidmap *replace_map;
 
+       struct commit_graph *commit_graph;
+       unsigned commit_graph_attempted : 1; /* if loading has been attempted */
+
        /*
         * private data
         *
index b0faab85d40ae8a04cc219085d1b70531f179c82..e2c112cc1a757f666ce3ca234227c98abf146910 100644 (file)
--- a/object.c
+++ b/object.c
@@ -9,6 +9,7 @@
 #include "alloc.h"
 #include "object-store.h"
 #include "packfile.h"
+#include "commit-graph.h"
 
 unsigned int get_max_object_index(void)
 {
@@ -507,6 +508,10 @@ void raw_object_store_clear(struct raw_object_store *o)
        oidmap_free(o->replace_map, 1);
        FREE_AND_NULL(o->replace_map);
 
+       free_commit_graph(o->commit_graph);
+       o->commit_graph = NULL;
+       o->commit_graph_attempted = 0;
+
        free_alt_odbs(o);
        o->alt_odb_tail = NULL;
 
index 600774de68a95ca416b069351cef4724ca7a9cd0..a8def7b3a36fde5fd4b7184b84f8ae36740642f5 100644 (file)
@@ -1713,7 +1713,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 
        for (p = want; p; p = p->next) {
                struct commit *c = p->item;
-               load_commit_graph_info(c);
+               load_commit_graph_info(the_repository, c);
                if (c->generation < cutoff)
                        cutoff = c->generation;
        }
diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
new file mode 100644 (file)
index 0000000..2762ca6
--- /dev/null
@@ -0,0 +1,82 @@
+#include "test-tool.h"
+#include "cache.h"
+#include "commit-graph.h"
+#include "commit.h"
+#include "config.h"
+#include "object-store.h"
+#include "object.h"
+#include "repository.h"
+#include "tree.h"
+
+static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,
+                                      const struct object_id *commit_oid)
+{
+       struct repository r;
+       struct commit *c;
+       struct commit_list *parent;
+
+       repo_init(&r, gitdir, worktree);
+
+       c = lookup_commit(&r, commit_oid);
+
+       if (!parse_commit_in_graph(&r, c))
+               die("Couldn't parse commit");
+
+       printf("%"PRItime, c->date);
+       for (parent = c->parents; parent; parent = parent->next)
+               printf(" %s", oid_to_hex(&parent->item->object.oid));
+       printf("\n");
+
+       repo_clear(&r);
+}
+
+static void test_get_commit_tree_in_graph(const char *gitdir,
+                                         const char *worktree,
+                                         const struct object_id *commit_oid)
+{
+       struct repository r;
+       struct commit *c;
+       struct tree *tree;
+
+       repo_init(&r, gitdir, worktree);
+
+       c = lookup_commit(&r, commit_oid);
+
+       /*
+        * get_commit_tree_in_graph does not automatically parse the commit, so
+        * parse it first.
+        */
+       if (!parse_commit_in_graph(&r, c))
+               die("Couldn't parse commit");
+       tree = get_commit_tree_in_graph(&r, c);
+       if (!tree)
+               die("Couldn't get commit tree");
+
+       printf("%s\n", oid_to_hex(&tree->object.oid));
+
+       repo_clear(&r);
+}
+
+int cmd__repository(int argc, const char **argv)
+{
+       if (argc < 2)
+               die("must have at least 2 arguments");
+       if (!strcmp(argv[1], "parse_commit_in_graph")) {
+               struct object_id oid;
+               if (argc < 5)
+                       die("not enough arguments");
+               if (parse_oid_hex(argv[4], &oid, &argv[4]))
+                       die("cannot parse oid '%s'", argv[4]);
+               test_parse_commit_in_graph(argv[2], argv[3], &oid);
+       } else if (!strcmp(argv[1], "get_commit_tree_in_graph")) {
+               struct object_id oid;
+               if (argc < 5)
+                       die("not enough arguments");
+               if (parse_oid_hex(argv[4], &oid, &argv[4]))
+                       die("cannot parse oid '%s'", argv[4]);
+               test_get_commit_tree_in_graph(argv[2], argv[3], &oid);
+       } else {
+               die("unrecognized '%s'", argv[1]);
+       }
+       return 0;
+}
index 805a45de9c877d9e5fecb6aa543e6ac985443d5b..dafc91c240a44079c82db1cf8a400650ccb5fbea 100644 (file)
@@ -29,6 +29,7 @@ static struct test_cmd cmds[] = {
        { "read-cache", cmd__read_cache },
        { "ref-store", cmd__ref_store },
        { "regex", cmd__regex },
+       { "repository", cmd__repository },
        { "revision-walking", cmd__revision_walking },
        { "run-command", cmd__run_command },
        { "scrap-cache-tree", cmd__scrap_cache_tree },
index 7116ddfb94398da98063b7efed77240058c6961f..80cbcf08575d5e365f8d025d5f3b1d99c6e4fec2 100644 (file)
@@ -23,6 +23,7 @@ int cmd__prio_queue(int argc, const char **argv);
 int cmd__read_cache(int argc, const char **argv);
 int cmd__ref_store(int argc, const char **argv);
 int cmd__regex(int argc, const char **argv);
+int cmd__repository(int argc, const char **argv);
 int cmd__revision_walking(int argc, const char **argv);
 int cmd__run_command(int argc, const char **argv);
 int cmd__scrap_cache_tree(int argc, const char **argv);
index 5947de3d2438ea301e33f2fc3430ee703d1436b9..4f17d7701e456bbb0992efdba2861e1dac8f7c23 100755 (executable)
@@ -431,4 +431,39 @@ test_expect_success 'git fsck (checks commit-graph)' '
        test_must_fail git fsck
 '
 
+test_expect_success 'setup non-the_repository tests' '
+       rm -rf repo &&
+       git init repo &&
+       test_commit -C repo one &&
+       test_commit -C repo two &&
+       git -C repo config core.commitGraph true &&
+       git -C repo rev-parse two | \
+               git -C repo commit-graph write --stdin-commits
+'
+
+test_expect_success 'parse_commit_in_graph works for non-the_repository' '
+       test-tool repository parse_commit_in_graph \
+               repo/.git repo "$(git -C repo rev-parse two)" >actual &&
+       echo $(git -C repo log --pretty="%ct" -1) \
+               $(git -C repo rev-parse one) >expect &&
+       test_cmp expect actual &&
+
+       test-tool repository parse_commit_in_graph \
+               repo/.git repo "$(git -C repo rev-parse one)" >actual &&
+       echo $(git -C repo log --pretty="%ct" -1 one) >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'get_commit_tree_in_graph works for non-the_repository' '
+       test-tool repository get_commit_tree_in_graph \
+               repo/.git repo "$(git -C repo rev-parse two)" >actual &&
+       echo $(git -C repo rev-parse two^{tree}) >expect &&
+       test_cmp expect actual &&
+
+       test-tool repository get_commit_tree_in_graph \
+               repo/.git repo "$(git -C repo rev-parse one)" >actual &&
+       echo $(git -C repo rev-parse one^{tree}) >expect &&
+       test_cmp expect actual
+'
+
 test_done