{"thread":{"id":"64090","subject":"[PATCH 0/6] odb: track commit graphs via object source","startedAt":"2025-09-04T12:50:04Z","lastAt":"2025-10-03T16:56:55Z","messageCount":26,"participants":["Patrick Steinhardt","Junio C Hamano","Derrick Stolee","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"525496","messageId":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","threadId":"64090","inReplyTo":null,"subject":"[PATCH 0/6] odb: track commit graphs via object source","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-04T12:49:54Z","receivedAt":"2025-09-04T12:50:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\ncommit graphs are currently stored on the object database level. This\ndoesn't really make much sense conceptually, given that commit graphs\nare specific to one object source. Furthermore, with the upcoming\npluggable object database effort, an object source's backend may not\nevene have a commit graph in the first place but store that information\nin a different format altogether.\n\nThis patch series prepares for that by moving the commit graph from\n`struct object_database` into `struct odb_source`.\n\nThere's a trivial conflict with tc/last-modified that can be solved like\nthis:\n\ndiff --cc commit-graph.c\nindex 9929c1ed87,2f20f66cfd..0000000000\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@@ -823,7 -812,12 +823,11 @@@ int corrected_commit_dates_enabled(stru\n  \n  struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)\n  {\n -\tstruct commit_graph *g;\n +\tstruct commit_graph *g = prepare_commit_graph(r);\n+ \n -\tif (!prepare_commit_graph(r))\n++\tif (!g)\n+ \t       return NULL;\n+ \n -\tg = r->objects->commit_graph;\n  \twhile (g) {\n  \t\tif (g->bloom_filter_settings)\n  \t\t\treturn g->bloom_filter_settings;\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (6):\n      blame: drop explicit check for commit graph\n      revision: drop explicit check for commit graph\n      commit-graph: return the prepared commit graph from `prepare_commit_graph()`\n      commit-graph: return commit graph from `repo_find_commit_pos_in_graph()`\n      commit-graph: pass graphs that are to be merged as parameter\n      odb: move commit-graph into the object sources\n\n blame.c        |   3 -\n bloom.c        |   8 ++-\n commit-graph.c | 177 +++++++++++++++++++++++++++++++--------------------------\n commit-graph.h |  14 ++---\n odb.c          |   9 +--\n odb.h          |   6 +-\n packfile.c     |   3 +-\n revision.c     |   3 -\n 8 files changed, 118 insertions(+), 105 deletions(-)\n\n\n---\nbase-commit: 2462961280690837670d997bde64bd4ebf8ae66d\nchange-id: 20250904-b4-pks-commit-graph-via-source-b4e1a1edd214\n\n"},{"id":"525497","messageId":"20250904-b4-pks-commit-graph-via-source-v1-1-d932c2481e1a@pks.im","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"[PATCH 1/6] blame: drop explicit check for commit graph","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-04T12:49:55Z","receivedAt":"2025-09-04T12:50:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Our blaming subsystem knows to use bloom filters from commit graphs to\nspeed up the whole computation. The setup of this happens in\n`setup_blame_bloom_data()`, where we first verify that we even have a\ncommit graph in the first place. This check is redundant though, as we\ncall `get_bloom_filter_settings()` immediately afterwards which, which\nalready knows to return a `NULL` pointer in case we don't have a commit\ngraph.\n\nDrop the redundant check.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n blame.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/blame.c b/blame.c\nindex f1c0670144..cb0b083423 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -2909,9 +2909,6 @@ void setup_blame_bloom_data(struct blame_scoreboard *sb)\n \tstruct blame_bloom_data *bd;\n \tstruct bloom_filter_settings *bs;\n \n-\tif (!sb->repo->objects->commit_graph)\n-\t\treturn;\n-\n \tbs = get_bloom_filter_settings(sb->repo);\n \tif (!bs)\n \t\treturn;\n\n-- \n2.51.0.417.g1ba7204a04.dirty\n\n"},{"id":"525498","messageId":"20250904-b4-pks-commit-graph-via-source-v1-2-d932c2481e1a@pks.im","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"[PATCH 2/6] revision: drop explicit check for commit graph","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-04T12:49:56Z","receivedAt":"2025-09-04T12:50:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When filtering down revisions by paths we know to use bloom filters from\nthe commit graph, if we have any. The entry point for this is in\n`check_maybe_different_in_bloom_filter()`, where we first verify that:\n\n  - We do have a commit graph.\n\n  - That the commit is contained therein by checking that we have a\n    proper generation number.\n\n  - And that the graph contains a bloom filter.\n\nThe first check is somewhat redundant though: if we don't have a commit\ngraph, then the second check would already tell us that we don't have a\ngeneration number for the specific commit.\n\nIn theory this could be seen as a performance optimization to\nshort-circuit for scenarios where there is no commit graph. But in\npractice this shouldn't matter: if there is no commit graph, then the\ncommit graph data slab would also be unpopulated and thus a lookup of\nthe commit should happen in constant time.\n\nDrop the unnecessary check.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n revision.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 6ba8f67054..6018f30a99 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -774,9 +774,6 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs,\n \tstruct bloom_filter *filter;\n \tint result = 0;\n \n-\tif (!revs->repo->objects->commit_graph)\n-\t\treturn -1;\n-\n \tif (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY)\n \t\treturn -1;\n \n\n-- \n2.51.0.417.g1ba7204a04.dirty\n\n"},{"id":"525499","messageId":"20250904-b4-pks-commit-graph-via-source-v1-3-d932c2481e1a@pks.im","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"[PATCH 3/6] commit-graph: return the prepared commit graph from `prepare_commit_graph()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-04T12:49:57Z","receivedAt":"2025-09-04T12:50:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When making use of commit graphs, one needs to first prepare them by\ncalling `prepare_commit_graph()`. Once that function was called and the\ncommit graph was prepared successfully, the caller is now expected to\naccess the graph directly via `struct object_database::commit_graph`.\n\nIn a subsequent change, we're going to move the commit graph pointer\nfrom `struct object_database` into `struct odb_source`. With this\nchange, semantics will change so that we use the commit graph of the\nfirst source that has one. Consequently, all callers that currently\ndeference the `commit_graph` pointer would now have to loop around the\nlist of sources to find the commit graph.\n\nThis would become quite unwieldy. So instead of shifting the burden onto\nsuch callers, adapt `prepare_commit_graph()` to return the prepared\ncommit graph, if any. Like this, callers are expected to call that\nfunction and then use the returned commit graph.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n commit-graph.c | 82 +++++++++++++++++++++++-----------------------------------\n 1 file changed, 32 insertions(+), 50 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 3cd9e73e2a..62260a2026 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -735,7 +735,7 @@ struct commit_graph *read_commit_graph_one(struct odb_source *source)\n  * On the first invocation, this function attempts to load the commit\n  * graph if the repository is configured to have one.\n  */\n-static int prepare_commit_graph(struct repository *r)\n+static struct commit_graph *prepare_commit_graph(struct repository *r)\n {\n \tstruct odb_source *source;\n \n@@ -747,10 +747,10 @@ static int prepare_commit_graph(struct repository *r)\n \t * we want to disable even an already-loaded graph file.\n \t */\n \tif (!r->gitdir || r->commit_graph_disabled)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tif (r->objects->commit_graph_attempted)\n-\t\treturn !!r->objects->commit_graph;\n+\t\treturn r->objects->commit_graph;\n \tr->objects->commit_graph_attempted = 1;\n \n \tprepare_repo_settings(r);\n@@ -763,10 +763,10 @@ static int prepare_commit_graph(struct repository *r)\n \t\t * so that commit graph loading is not attempted again for this\n \t\t * repository.)\n \t\t */\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tif (!commit_graph_compatible(r))\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n@@ -775,20 +775,17 @@ static int prepare_commit_graph(struct repository *r)\n \t\t\tbreak;\n \t}\n \n-\treturn !!r->objects->commit_graph;\n+\treturn r->objects->commit_graph;\n }\n \n int generation_numbers_enabled(struct repository *r)\n {\n \tuint32_t first_generation;\n \tstruct commit_graph *g;\n-\tif (!prepare_commit_graph(r))\n-\t       return 0;\n \n-\tg = r->objects->commit_graph;\n-\n-\tif (!g->num_commits)\n-\t\treturn 0;\n+\tg = prepare_commit_graph(r);\n+\tif (!g || !g->num_commits)\n+\t       return 0;\n \n \tfirst_generation = get_be32(g->chunk_commit_data +\n \t\t\t\t    g->hash_algo->rawsz + 8) >> 2;\n@@ -799,12 +796,9 @@ int generation_numbers_enabled(struct repository *r)\n int corrected_commit_dates_enabled(struct repository *r)\n {\n \tstruct commit_graph *g;\n-\tif (!prepare_commit_graph(r))\n-\t\treturn 0;\n \n-\tg = r->objects->commit_graph;\n-\n-\tif (!g->num_commits)\n+\tg = prepare_commit_graph(r);\n+\tif (!g || !g->num_commits)\n \t\treturn 0;\n \n \treturn g->read_generation_data;\n@@ -1012,23 +1006,26 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g,\n int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,\n \t\t\t\t  uint32_t *pos)\n {\n-\tif (!prepare_commit_graph(r))\n+\tstruct commit_graph *g = prepare_commit_graph(r);\n+\tif (!g)\n \t\treturn 0;\n-\treturn find_commit_pos_in_graph(c, r->objects->commit_graph, pos);\n+\treturn find_commit_pos_in_graph(c, g, pos);\n }\n \n struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id)\n {\n \tstatic int commit_graph_paranoia = -1;\n+\tstruct commit_graph *g;\n \tstruct commit *commit;\n \tuint32_t pos;\n \n \tif (commit_graph_paranoia == -1)\n \t\tcommit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 0);\n \n-\tif (!prepare_commit_graph(repo))\n+\tg = prepare_commit_graph(repo);\n+\tif (!g)\n \t\treturn NULL;\n-\tif (!search_commit_pos_in_graph(id, repo->objects->commit_graph, &pos))\n+\tif (!search_commit_pos_in_graph(id, g, &pos))\n \t\treturn NULL;\n \tif (commit_graph_paranoia && !odb_has_object(repo->objects, id, 0))\n \t\treturn NULL;\n@@ -1039,7 +1036,7 @@ struct commit *lookup_commit_in_graph(struct repository *repo, const struct obje\n \tif (commit->object.parsed)\n \t\treturn commit;\n \n-\tif (!fill_commit_in_graph(commit, repo->objects->commit_graph, pos))\n+\tif (!fill_commit_in_graph(commit, g, pos))\n \t\treturn NULL;\n \n \treturn commit;\n@@ -1062,6 +1059,7 @@ static int parse_commit_in_graph_one(struct commit_graph *g,\n int parse_commit_in_graph(struct repository *r, struct commit *item)\n {\n \tstatic int checked_env = 0;\n+\tstruct commit_graph *g;\n \n \tif (!checked_env &&\n \t    git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE, 0))\n@@ -1069,9 +1067,10 @@ int parse_commit_in_graph(struct repository *r, struct commit *item)\n \t\t    GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE);\n \tchecked_env = 1;\n \n-\tif (!prepare_commit_graph(r))\n+\tg = prepare_commit_graph(r);\n+\tif (!g)\n \t\treturn 0;\n-\treturn parse_commit_in_graph_one(r->objects->commit_graph, item);\n+\treturn parse_commit_in_graph_one(g, item);\n }\n \n void load_commit_graph_info(struct repository *r, struct commit *item)\n@@ -2519,6 +2518,7 @@ int write_commit_graph(struct odb_source *source,\n \tint replace = 0;\n \tstruct bloom_filter_settings bloom_settings = DEFAULT_BLOOM_FILTER_SETTINGS;\n \tstruct topo_level_slab topo_levels;\n+\tstruct commit_graph *g;\n \n \tprepare_repo_settings(r);\n \tif (!r->settings.core_commit_graph) {\n@@ -2547,23 +2547,13 @@ int write_commit_graph(struct odb_source *source,\n \tinit_topo_level_slab(&topo_levels);\n \tctx.topo_levels = &topo_levels;\n \n-\tprepare_commit_graph(ctx.r);\n-\tif (ctx.r->objects->commit_graph) {\n-\t\tstruct commit_graph *g = ctx.r->objects->commit_graph;\n-\n-\t\twhile (g) {\n-\t\t\tg->topo_levels = &topo_levels;\n-\t\t\tg = g->base_graph;\n-\t\t}\n-\t}\n+\tg = prepare_commit_graph(ctx.r);\n+\tfor (struct commit_graph *chain = g; chain; chain = chain->base_graph)\n+\t\tg->topo_levels = &topo_levels;\n \n \tif (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)\n \t\tctx.changed_paths = 1;\n \tif (!(flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) {\n-\t\tstruct commit_graph *g;\n-\n-\t\tg = ctx.r->objects->commit_graph;\n-\n \t\t/* We have changed-paths already. Keep them in the next graph */\n \t\tif (g && g->bloom_filter_settings) {\n \t\t\tctx.changed_paths = 1;\n@@ -2580,22 +2570,15 @@ int write_commit_graph(struct odb_source *source,\n \tbloom_settings.hash_version = bloom_settings.hash_version == 2 ? 2 : 1;\n \n \tif (ctx.split) {\n-\t\tstruct commit_graph *g = ctx.r->objects->commit_graph;\n-\n-\t\twhile (g) {\n+\t\tfor (struct commit_graph *chain = g; chain; chain = chain->base_graph)\n \t\t\tctx.num_commit_graphs_before++;\n-\t\t\tg = g->base_graph;\n-\t\t}\n \n \t\tif (ctx.num_commit_graphs_before) {\n \t\t\tALLOC_ARRAY(ctx.commit_graph_filenames_before, ctx.num_commit_graphs_before);\n \t\t\ti = ctx.num_commit_graphs_before;\n-\t\t\tg = ctx.r->objects->commit_graph;\n \n-\t\t\twhile (g) {\n-\t\t\t\tctx.commit_graph_filenames_before[--i] = xstrdup(g->filename);\n-\t\t\t\tg = g->base_graph;\n-\t\t\t}\n+\t\t\tfor (struct commit_graph *chain = g; chain; chain = chain->base_graph)\n+\t\t\t\tctx.commit_graph_filenames_before[--i] = xstrdup(chain->filename);\n \t\t}\n \n \t\tif (ctx.opts)\n@@ -2604,8 +2587,7 @@ int write_commit_graph(struct odb_source *source,\n \n \tctx.approx_nr_objects = repo_approximate_object_count(r);\n \n-\tif (ctx.append && ctx.r->objects->commit_graph) {\n-\t\tstruct commit_graph *g = ctx.r->objects->commit_graph;\n+\tif (ctx.append && g) {\n \t\tfor (i = 0; i < g->num_commits; i++) {\n \t\t\tstruct object_id oid;\n \t\t\toidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_algo->rawsz, i),\n@@ -2651,7 +2633,7 @@ int write_commit_graph(struct odb_source *source,\n \t} else\n \t\tctx.num_commit_graphs_after = 1;\n \n-\tctx.trust_generation_numbers = validate_mixed_generation_chain(ctx.r->objects->commit_graph);\n+\tctx.trust_generation_numbers = validate_mixed_generation_chain(g);\n \n \tcompute_topological_levels(&ctx);\n \tif (ctx.write_generation_data)\n\n-- \n2.51.0.417.g1ba7204a04.dirty\n\n"},{"id":"525500","messageId":"20250904-b4-pks-commit-graph-via-source-v1-4-d932c2481e1a@pks.im","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"[PATCH 4/6] commit-graph: return commit graph from `repo_find_commit_pos_in_graph()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-04T12:49:58Z","receivedAt":"2025-09-04T12:50:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `repo_find_commit_pos_in_graph()` takes a commit as input\nand tries to figure out whether the given repository has a commit graph\nthat contains that specific commit. If so, it returns the corresponding\nposition of that commit inside the graph.\n\nRight now though we only return the position, but not the actual graph\nthat the commit has been found in. This is sensible as repositories\nalways have the graph in `struct repository::objects::commit_graph`.\nConsequently, the caller always knows where to find it.\n\nBut in a subsequent change we're going to move the graph into the object\nsources. This would require callers of the function to loop through all\nsources to find the relevant commit graph.\n\nRefactor the code so that we instead return the commit-graph that the\ncommit has been found with.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n bloom.c        |  8 +++++---\n commit-graph.c | 18 ++++++++++++------\n commit-graph.h | 12 ++++++------\n 3 files changed, 23 insertions(+), 15 deletions(-)\n\ndiff --git a/bloom.c b/bloom.c\nindex b86015f6d1..2d7b951e5b 100644\n--- a/bloom.c\n+++ b/bloom.c\n@@ -452,10 +452,12 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,\n \tfilter = bloom_filter_slab_at(&bloom_filters, c);\n \n \tif (!filter->data) {\n+\t\tstruct commit_graph *g;\n \t\tuint32_t graph_pos;\n-\t\tif (repo_find_commit_pos_in_graph(r, c, &graph_pos))\n-\t\t\tload_bloom_filter_from_graph(r->objects->commit_graph,\n-\t\t\t\t\t\t     filter, graph_pos);\n+\n+\t\tg = repo_find_commit_pos_in_graph(r, c, &graph_pos);\n+\t\tif (g)\n+\t\t\tload_bloom_filter_from_graph(g, filter, graph_pos);\n \t}\n \n \tif (filter->data && filter->len) {\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 62260a2026..16dfe58229 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1003,13 +1003,16 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g,\n \t}\n }\n \n-int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,\n-\t\t\t\t  uint32_t *pos)\n+struct commit_graph *repo_find_commit_pos_in_graph(struct repository *r,\n+\t\t\t\t\t\t   struct commit *c,\n+\t\t\t\t\t\t   uint32_t *pos)\n {\n \tstruct commit_graph *g = prepare_commit_graph(r);\n \tif (!g)\n-\t\treturn 0;\n-\treturn find_commit_pos_in_graph(c, g, pos);\n+\t\treturn NULL;\n+\tif (!find_commit_pos_in_graph(c, g, pos))\n+\t\treturn NULL;\n+\treturn g;\n }\n \n struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id)\n@@ -1075,9 +1078,12 @@ int parse_commit_in_graph(struct repository *r, struct commit *item)\n \n void load_commit_graph_info(struct repository *r, struct commit *item)\n {\n+\tstruct commit_graph *g;\n \tuint32_t pos;\n-\tif (repo_find_commit_pos_in_graph(r, item, &pos))\n-\t\tfill_commit_graph_info(item, r->objects->commit_graph, pos);\n+\n+\tg = repo_find_commit_pos_in_graph(r, item, &pos);\n+\tif (g)\n+\t\tfill_commit_graph_info(item, g, pos);\n }\n \n static struct tree *load_tree_for_commit(struct commit_graph *g,\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 4899b54ef8..f6a5433641 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -48,10 +48,9 @@ int open_commit_graph_chain(const char *chain_file, int *fd, struct stat *st,\n int parse_commit_in_graph(struct repository *r, struct commit *item);\n \n /*\n- * Fills `*pos` with the graph position of `c`, and returns 1 if `c` is\n- * found in the commit-graph belonging to `r`, or 0 otherwise.\n- * Initializes the commit-graph belonging to `r` if it hasn't been\n- * already.\n+ * Fills `*pos` with the graph position of `c`, and returns the graph `c` is\n+ * found in, or NULL otherwise. Initializes the commit-graphs belonging to\n+ * `r` if it hasn't been already.\n  *\n  * Note: this is a low-level helper that does not alter any slab data\n  * associated with `c`. Useful in circumstances where the slab data is\n@@ -59,8 +58,9 @@ int parse_commit_in_graph(struct repository *r, struct commit *item);\n  *\n  * In most cases, callers should use `parse_commit_in_graph()` instead.\n  */\n-int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,\n-\t\t\t\t  uint32_t *pos);\n+struct commit_graph *repo_find_commit_pos_in_graph(struct repository *r,\n+\t\t\t\t\t\t   struct commit *c,\n+\t\t\t\t\t\t   uint32_t *pos);\n \n /*\n  * Look up the given commit ID in the commit-graph. This will only return a\n\n-- \n2.51.0.417.g1ba7204a04.dirty\n\n"},{"id":"525501","messageId":"20250904-b4-pks-commit-graph-via-source-v1-5-d932c2481e1a@pks.im","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"[PATCH 5/6] commit-graph: pass graphs that are to be merged as parameter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-04T12:49:59Z","receivedAt":"2025-09-04T12:50:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When determining whether or not we want to merge a commit graph chain we\nretrieve the graph that is to be merged via the context's repository.\nWith an upcoming change though it will become a bit more complex to\nfigure out the commit graph, which would lead to code duplication.\n\nPrepare for this change by passing the graph that is to be merged as a\nparameter.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n commit-graph.c | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 16dfe58229..0e25b14076 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -2226,7 +2226,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)\n \treturn 0;\n }\n \n-static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)\n+static void split_graph_merge_strategy(struct write_commit_graph_context *ctx,\n+\t\t\t\t       struct commit_graph *graph_to_merge)\n {\n \tstruct commit_graph *g;\n \tuint32_t num_commits;\n@@ -2245,7 +2246,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)\n \t\tflags = ctx->opts->split_flags;\n \t}\n \n-\tg = ctx->r->objects->commit_graph;\n+\tg = graph_to_merge;\n \tnum_commits = ctx->commits.nr;\n \tif (flags == COMMIT_GRAPH_SPLIT_REPLACE)\n \t\tctx->num_commit_graphs_after = 1;\n@@ -2297,7 +2298,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)\n \t\tctx->commit_graph_filenames_after[i] = xstrdup(ctx->commit_graph_filenames_before[i]);\n \n \ti = ctx->num_commit_graphs_before - 1;\n-\tg = ctx->r->objects->commit_graph;\n+\tg = graph_to_merge;\n \n \twhile (g) {\n \t\tif (i < ctx->num_commit_graphs_after)\n@@ -2395,9 +2396,9 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx)\n \tstop_progress(&ctx->progress);\n }\n \n-static void merge_commit_graphs(struct write_commit_graph_context *ctx)\n+static void merge_commit_graphs(struct write_commit_graph_context *ctx,\n+\t\t\t\tstruct commit_graph *g)\n {\n-\tstruct commit_graph *g = ctx->r->objects->commit_graph;\n \tuint32_t current_graph_number = ctx->num_commit_graphs_before;\n \n \twhile (g && current_graph_number >= ctx->num_commit_graphs_after) {\n@@ -2632,12 +2633,13 @@ int write_commit_graph(struct odb_source *source,\n \t\tgoto cleanup;\n \n \tif (ctx.split) {\n-\t\tsplit_graph_merge_strategy(&ctx);\n+\t\tsplit_graph_merge_strategy(&ctx, g);\n \n \t\tif (!replace)\n-\t\t\tmerge_commit_graphs(&ctx);\n-\t} else\n+\t\t\tmerge_commit_graphs(&ctx, g);\n+\t} else {\n \t\tctx.num_commit_graphs_after = 1;\n+\t}\n \n \tctx.trust_generation_numbers = validate_mixed_generation_chain(g);\n \n\n-- \n2.51.0.417.g1ba7204a04.dirty\n\n"},{"id":"525502","messageId":"20250904-b4-pks-commit-graph-via-source-v1-6-d932c2481e1a@pks.im","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"[PATCH 6/6] odb: move commit-graph into the object sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-04T12:50:00Z","receivedAt":"2025-09-04T12:50:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Commit graphs are inherently tied to one specific object source.\nFurthermore, with the upcoming pluggable object sources, it is not even\nguaranteed that an object source may even have a commit graph as these\nare specific to the actual on-disk data format.\n\nPrepare for this future by moving the commit-graph pointer from `struct\nobject_database` to `struct odb_source`. Eventually, this will allow us\nto make commit graphs an implementation detail of an object source's\nbackend.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n commit-graph.c | 65 +++++++++++++++++++++++++++++++++++++++++-----------------\n commit-graph.h |  2 +-\n odb.c          |  9 ++++----\n odb.h          |  6 +++---\n packfile.c     |  3 +--\n 5 files changed, 56 insertions(+), 29 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 0e25b14076..9929c1ed87 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -721,11 +721,15 @@ static struct commit_graph *load_commit_graph_chain(struct odb_source *source)\n \n struct commit_graph *read_commit_graph_one(struct odb_source *source)\n {\n-\tstruct commit_graph *g = load_commit_graph_v1(source);\n+\tstruct commit_graph *g;\n+\n+\tif (source->commit_graph_attempted)\n+\t\treturn NULL;\n+\tsource->commit_graph_attempted = true;\n \n+\tg = load_commit_graph_v1(source);\n \tif (!g)\n \t\tg = load_commit_graph_chain(source);\n-\n \treturn g;\n }\n \n@@ -737,6 +741,7 @@ struct commit_graph *read_commit_graph_one(struct odb_source *source)\n  */\n static struct commit_graph *prepare_commit_graph(struct repository *r)\n {\n+\tbool all_attempted = true;\n \tstruct odb_source *source;\n \n \t/*\n@@ -749,9 +754,19 @@ static struct commit_graph *prepare_commit_graph(struct repository *r)\n \tif (!r->gitdir || r->commit_graph_disabled)\n \t\treturn NULL;\n \n-\tif (r->objects->commit_graph_attempted)\n-\t\treturn r->objects->commit_graph;\n-\tr->objects->commit_graph_attempted = 1;\n+\todb_prepare_alternates(r->objects);\n+\tfor (source = r->objects->sources; source; source = source->next) {\n+\t\tall_attempted &= source->commit_graph_attempted;\n+\t\tif (source->commit_graph)\n+\t\t\treturn source->commit_graph;\n+\t}\n+\n+\t/*\n+\t * There is no point in re-trying to load commit graphs if we already\n+\t * tried loading all of them beforehand.\n+\t */\n+\tif (all_attempted)\n+\t\treturn NULL;\n \n \tprepare_repo_settings(r);\n \n@@ -768,14 +783,16 @@ static struct commit_graph *prepare_commit_graph(struct repository *r)\n \tif (!commit_graph_compatible(r))\n \t\treturn NULL;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n-\t\tr->objects->commit_graph = read_commit_graph_one(source);\n-\t\tif (r->objects->commit_graph)\n-\t\t\tbreak;\n+\t\tif (source->commit_graph_attempted)\n+\t\t\tcontinue;\n+\n+\t\tsource->commit_graph = read_commit_graph_one(source);\n+\t\tif (source->commit_graph)\n+\t\t\treturn source->commit_graph;\n \t}\n \n-\treturn r->objects->commit_graph;\n+\treturn NULL;\n }\n \n int generation_numbers_enabled(struct repository *r)\n@@ -806,7 +823,7 @@ int corrected_commit_dates_enabled(struct repository *r)\n \n struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)\n {\n-\tstruct commit_graph *g = r->objects->commit_graph;\n+\tstruct commit_graph *g = prepare_commit_graph(r);\n \twhile (g) {\n \t\tif (g->bloom_filter_settings)\n \t\t\treturn g->bloom_filter_settings;\n@@ -815,15 +832,16 @@ struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)\n \treturn NULL;\n }\n \n-void close_commit_graph(struct object_database *o)\n+void close_commit_graph(struct odb_source *source)\n {\n-\tif (!o->commit_graph)\n+\tif (!source->commit_graph)\n \t\treturn;\n \n \tclear_commit_graph_data_slab(&commit_graph_data_slab);\n \tdeinit_bloom_filters();\n-\tfree_commit_graph(o->commit_graph);\n-\to->commit_graph = NULL;\n+\tfree_commit_graph(source->commit_graph);\n+\tsource->commit_graph = NULL;\n+\tsource->commit_graph_attempted = 0;\n }\n \n static int bsearch_graph(struct commit_graph *g, const struct object_id *oid, uint32_t *pos)\n@@ -1119,7 +1137,15 @@ static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g,\n \n struct tree *get_commit_tree_in_graph(struct repository *r, const struct commit *c)\n {\n-\treturn get_commit_tree_in_graph_one(r->objects->commit_graph, c);\n+\tstruct odb_source *source;\n+\n+\tfor (source = r->objects->sources; source; source = source->next) {\n+\t\tif (!source->commit_graph)\n+\t\t\tcontinue;\n+\t\treturn get_commit_tree_in_graph_one(source->commit_graph, c);\n+\t}\n+\n+\treturn NULL;\n }\n \n struct packed_commit_list {\n@@ -2165,7 +2191,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)\n \t\tctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 2] = new_base_hash;\n \t}\n \n-\tclose_commit_graph(ctx->r->objects);\n+\tfor (struct odb_source *s = ctx->r->objects->sources; s; s = s->next)\n+\t\tclose_commit_graph(s);\n \tfinalize_hashfile(f, file_hash, FSYNC_COMPONENT_COMMIT_GRAPH,\n \t\t\t  CSUM_HASH_IN_STREAM | CSUM_FSYNC);\n \tfree_chunkfile(cf);\n@@ -2667,8 +2694,8 @@ int write_commit_graph(struct odb_source *source,\n \toid_array_clear(&ctx.oids);\n \tclear_topo_level_slab(&topo_levels);\n \n-\tif (ctx.r->objects->commit_graph) {\n-\t\tstruct commit_graph *g = ctx.r->objects->commit_graph;\n+\tif (source->commit_graph) {\n+\t\tstruct commit_graph *g = source->commit_graph;\n \n \t\twhile (g) {\n \t\t\tg->topo_levels = NULL;\ndiff --git a/commit-graph.h b/commit-graph.h\nindex f6a5433641..33cb6a7577 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -185,7 +185,7 @@ int write_commit_graph(struct odb_source *source,\n \n int verify_commit_graph(struct commit_graph *g, int flags);\n \n-void close_commit_graph(struct object_database *);\n+void close_commit_graph(struct odb_source *);\n void free_commit_graph(struct commit_graph *);\n \n /*\ndiff --git a/odb.c b/odb.c\nindex 2a92a018c4..fdcc6849a8 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -363,6 +363,11 @@ static void free_object_directory(struct odb_source *source)\n \tfree(source->path);\n \todb_clear_loose_cache(source);\n \tloose_object_map_clear(&source->loose_map);\n+\n+\tfree_commit_graph(source->commit_graph);\n+\tsource->commit_graph = NULL;\n+\tsource->commit_graph_attempted = 0;\n+\n \tfree(source);\n }\n \n@@ -1023,10 +1028,6 @@ void odb_clear(struct object_database *o)\n \toidmap_clear(&o->replace_map, 1);\n \tpthread_mutex_destroy(&o->replace_mutex);\n \n-\tfree_commit_graph(o->commit_graph);\n-\to->commit_graph = NULL;\n-\to->commit_graph_attempted = 0;\n-\n \tfree_object_directories(o);\n \to->sources_tail = NULL;\n \to->loaded_alternates = 0;\ndiff --git a/odb.h b/odb.h\nindex 3dfc66d75a..a4835db685 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -63,6 +63,9 @@ struct odb_source {\n \t */\n \tstruct multi_pack_index *midx;\n \n+\tstruct commit_graph *commit_graph;\n+\tbool commit_graph_attempted; /* if loading has been attempted */\n+\n \t/*\n \t * This is a temporary object store created by the tmp_objdir\n \t * facility. Disable ref updates since the objects in the store\n@@ -120,9 +123,6 @@ struct object_database {\n \tunsigned replace_map_initialized : 1;\n \tpthread_mutex_t replace_mutex; /* protect object replace functions */\n \n-\tstruct commit_graph *commit_graph;\n-\tunsigned commit_graph_attempted : 1; /* if loading has been attempted */\n-\n \t/*\n \t * private data\n \t *\ndiff --git a/packfile.c b/packfile.c\nindex 5d73932f50..5d3a25816f 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -374,9 +374,8 @@ void close_object_store(struct object_database *o)\n \t\tif (source->midx)\n \t\t\tclose_midx(source->midx);\n \t\tsource->midx = NULL;\n+\t\tclose_commit_graph(source);\n \t}\n-\n-\tclose_commit_graph(o);\n }\n \n void unlink_pack_path(const char *pack_name, int force_delete)\n\n-- \n2.51.0.417.g1ba7204a04.dirty\n\n"},{"id":"525572","messageId":"xmqq5xdx7qx4.fsf@gitster.g","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-04T23:12:55Z","receivedAt":"2025-09-04T23:12:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> commit graphs are currently stored on the object database level. This\n> doesn't really make much sense conceptually, given that commit graphs\n> are specific to one object source. Furthermore, with the upcoming\n> pluggable object database effort, an object source's backend may not\n> evene have a commit graph in the first place but store that information\n> in a different format altogether.\n>\n> This patch series prepares for that by moving the commit graph from\n> `struct object_database` into `struct odb_source`.\n\nHmph, I am finding the above hard to agree with at the conceptual\nlevel.  In some future, we may use multiple object stores in a\nsingle repository.  Perhaps we would be storing older parts of\nhistory in semi-online storage while newer parts are stored in\nreadily available storage.  But the side data structure that allows\nus to quickly learn who are parents of one commit is without having\nto go to the object store in order to parse the actualy commit\nobject can be stored for the entire history if we wanted to, or more\nrecent part of the history but not limited to the \"readily available\nstorage\" part.  IOW, where the boundary between the older and the\nnewer parts of the history lies and which commits the commit graph\ncovers should be pretty much independent.\n\nSo moving from object_database (i.e. the whole world) to individual\nodb_source (i.e. where one particular subset of the history is\nstored) feels like totally backwards to me.  Surely, a commit graph\nfile may be defined over a set of packfiles and remaining loose\nobject files, but it is not like an instance of the commit-graph\nfile is tied to packfiles in the sense that it uses the index into\nsome packfile instead of the actual object names to refer to\ncommits, or anything like that (this is quite different from other\nfiles that are very specific to a single object store, like midx\nthat is tied to the packfiles it describes).\n\n\n\n\n"},{"id":"525574","messageId":"xmqq1pol7q8i.fsf@gitster.g","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-0-d932c2481e1a@pks.im","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-04T23:27:41Z","receivedAt":"2025-09-04T23:27:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> There's a trivial conflict with tc/last-modified that can be solved like\n> this:\n>\n> diff --cc commit-graph.c\n> index 9929c1ed87,2f20f66cfd..0000000000\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@@ -823,7 -812,12 +823,11 @@@ int corrected_commit_dates_enabled(stru\n>   \n>   struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)\n>   {\n>  -\tstruct commit_graph *g;\n>  +\tstruct commit_graph *g = prepare_commit_graph(r);\n> + \n>  -\tif (!prepare_commit_graph(r))\n> ++\tif (!g)\n> + \t       return NULL;\n> + \n\nThe while (g) loop will be entirely skipped when g==NULL, and then\nthe function returns NULL after iterating the loop, so there is not\nmuch reason to have these three lines for early-return, no?\n\n>  -\tg = r->objects->commit_graph;\n>   \twhile (g) {\n>   \t\tif (g->bloom_filter_settings)\n>   \t\t\treturn g->bloom_filter_settings;\n\nThanks.\n"},{"id":"525592","messageId":"aLqAtVW667nZI5JG@pks.im","threadId":"64090","inReplyTo":"xmqq1pol7q8i.fsf@gitster.g","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-05T06:18:29Z","receivedAt":"2025-09-05T06:18:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Sep 04, 2025 at 04:27:41PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --cc commit-graph.c\n> > index 9929c1ed87,2f20f66cfd..0000000000\n> > --- a/commit-graph.c\n> > +++ b/commit-graph.c\n> > @@@ -823,7 -812,12 +823,11 @@@ int corrected_commit_dates_enabled(stru\n> >   \n> >   struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)\n> >   {\n> >  -\tstruct commit_graph *g;\n> >  +\tstruct commit_graph *g = prepare_commit_graph(r);\n> > + \n> >  -\tif (!prepare_commit_graph(r))\n> > ++\tif (!g)\n> > + \t       return NULL;\n> > + \n> \n> The while (g) loop will be entirely skipped when g==NULL, and then\n> the function returns NULL after iterating the loop, so there is not\n> much reason to have these three lines for early-return, no?\n\nAh, true. That's even better.\n\nPatrick\n"},{"id":"525656","messageId":"cf7aeda1-297a-4805-b0ae-e379ce11bbcf@gmail.com","threadId":"64090","inReplyTo":"xmqq5xdx7qx4.fsf@gitster.g","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-09-05T18:29:50Z","receivedAt":"2025-09-05T18:30:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/4/2025 7:12 PM, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n>> commit graphs are currently stored on the object database level. This\n>> doesn't really make much sense conceptually, given that commit graphs\n>> are specific to one object source. Furthermore, with the upcoming\n>> pluggable object database effort, an object source's backend may not\n>> evene have a commit graph in the first place but store that information\n>> in a different format altogether.\n>>\n>> This patch series prepares for that by moving the commit graph from\n>> `struct object_database` into `struct odb_source`.\n> \n> Hmph, I am finding the above hard to agree with at the conceptual\n> level.  In some future, we may use multiple object stores in a\n> single repository.  Perhaps we would be storing older parts of\n> history in semi-online storage while newer parts are stored in\n> readily available storage.  But the side data structure that allows\n> us to quickly learn who are parents of one commit is without having\n> to go to the object store in order to parse the actualy commit\n> object can be stored for the entire history if we wanted to, or more\n> recent part of the history but not limited to the \"readily available\n> storage\" part.  IOW, where the boundary between the older and the\n> newer parts of the history lies and which commits the commit graph\n> covers should be pretty much independent.\n> \n> So moving from object_database (i.e. the whole world) to individual\n> odb_source (i.e. where one particular subset of the history is\n> stored) feels like totally backwards to me.  Surely, a commit graph\n> file may be defined over a set of packfiles and remaining loose\n> object files, but it is not like an instance of the commit-graph\n> file is tied to packfiles in the sense that it uses the index into\n> some packfile instead of the actual object names to refer to\n> commits, or anything like that (this is quite different from other\n> files that are very specific to a single object store, like midx\n> that is tied to the packfiles it describes).\n\nThis is an interesting aspect to things, where the commit-graph file\nis a \"structured cache\" of certain commit information. It happens to\nbe located within the object stores (either local or in an alternate)\nbut is conceptually different in a few ways.\n\nThe biggest difference is that you can only open one commit-graph\n(or chain of commit-graphs). Having multiple files across different\nobject stores will not accumulate additional context. Instead, we\nhave a \"first one wins\" approach.\n\nThis does seem to be something that you are attempting to change\nby including the ability to load a commit-graph for each odb (and\nclosing them in sequence as we close a repo).\n\nSo in this sense, the commit-graph lives at the repository level,\nnot an object store level. When doing I/O to write or read a graph,\nwe use a specific object store at a time.\n\nThe other direction to consider is what context we have when we\ninteract with a commit-graph. We generally are parsing commits from\na repository or loading Bloom filter data during file history walks.\nEach of these do not have a predictable nature of which object store\nwill \"own\" the commit we are inspecting, so it wouldn't make sense\nto restrict things like odb_parse_commit() over repo_parse_commit().\n\nWith these thoughts in mind, I have these big-picture thoughts:\n\n1. Patches 1-5 are great. Nice cleanups.\n\n2. Some of Patch 6 is great, including having the I/O methods use\n   an odb_source to help focus the specific location of the files\n   being read or written. However, the movement of the struct into\n   the odb_source makes less sense and should still exist at the\n   object_database level.\n\nThanks,\n-Stolee\n\n"},{"id":"525790","messageId":"aL67U0-tw7O-y6_X@pks.im","threadId":"64090","inReplyTo":"cf7aeda1-297a-4805-b0ae-e379ce11bbcf@gmail.com","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-08T11:17:39Z","receivedAt":"2025-09-08T11:17:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Sep 05, 2025 at 02:29:50PM -0400, Derrick Stolee wrote:\n> On 9/4/2025 7:12 PM, Junio C Hamano wrote:\n> > Patrick Steinhardt <ps@pks.im> writes:\n> > \n> >> commit graphs are currently stored on the object database level. This\n> >> doesn't really make much sense conceptually, given that commit graphs\n> >> are specific to one object source. Furthermore, with the upcoming\n> >> pluggable object database effort, an object source's backend may not\n> >> evene have a commit graph in the first place but store that information\n> >> in a different format altogether.\n> >>\n> >> This patch series prepares for that by moving the commit graph from\n> >> `struct object_database` into `struct odb_source`.\n> > \n> > Hmph, I am finding the above hard to agree with at the conceptual\n> > level.  In some future, we may use multiple object stores in a\n> > single repository.  Perhaps we would be storing older parts of\n> > history in semi-online storage while newer parts are stored in\n> > readily available storage.  But the side data structure that allows\n> > us to quickly learn who are parents of one commit is without having\n> > to go to the object store in order to parse the actualy commit\n> > object can be stored for the entire history if we wanted to, or more\n> > recent part of the history but not limited to the \"readily available\n> > storage\" part.  IOW, where the boundary between the older and the\n> > newer parts of the history lies and which commits the commit graph\n> > covers should be pretty much independent.\n> > \n> > So moving from object_database (i.e. the whole world) to individual\n> > odb_source (i.e. where one particular subset of the history is\n> > stored) feels like totally backwards to me.  Surely, a commit graph\n> > file may be defined over a set of packfiles and remaining loose\n> > object files, but it is not like an instance of the commit-graph\n> > file is tied to packfiles in the sense that it uses the index into\n> > some packfile instead of the actual object names to refer to\n> > commits, or anything like that (this is quite different from other\n> > files that are very specific to a single object store, like midx\n> > that is tied to the packfiles it describes).\n> \n> This is an interesting aspect to things, where the commit-graph file\n> is a \"structured cache\" of certain commit information. It happens to\n> be located within the object stores (either local or in an alternate)\n> but is conceptually different in a few ways.\n> \n> The biggest difference is that you can only open one commit-graph\n> (or chain of commit-graphs). Having multiple files across different\n> object stores will not accumulate additional context. Instead, we\n> have a \"first one wins\" approach.\n> \n> This does seem to be something that you are attempting to change\n> by including the ability to load a commit-graph for each odb (and\n> closing them in sequence as we close a repo).\n> \n> So in this sense, the commit-graph lives at the repository level,\n> not an object store level. When doing I/O to write or read a graph,\n> we use a specific object store at a time.\n> \n> The other direction to consider is what context we have when we\n> interact with a commit-graph. We generally are parsing commits from\n> a repository or loading Bloom filter data during file history walks.\n> Each of these do not have a predictable nature of which object store\n> will \"own\" the commit we are inspecting, so it wouldn't make sense\n> to restrict things like odb_parse_commit() over repo_parse_commit().\n> \n> With these thoughts in mind, I have these big-picture thoughts:\n> \n> 1. Patches 1-5 are great. Nice cleanups.\n> \n> 2. Some of Patch 6 is great, including having the I/O methods use\n>    an odb_source to help focus the specific location of the files\n>    being read or written. However, the movement of the struct into\n>    the odb_source makes less sense and should still exist at the\n>    object_database level.\n\nI (probably unsurprisingly :)) don't quite agree with this.\n\nLet's take a step back: why does the commit-graph exist in the first\nplace? It basically provides a caching mechanism to efficiently return\ninformation that is otherwise more expensive to obtain:\n\n  - It contains a cached representation of the graph so that we don't\n    have to parse each commit from the object database.\n\n  - It encodes generation numbers.\n\n  - It contains bloom filters.\n\nAll of which makes sense with the current design of our object storage\nformat, because obtaining this information can be quite expensive. But\nlet's consider a different world where we for example store objects in a\nproper database:\n\n  - This database may have an efficient way to compute generation\n    numbers on the fly, either when reading an object or when writing it\n    to disk. We cannot currently store that information in the packfile\n    right now, so it needs to be stored out-of-band. But with a database\n    there is no reason why we couldn't immediately compute and store the\n    generation number on each insert.\n\n  - This database may have an efficient way to store bloom filters next\n    to a specific commit directly, without requiring a separate file.\n\n  - This database may be distributed. So why should every client now\n    have to recompute a commit graph if we can instead store the data in\n    the database and thus have it accessible to all clients thereof?\n\n  - It may be _less_ efficient to use the commit graph to access data\n    compared to what that database can provide.\n\nSo I would claim that the commit graph is specifically tied to the\nactual storage format of objects, and it's not at all obvious that it\nwould need to exist if we had a different storage format.\n\nThe goal of this patch series is thus explicitly _not_ to allow loading\none commit graph per object source. In fact, the refactorings I did\nensure that we still only ever load a single commit graph.\n\nInstead, the goal is to allow each object source to decide for itself\nhow this additional information is to be stored and retrieved. This\n_may_ be a commit graph if that makes sense for a particular storage\nformat. But it may just as well _not_ be a commit graph, as other\nstorage formats may have way better solutions for making the commit\ngraph information accessible.\n\nPatrick\n"},{"id":"525829","messageId":"4e67fb02-bbbb-4cd8-9892-6f65b4f82b26@gmail.com","threadId":"64090","inReplyTo":"aL67U0-tw7O-y6_X@pks.im","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-09-08T14:46:40Z","receivedAt":"2025-09-08T14:47:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/8/2025 7:17 AM, Patrick Steinhardt wrote:\n> I (probably unsurprisingly :)) don't quite agree with this.\n\nI think I can summarize the main point you seem to be making with\nthis quote:\n\n> So I would claim that the commit graph is specifically tied to the\n> actual storage format of objects, and it's not at all obvious that it\n> would need to exist if we had a different storage format.\n\nI think I agree in principle, if you are saying \"different storage\nformat\" means \"different commit object information\" which then means\nwe are talking about a completely different object type that is not\nat all compatible with current Git.\n\nYou could store commit and object data in SQLite, in the cloud, or\nvia plaintext files on disk. As long as the data is still representing\ncommit objects as we format them today, the commit-graph is still a\ncache that can be used as a faster way to fill 'struct commit' objects\nin memory without navigating to that object database.\n\nAnd you also mention that the commit-graph format itself could be\nmore efficient. You're right! I think the way we did it within Azure\nDevOps is more efficient, because most of the commit walking algorithms\nare built working directly on the integer labels within the in-memory\ndata structure instead of operating on commit structs. This allows for\nless overhead when loading the graph (it's already cached in memory)\nand when walking thousands of commits (we only translate to object IDs\nif they are important for the output). But this is all the more reason\nto keep the commit-graph structures outside of \"the object store\" since\na \"commit-graph database\" can be implemented without being tied to an\nobject store.\n\nIf you are saying \"but our existing commit-graph format puts it in\n'objects/info/commit-graph[s]'\" then yes the storage of a commit-graph\nis tied to our storage of objects. But the way we interact with it in\ncode is in some way a layer above that.\n\nThanks,\n-Stolee\n\n"},{"id":"526024","messageId":"aMFjGoPhGsRCTihO@pks.im","threadId":"64090","inReplyTo":"4e67fb02-bbbb-4cd8-9892-6f65b4f82b26@gmail.com","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-10T11:38:02Z","receivedAt":"2025-09-10T11:38:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 08, 2025 at 10:46:40AM -0400, Derrick Stolee wrote:\n> On 9/8/2025 7:17 AM, Patrick Steinhardt wrote:\n> > I (probably unsurprisingly :)) don't quite agree with this.\n> \n> I think I can summarize the main point you seem to be making with\n> this quote:\n> \n> > So I would claim that the commit graph is specifically tied to the\n> > actual storage format of objects, and it's not at all obvious that it\n> > would need to exist if we had a different storage format.\n> \n> I think I agree in principle, if you are saying \"different storage\n> format\" means \"different commit object information\" which then means\n> we are talking about a completely different object type that is not\n> at all compatible with current Git.\n\nI don't plan to introduce a new type as in commit, blob or tag, but\nreally only the possibility to have a different format for how exactly\nthe objects are stored. So it's not really \"different commit object\ninformation\" that I want to store. It's rather \"additional commit object\ninformation\" that we currently have to store out-of-band, which includes\nitems like generation numbers or bloom filters.\n\nAs an example, if the object format stores data in the cloud it would be\nsensible to also store generation numbers there, as it would ensure that\nwe don't have to recompute them on every client. In other words, the\ndata that is currently stored locally in the commit-graph also becomes\nfully distributed.\n\n> You could store commit and object data in SQLite, in the cloud, or\n> via plaintext files on disk. As long as the data is still representing\n> commit objects as we format them today, the commit-graph is still a\n> cache that can be used as a faster way to fill 'struct commit' objects\n> in memory without navigating to that object database.\n> \n> And you also mention that the commit-graph format itself could be\n> more efficient. You're right! I think the way we did it within Azure\n> DevOps is more efficient, because most of the commit walking algorithms\n> are built working directly on the integer labels within the in-memory\n> data structure instead of operating on commit structs. This allows for\n> less overhead when loading the graph (it's already cached in memory)\n> and when walking thousands of commits (we only translate to object IDs\n> if they are important for the output). But this is all the more reason\n> to keep the commit-graph structures outside of \"the object store\" since\n> a \"commit-graph database\" can be implemented without being tied to an\n> object store.\n> \n> If you are saying \"but our existing commit-graph format puts it in\n> 'objects/info/commit-graph[s]'\" then yes the storage of a commit-graph\n> is tied to our storage of objects. But the way we interact with it in\n> code is in some way a layer above that.\n\nThere is no inherent reason why a new backend would not be able to use\nthe existing commit-graph infrastructure indeed. But there are reasons\nthat specific backends may not want to do so. If objects are already\nstored in a database table, then it may make way more sense to store\nadditional metadata that is currently stored in the commit-graph in a\nsecondary database table instead of in the commit graph.\n\nSo that raises the question who is going to decide what caching format\nto use. That is, given a repository, do we want to use a commit-graph or\ndo we rather want to store that data in the database?\n\nI think the most sensible way to decide this is by going via the backend\nof a specific object source. The backend knows how objects are stored,\nso it'll also know whether there is a better way to store metadata than\nvia commit graphs. If it is the \"files\" backend it will decide to\nconsult the commit-graph. If it is a SQlite backend it _may_ make sense\nto use a commit-graph, as well. But if it's a remote database for\nexample it may make more sense to store the information in that database\ndirectly.\n\nBy moving this logic into the object source we can tie this decision to\nthe object source and also abstract it away. In the pluggable object\ndatabase world we can make this data available via a couple of function\npointers that are per object source:\n\n  - A function to check whether a cached representation of the graph\n    exists in a specific source.\n\n  - A function to load bloom filters and generation numbers,\n    respectively via that source.\n\n  - A function to load a commit via the cached representation of its\n    source.\n\nThus, the details around the actual data format will be hidden away and\nmost of the code never has to care whether it's a commit graph that\ncontains the data or whether it's stored in a distributed database.\n\nNow it's not impossible to make this work in your proposed world where\nthe commit graph continues to sit at the database level. But we'd still\nhave to abstract away logic so that we can have different ways to store\ncached data. We could for example implement logic to ask every backend\nwhether it has a cached representation of the commit graph, and if so,\nto provide an opaque data structure that contains the above function\npointers. We could then continue to store that structure on the database\nlevel and use it whenever available. But I'm not sure whether that\ndesign is in any way more obvious -- quite on the contrary, I expect\nthat it may be more complex.\n\nThis is roughly what I have in my head right now. And I realize that\nthis information really should be sitting in a design document. I'm\nworking on that, but still need to land two more patch series before I\nwant to send such a patch series to the list.\n\nThanks for the discussion by the way, really appreciate it!\n\nPatrick\n"},{"id":"526152","messageId":"aMNIhquknyCSquvf@nand.local","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-1-d932c2481e1a@pks.im","subject":"Re: [PATCH 1/6] blame: drop explicit check for commit graph","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-09-11T22:09:10Z","receivedAt":"2025-09-11T22:09:13Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Sep 04, 2025 at 02:49:55PM +0200, Patrick Steinhardt wrote:\n> diff --git a/blame.c b/blame.c\n> index f1c0670144..cb0b083423 100644\n> --- a/blame.c\n> +++ b/blame.c\n> @@ -2909,9 +2909,6 @@ void setup_blame_bloom_data(struct blame_scoreboard *sb)\n>  \tstruct blame_bloom_data *bd;\n>  \tstruct bloom_filter_settings *bs;\n>\n> -\tif (!sb->repo->objects->commit_graph)\n> -\t\treturn;\n> -\n>  \tbs = get_bloom_filter_settings(sb->repo);\n>  \tif (!bs)\n>  \t\treturn;\n\nMakes sense; get_bloom_filter_settings() (as you note) will return NULL\nif r->objects->commit_graph is NULL, which will cause this function to\nterminate early anyway.\n\nThanks,\nTaylor\n"},{"id":"526153","messageId":"aMNKKOo+BsNxGKgh@nand.local","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-2-d932c2481e1a@pks.im","subject":"Re: [PATCH 2/6] revision: drop explicit check for commit graph","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-09-11T22:16:08Z","receivedAt":"2025-09-11T22:16:11Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Sep 04, 2025 at 02:49:56PM +0200, Patrick Steinhardt wrote:\n> When filtering down revisions by paths we know to use bloom filters from\n> the commit graph, if we have any. The entry point for this is in\n> `check_maybe_different_in_bloom_filter()`, where we first verify that:\n>\n>   - We do have a commit graph.\n>\n>   - That the commit is contained therein by checking that we have a\n>     proper generation number.\n>\n>   - And that the graph contains a bloom filter.\n>\n> The first check is somewhat redundant though: if we don't have a commit\n> graph, then the second check would already tell us that we don't have a\n> generation number for the specific commit.\n\nMakes sense; and we already inspect the commit_graph_data_slab\nregardless of whether or not we know we have a commit-graph, so this\nclean-up makes sense to me, too.\n\n> In theory this could be seen as a performance optimization to\n> short-circuit for scenarios where there is no commit graph. But in\n> practice this shouldn't matter: if there is no commit graph, then the\n> commit graph data slab would also be unpopulated and thus a lookup of\n> the commit should happen in constant time.\n\nYeah, this should be a few extra cycles at most. I think it should be\nfine.\n\nTaylor\n"},{"id":"526154","messageId":"aMNMQdMk2S4MTARG@nand.local","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-3-d932c2481e1a@pks.im","subject":"Re: [PATCH 3/6] commit-graph: return the prepared commit graph from `prepare_commit_graph()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-09-11T22:25:05Z","receivedAt":"2025-09-11T22:25:08Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Sep 04, 2025 at 02:49:57PM +0200, Patrick Steinhardt wrote:\n> When making use of commit graphs, one needs to first prepare them by\n> calling `prepare_commit_graph()`. Once that function was called and the\n> commit graph was prepared successfully, the caller is now expected to\n> access the graph directly via `struct object_database::commit_graph`.\n>\n> In a subsequent change, we're going to move the commit graph pointer\n> from `struct object_database` into `struct odb_source`. With this\n> change, semantics will change so that we use the commit graph of the\n> first source that has one. Consequently, all callers that currently\n> deference the `commit_graph` pointer would now have to loop around the\n> list of sources to find the commit graph.\n\n> This would become quite unwieldy. So instead of shifting the burden onto\n> such callers, adapt `prepare_commit_graph()` to return the prepared\n> commit graph, if any. Like this, callers are expected to call that\n> function and then use the returned commit graph.\n\nHmmph. I see what you're saying, though I'm not sure I agree with the\nimplication here. Presumably \"r->objects->commit_graph\" could be\nrewritten as \"repo_commit_graph(r->objects->sources)\" or similar. But\nI'm OK with this approach, too.\n\n>  int generation_numbers_enabled(struct repository *r)\n>  {\n>  \tuint32_t first_generation;\n>  \tstruct commit_graph *g;\n> -\tif (!prepare_commit_graph(r))\n> -\t       return 0;\n>\n> -\tg = r->objects->commit_graph;\n> -\n> -\tif (!g->num_commits)\n> -\t\treturn 0;\n> +\tg = prepare_commit_graph(r);\n> +\tif (!g || !g->num_commits)\n\nMakes sense; this isn't an exact translation, since the conditional now\nalso checks for the NULL-ness of \"g\" first. But that's necessary, since\nif the (now-removed) earlier call to prepare_commit_graph() succeeded,\nwe know that \"g\" is non-NULL here.\n\nSince that function is now responsible for handing us the commit_graph\nitself, checking for success means that we have to see if \"g\" is\nnon-NULL first before doing something with it.\n\n> @@ -799,12 +796,9 @@ int generation_numbers_enabled(struct repository *r)\n>  int corrected_commit_dates_enabled(struct repository *r)\n>  {\n>  \tstruct commit_graph *g;\n> -\tif (!prepare_commit_graph(r))\n> -\t\treturn 0;\n>\n> -\tg = r->objects->commit_graph;\n> -\n> -\tif (!g->num_commits)\n> +\tg = prepare_commit_graph(r);\n> +\tif (!g || !g->num_commits)\n\nSame here.\n\n> @@ -1012,23 +1006,26 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g,\n>  int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,\n>  \t\t\t\t  uint32_t *pos)\n>  {\n> -\tif (!prepare_commit_graph(r))\n> +\tstruct commit_graph *g = prepare_commit_graph(r);\n> +\tif (!g)\n>  \t\treturn 0;\n> -\treturn find_commit_pos_in_graph(c, r->objects->commit_graph, pos);\n> +\treturn find_commit_pos_in_graph(c, g, pos);\n\nThese and other changes may have read a little bit cleaner if there were\na preparatory commit which introduced \"g\" as a variable on the stack,\nsince that would change:\n\n    struct commit_graph *g;\n\n    if (!prepare_commit_graph(r))\n        return 0;\n\n    g = the_repository->objects->commit_graph;\n\ninto:\n\n    struct commit_graph *g = prepare_commit_graph(r);\n    if (!g)\n        return 0;\n\n, without affecting the rest of the function, keeping the diff at least\neasier to read (or smaller) by eliminating the \"r->objects->commit_graph\"\nto \"g\" change.\n\nNot a big deal at all, just a thought that I had while reviewing.\n\n> @@ -2519,6 +2518,7 @@ int write_commit_graph(struct odb_source *source,\n>  \tint replace = 0;\n>  \tstruct bloom_filter_settings bloom_settings = DEFAULT_BLOOM_FILTER_SETTINGS;\n>  \tstruct topo_level_slab topo_levels;\n> +\tstruct commit_graph *g;\n>\n>  \tprepare_repo_settings(r);\n>  \tif (!r->settings.core_commit_graph) {\n> @@ -2547,23 +2547,13 @@ int write_commit_graph(struct odb_source *source,\n>  \tinit_topo_level_slab(&topo_levels);\n>  \tctx.topo_levels = &topo_levels;\n>\n> -\tprepare_commit_graph(ctx.r);\n> -\tif (ctx.r->objects->commit_graph) {\n> -\t\tstruct commit_graph *g = ctx.r->objects->commit_graph;\n> -\n> -\t\twhile (g) {\n> -\t\t\tg->topo_levels = &topo_levels;\n> -\t\t\tg = g->base_graph;\n> -\t\t}\n> -\t}\n> +\tg = prepare_commit_graph(ctx.r);\n> +\tfor (struct commit_graph *chain = g; chain; chain = chain->base_graph)\n> +\t\tg->topo_levels = &topo_levels;\n\nMakes sense.\n\nThe rest looks all good.\n\nThanks,\nTaylor\n"},{"id":"526155","messageId":"aMNTELw0Wk8jWoPc@nand.local","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-4-d932c2481e1a@pks.im","subject":"Re: [PATCH 4/6] commit-graph: return commit graph from `repo_find_commit_pos_in_graph()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-09-11T22:54:08Z","receivedAt":"2025-09-11T22:54:10Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Sep 04, 2025 at 02:49:58PM +0200, Patrick Steinhardt wrote:\n> ---\n>  bloom.c        |  8 +++++---\n>  commit-graph.c | 18 ++++++++++++------\n>  commit-graph.h | 12 ++++++------\n>  3 files changed, 23 insertions(+), 15 deletions(-)\n\nAll makes sense and looks good to me.\n\nThanks,\nTaylor\n"},{"id":"526156","messageId":"aMNUi9+MZd0rtmOT@nand.local","threadId":"64090","inReplyTo":"20250904-b4-pks-commit-graph-via-source-v1-6-d932c2481e1a@pks.im","subject":"Re: [PATCH 6/6] odb: move commit-graph into the object sources","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-09-11T23:00:27Z","receivedAt":"2025-09-11T23:00:30Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Sep 04, 2025 at 02:50:00PM +0200, Patrick Steinhardt wrote:\n> Commit graphs are inherently tied to one specific object source.\n> Furthermore, with the upcoming pluggable object sources, it is not even\n> guaranteed that an object source may even have a commit graph as these\n> are specific to the actual on-disk data format.\n>\n> Prepare for this future by moving the commit-graph pointer from `struct\n> object_database` to `struct odb_source`. Eventually, this will allow us\n> to make commit graphs an implementation detail of an object source's\n> backend.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  commit-graph.c | 65 +++++++++++++++++++++++++++++++++++++++++-----------------\n>  commit-graph.h |  2 +-\n>  odb.c          |  9 ++++----\n>  odb.h          |  6 +++---\n>  packfile.c     |  3 +--\n>  5 files changed, 56 insertions(+), 29 deletions(-)\n>\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 0e25b14076..9929c1ed87 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -721,11 +721,15 @@ static struct commit_graph *load_commit_graph_chain(struct odb_source *source)\n>\n>  struct commit_graph *read_commit_graph_one(struct odb_source *source)\n>  {\n> -\tstruct commit_graph *g = load_commit_graph_v1(source);\n> +\tstruct commit_graph *g;\n> +\n> +\tif (source->commit_graph_attempted)\n> +\t\treturn NULL;\n\nI wonder if some callers will find this surprising. The current analog\nof this function is commit-graph.c::prepare_commit_graph(), which does:\n\n    if (r->objects->commit_graph_attempted)\n        return r->objects->commit_graph;\n\n, but this function returns NULL on the second (and subsequent) calls,\neven if the object source in question has a commit graph that was loaded\nin an earlier step.\n\n> +\tsource->commit_graph_attempted = true;\n>\n> +\tg = load_commit_graph_v1(source);\n>  \tif (!g)\n>  \t\tg = load_commit_graph_chain(source);\n> -\n>  \treturn g;\n>  }\n>\n> @@ -737,6 +741,7 @@ struct commit_graph *read_commit_graph_one(struct odb_source *source)\n>   */\n>  static struct commit_graph *prepare_commit_graph(struct repository *r)\n>  {\n> +\tbool all_attempted = true;\n>  \tstruct odb_source *source;\n>\n>  \t/*\n> @@ -749,9 +754,19 @@ static struct commit_graph *prepare_commit_graph(struct repository *r)\n>  \tif (!r->gitdir || r->commit_graph_disabled)\n>  \t\treturn NULL;\n>\n> -\tif (r->objects->commit_graph_attempted)\n> -\t\treturn r->objects->commit_graph;\n> -\tr->objects->commit_graph_attempted = 1;\n> +\todb_prepare_alternates(r->objects);\n> +\tfor (source = r->objects->sources; source; source = source->next) {\n> +\t\tall_attempted &= source->commit_graph_attempted;\n> +\t\tif (source->commit_graph)\n> +\t\t\treturn source->commit_graph;\n> +\t}\n> +\n> +\t/*\n> +\t * There is no point in re-trying to load commit graphs if we already\n> +\t * tried loading all of them beforehand.\n> +\t */\n> +\tif (all_attempted)\n> +\t\treturn NULL;\n>\n>  \tprepare_repo_settings(r);\n>\n> @@ -768,14 +783,16 @@ static struct commit_graph *prepare_commit_graph(struct repository *r)\n>  \tif (!commit_graph_compatible(r))\n>  \t\treturn NULL;\n>\n> -\todb_prepare_alternates(r->objects);\n>  \tfor (source = r->objects->sources; source; source = source->next) {\n> -\t\tr->objects->commit_graph = read_commit_graph_one(source);\n> -\t\tif (r->objects->commit_graph)\n> -\t\t\tbreak;\n> +\t\tif (source->commit_graph_attempted)\n> +\t\t\tcontinue;\n> +\n> +\t\tsource->commit_graph = read_commit_graph_one(source);\n> +\t\tif (source->commit_graph)\n> +\t\t\treturn source->commit_graph;\n\nIs this what callers expect? The pre-image here returned the\ncommit-graph belonging to the current repository's object database, not\nany of its alternate(s).\n\nI'm not opposed to the idea that perhaps loading commit-graphs from\nalternates would be useful, but I am uncomfortable making that change in\nthis series.\n\nThanks,\nTaylor\n"},{"id":"526157","messageId":"aMNWgD4wKa1a5R8g@nand.local","threadId":"64090","inReplyTo":"aL67U0-tw7O-y6_X@pks.im","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-09-11T23:08:48Z","receivedAt":"2025-09-11T23:08:51Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Sep 08, 2025 at 01:17:39PM +0200, Patrick Steinhardt wrote:\n> On Fri, Sep 05, 2025 at 02:29:50PM -0400, Derrick Stolee wrote:\n> > On 9/4/2025 7:12 PM, Junio C Hamano wrote:\n> > > Patrick Steinhardt <ps@pks.im> writes:\n> > >\n> > >> commit graphs are currently stored on the object database level. This\n> > >> doesn't really make much sense conceptually, given that commit graphs\n> > >> are specific to one object source. Furthermore, with the upcoming\n> > >> pluggable object database effort, an object source's backend may not\n> > >> evene have a commit graph in the first place but store that information\n> > >> in a different format altogether.\n> > >>\n> > >> This patch series prepares for that by moving the commit graph from\n> > >> `struct object_database` into `struct odb_source`.\n> > >\n> > > Hmph, I am finding the above hard to agree with at the conceptual\n> > > level.  In some future, we may use multiple object stores in a\n> > > single repository.  Perhaps we would be storing older parts of\n> > > history in semi-online storage while newer parts are stored in\n> > > readily available storage.  But the side data structure that allows\n> > > us to quickly learn who are parents of one commit is without having\n> > > to go to the object store in order to parse the actualy commit\n> > > object can be stored for the entire history if we wanted to, or more\n> > > recent part of the history but not limited to the \"readily available\n> > > storage\" part.  IOW, where the boundary between the older and the\n> > > newer parts of the history lies and which commits the commit graph\n> > > covers should be pretty much independent.\n> > >\n> > > So moving from object_database (i.e. the whole world) to individual\n> > > odb_source (i.e. where one particular subset of the history is\n> > > stored) feels like totally backwards to me.  Surely, a commit graph\n> > > file may be defined over a set of packfiles and remaining loose\n> > > object files, but it is not like an instance of the commit-graph\n> > > file is tied to packfiles in the sense that it uses the index into\n> > > some packfile instead of the actual object names to refer to\n> > > commits, or anything like that (this is quite different from other\n> > > files that are very specific to a single object store, like midx\n> > > that is tied to the packfiles it describes).\n> >\n> > This is an interesting aspect to things, where the commit-graph file\n> > is a \"structured cache\" of certain commit information. It happens to\n> > be located within the object stores (either local or in an alternate)\n> > but is conceptually different in a few ways.\n> >\n> > The biggest difference is that you can only open one commit-graph\n> > (or chain of commit-graphs). Having multiple files across different\n> > object stores will not accumulate additional context. Instead, we\n> > have a \"first one wins\" approach.\n> >\n> > This does seem to be something that you are attempting to change\n> > by including the ability to load a commit-graph for each odb (and\n> > closing them in sequence as we close a repo).\n> >\n> > So in this sense, the commit-graph lives at the repository level,\n> > not an object store level. When doing I/O to write or read a graph,\n> > we use a specific object store at a time.\n> >\n> > The other direction to consider is what context we have when we\n> > interact with a commit-graph. We generally are parsing commits from\n> > a repository or loading Bloom filter data during file history walks.\n> > Each of these do not have a predictable nature of which object store\n> > will \"own\" the commit we are inspecting, so it wouldn't make sense\n> > to restrict things like odb_parse_commit() over repo_parse_commit().\n> >\n> > With these thoughts in mind, I have these big-picture thoughts:\n> >\n> > 1. Patches 1-5 are great. Nice cleanups.\n> >\n> > 2. Some of Patch 6 is great, including having the I/O methods use\n> >    an odb_source to help focus the specific location of the files\n> >    being read or written. However, the movement of the struct into\n> >    the odb_source makes less sense and should still exist at the\n> >    object_database level.\n>\n> I (probably unsurprisingly :)) don't quite agree with this.\n>\n> Let's take a step back: why does the commit-graph exist in the first\n> place? It basically provides a caching mechanism to efficiently return\n> information that is otherwise more expensive to obtain:\n>\n>   - It contains a cached representation of the graph so that we don't\n>     have to parse each commit from the object database.\n>\n>   - It encodes generation numbers.\n>\n>   - It contains bloom filters.\n\nI don't think these three things are all on the same conceptual footing.\n\nIn the current object storage format, yes, part of the commit-graph's\nresponsibility is to provide fast access to the repository's commits and\ntheir ancestors. That is a pure optimization that is inherently tied to\nthe object storage format, which necessitates such an optimization for\ncertain operations to be efficient.\n\nGeneration numbers and changed-path Bloom filters are different. Yes,\nthey *can* be used to optimize certain operations, but they also provide\n*new* information that cannot be otherwise stored in the existing object\nformat.\n\nSuppose that we have some future object storage backend that stores\ncommit objects in such a way that querying them natively through\nwhatever mechanism that store provides is already as efficient as using\na commit-graph file on-disk today. That eliminates the \"pure\noptimization\" aspect of the commit-graph.\n\nBut that object store implementation will still want to implement\nadditional features like generation numbers and changed-path Bloom\nfilters, because those provide new information that is not purely about\noptimizing reads.\n\n> All of which makes sense with the current design of our object storage\n> format, because obtaining this information can be quite expensive. But\n> let's consider a different world where we for example store objects in a\n> proper database:\n>\n>   - This database may have an efficient way to compute generation\n>     numbers on the fly, either when reading an object or when writing it\n>     to disk. We cannot currently store that information in the packfile\n>     right now, so it needs to be stored out-of-band. But with a database\n>     there is no reason why we couldn't immediately compute and store the\n>     generation number on each insert.\n\nYes, but that does not mean that other parts of the code are not\nthemselves interested in what generation number a commit has for their\nown purposes.\n\n>   - This database may have an efficient way to store bloom filters next\n>     to a specific commit directly, without requiring a separate file.\n\nSame thought as above.\n\n> So I would claim that the commit graph is specifically tied to the\n> actual storage format of objects, and it's not at all obvious that it\n> would need to exist if we had a different storage format.\n\nI am not sure I agree here. What I'm trying to say is that though it's\neasy to imagine an object storage format that stores objects in such a\nway that the commit-graph file's representation of commits is\nunnecessary, there is additional information in what we currently call\nthe commit-graph that is useful regardless of how fast the actual object\nstore is.\n\n> The goal of this patch series is thus explicitly _not_ to allow loading\n> one commit graph per object source. In fact, the refactorings I did\n> ensure that we still only ever load a single commit graph.\n\nRight, though if I understand correctly I think there *is* a behavior\nchange here in that we will happily fall back to an alternate ODB's\ncommit-graph if the main object store does not have one. That is not how\nit works today.\n\nThanks,\nTaylor\n"},{"id":"527351","messageId":"xmqqo6qyfijl.fsf@gitster.g","threadId":"64090","inReplyTo":"aMFjGoPhGsRCTihO@pks.im","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-25T19:17:50Z","receivedAt":"2025-09-25T19:17:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> There is no inherent reason why a new backend would not be able to use\n> the existing commit-graph infrastructure indeed. But there are reasons\n> that specific backends may not want to do so. If objects are already\n> stored in a database table, then it may make way more sense to store\n> additional metadata that is currently stored in the commit-graph in a\n> secondary database table instead of in the commit graph.\n> ...\n> This is roughly what I have in my head right now. And I realize that\n> this information really should be sitting in a design document. I'm\n> working on that, but still need to land two more patch series before I\n> want to send such a patch series to the list.\n\nSo is everybody happy with this line of thought that makes it\nmandatory for each backend to decide and implement the commit-graph\nsupport if they want to?\n\nMy reading of the later part of Taylor's message[*] tells me that at\nleast Taylor does not agree with that position, and I am not sure\nabout this design choice, either.  Surely, each backend can have its\nown optimization, but looking at the way data from the commit-graph\nand other auxiliary data files are used to optimize real operations\n(like populating the essential fields of the commit object first\nfrom the graph, only to read other things lazily from the object\ndatabase, or switching to completely different traversal machinery\nwhen reachability bitmap is available), we cannot say that each\nbackend can store whatever side data they please and leave it at\nthat.  The code paths that are supposed to be generic need to be\naware of these side data used for optimization to some degree, so\nconceptually it is much cleaner (well, at least to my eyes, that is)\nto declare that the auxiliary data files like commit-graph and\nreachability bitmaps are defined on the objects in the repository,\nno matter what backend is used to store them.\n\n\n\n\n[Reference]\n\n * https://lore.kernel.org/git/aMNWgD4wKa1a5R8g@nand.local/ \n"},{"id":"527391","messageId":"aNYiIsUxR6yNQy_1@pks.im","threadId":"64090","inReplyTo":"xmqqo6qyfijl.fsf@gitster.g","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-26T05:18:26Z","receivedAt":"2025-09-26T05:18:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Sep 25, 2025 at 12:17:50PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > There is no inherent reason why a new backend would not be able to use\n> > the existing commit-graph infrastructure indeed. But there are reasons\n> > that specific backends may not want to do so. If objects are already\n> > stored in a database table, then it may make way more sense to store\n> > additional metadata that is currently stored in the commit-graph in a\n> > secondary database table instead of in the commit graph.\n> > ...\n> > This is roughly what I have in my head right now. And I realize that\n> > this information really should be sitting in a design document. I'm\n> > working on that, but still need to land two more patch series before I\n> > want to send such a patch series to the list.\n> \n> So is everybody happy with this line of thought that makes it\n> mandatory for each backend to decide and implement the commit-graph\n> support if they want to?\n> \n> My reading of the later part of Taylor's message[*] tells me that at\n> least Taylor does not agree with that position, and I am not sure\n> about this design choice, either.  Surely, each backend can have its\n> own optimization, but looking at the way data from the commit-graph\n> and other auxiliary data files are used to optimize real operations\n> (like populating the essential fields of the commit object first\n> from the graph, only to read other things lazily from the object\n> database, or switching to completely different traversal machinery\n> when reachability bitmap is available), we cannot say that each\n> backend can store whatever side data they please and leave it at\n> that.  The code paths that are supposed to be generic need to be\n> aware of these side data used for optimization to some degree, so\n> conceptually it is much cleaner (well, at least to my eyes, that is)\n> to declare that the auxiliary data files like commit-graph and\n> reachability bitmaps are defined on the objects in the repository,\n> no matter what backend is used to store them.\n\nQuick update: I haven't found the time to reply to review feedback yet.\nI'll pick this topic up again in one or two weeks.\n\nPatrick\n"},{"id":"527788","messageId":"aN5gPoPC4WEYw9CF@pks.im","threadId":"64090","inReplyTo":"xmqqo6qyfijl.fsf@gitster.g","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-02T11:21:34Z","receivedAt":"2025-10-02T11:21:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Sep 25, 2025 at 12:17:50PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > There is no inherent reason why a new backend would not be able to use\n> > the existing commit-graph infrastructure indeed. But there are reasons\n> > that specific backends may not want to do so. If objects are already\n> > stored in a database table, then it may make way more sense to store\n> > additional metadata that is currently stored in the commit-graph in a\n> > secondary database table instead of in the commit graph.\n> > ...\n> > This is roughly what I have in my head right now. And I realize that\n> > this information really should be sitting in a design document. I'm\n> > working on that, but still need to land two more patch series before I\n> > want to send such a patch series to the list.\n> \n> So is everybody happy with this line of thought that makes it\n> mandatory for each backend to decide and implement the commit-graph\n> support if they want to?\n> \n> My reading of the later part of Taylor's message[*] tells me that at\n> least Taylor does not agree with that position, and I am not sure\n> about this design choice, either.  Surely, each backend can have its\n> own optimization, but looking at the way data from the commit-graph\n> and other auxiliary data files are used to optimize real operations\n> (like populating the essential fields of the commit object first\n> from the graph, only to read other things lazily from the object\n> database, or switching to completely different traversal machinery\n> when reachability bitmap is available), we cannot say that each\n> backend can store whatever side data they please and leave it at\n> that.  The code paths that are supposed to be generic need to be\n> aware of these side data used for optimization to some degree, so\n> conceptually it is much cleaner (well, at least to my eyes, that is)\n> to declare that the auxiliary data files like commit-graph and\n> reachability bitmaps are defined on the objects in the repository,\n> no matter what backend is used to store them.\n\nMy intent here is mostly to allow us to swap out how exactly the data is\nbeing cached. During the Git Merge I heard from some JJ developer (I\nthink) that they also have a pluggable cache, but they approach the\nissue differently: instead of making the cache a property of the object\nbackend, they instead make the cache itself pluggable.\n\nI think that's a worthwhile angle to explore. The cache would still sit\non the repository level, and it wouldn't have to care at all whether we\nuse loose objects/packfiles or any other backend. But in theory, we can\nstill swap it out for a different representation as desired.\n\nWhich overall means that we can defer this to a later point in time, as\nwe can make it pluggable independent from making the object database\nitself pluggable.\n\nSo I'd propose to merge the first six patches, as everyone seemed to\nagree that they improve the status quo, but drop the last patch that\nmoves the commit-graph into the ODB sources.\n\nDoes that seem reasonable to everyone? If so, I don't really see a\nreason to reroll at this point. But please let me know in case I miss\nanything that needs addressing.\n\nThanks!\n\nPatrick\n"},{"id":"527790","messageId":"aN5jm9zXMD8E7Slq@pks.im","threadId":"64090","inReplyTo":"aN5gPoPC4WEYw9CF@pks.im","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-02T11:35:55Z","receivedAt":"2025-10-02T11:36:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Oct 02, 2025 at 01:21:34PM +0200, Patrick Steinhardt wrote:\n> On Thu, Sep 25, 2025 at 12:17:50PM -0700, Junio C Hamano wrote:\n> > Patrick Steinhardt <ps@pks.im> writes:\n> > \n> > > There is no inherent reason why a new backend would not be able to use\n> > > the existing commit-graph infrastructure indeed. But there are reasons\n> > > that specific backends may not want to do so. If objects are already\n> > > stored in a database table, then it may make way more sense to store\n> > > additional metadata that is currently stored in the commit-graph in a\n> > > secondary database table instead of in the commit graph.\n> > > ...\n> > > This is roughly what I have in my head right now. And I realize that\n> > > this information really should be sitting in a design document. I'm\n> > > working on that, but still need to land two more patch series before I\n> > > want to send such a patch series to the list.\n> > \n> > So is everybody happy with this line of thought that makes it\n> > mandatory for each backend to decide and implement the commit-graph\n> > support if they want to?\n> > \n> > My reading of the later part of Taylor's message[*] tells me that at\n> > least Taylor does not agree with that position, and I am not sure\n> > about this design choice, either.  Surely, each backend can have its\n> > own optimization, but looking at the way data from the commit-graph\n> > and other auxiliary data files are used to optimize real operations\n> > (like populating the essential fields of the commit object first\n> > from the graph, only to read other things lazily from the object\n> > database, or switching to completely different traversal machinery\n> > when reachability bitmap is available), we cannot say that each\n> > backend can store whatever side data they please and leave it at\n> > that.  The code paths that are supposed to be generic need to be\n> > aware of these side data used for optimization to some degree, so\n> > conceptually it is much cleaner (well, at least to my eyes, that is)\n> > to declare that the auxiliary data files like commit-graph and\n> > reachability bitmaps are defined on the objects in the repository,\n> > no matter what backend is used to store them.\n> \n> My intent here is mostly to allow us to swap out how exactly the data is\n> being cached. During the Git Merge I heard from some JJ developer (I\n> think) that they also have a pluggable cache, but they approach the\n> issue differently: instead of making the cache a property of the object\n> backend, they instead make the cache itself pluggable.\n> \n> I think that's a worthwhile angle to explore. The cache would still sit\n> on the repository level, and it wouldn't have to care at all whether we\n> use loose objects/packfiles or any other backend. But in theory, we can\n> still swap it out for a different representation as desired.\n> \n> Which overall means that we can defer this to a later point in time, as\n> we can make it pluggable independent from making the object database\n> itself pluggable.\n> \n> So I'd propose to merge the first six patches, as everyone seemed to\n\nCorrection: first five patches, of course :)\n\nPatrick\n\n> agree that they improve the status quo, but drop the last patch that\n> moves the commit-graph into the ODB sources.\n> \n> Does that seem reasonable to everyone? If so, I don't really see a\n> reason to reroll at this point. But please let me know in case I miss\n> anything that needs addressing.\n> \n> Thanks!\n> \n> Patrick\n> \n"},{"id":"527812","messageId":"xmqqy0ptw8ns.fsf@gitster.g","threadId":"64090","inReplyTo":"aN5gPoPC4WEYw9CF@pks.im","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-02T16:49:43Z","receivedAt":"2025-10-02T16:49:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> My intent here is mostly to allow us to swap out how exactly the data is\n> being cached. During the Git Merge I heard from some JJ developer (I\n> think) that they also have a pluggable cache, but they approach the\n> issue differently: instead of making the cache a property of the object\n> backend, they instead make the cache itself pluggable.\n>\n> I think that's a worthwhile angle to explore. The cache would still sit\n> on the repository level, and it wouldn't have to care at all whether we\n> use loose objects/packfiles or any other backend. But in theory, we can\n> still swap it out for a different representation as desired.\n\nThe idea to allow these \"caches\" being pluggable to the system\nindependently from object store or reference store backends does\nmake quite a lot of sense.  If there is only one that is plugged,\nthat degenerates into the \"side data like commit-graph and\nreachability bitmaps do not belong to a specific object store\nbackend, but are defined over the objects known to the repository\" I\nwas talking about in the message you are replying to, I think.\n\n> Which overall means that we can defer this to a later point in time, as\n> we can make it pluggable independent from making the object database\n> itself pluggable.\n>\n> So I'd propose to merge the first six patches, as everyone seemed to\n> agree that they improve the status quo, but drop the last patch that\n> moves the commit-graph into the ODB sources.\n>\n> Does that seem reasonable to everyone? If so, I don't really see a\n> reason to reroll at this point. But please let me know in case I miss\n> anything that needs addressing.\n\nI am fine with that, but let's hear from everyone ;-).\n\nThanks.\n"},{"id":"527889","messageId":"f9b2235f-5043-47af-907f-9c9ab3f22a51@gmail.com","threadId":"64090","inReplyTo":"xmqqy0ptw8ns.fsf@gitster.g","subject":"Re: [PATCH 0/6] odb: track commit graphs via object source","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-10-03T16:56:49Z","receivedAt":"2025-10-03T16:56:55Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/2/2025 12:49 PM, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n>> My intent here is mostly to allow us to swap out how exactly the data is\n>> being cached. During the Git Merge I heard from some JJ developer (I\n>> think) that they also have a pluggable cache, but they approach the\n>> issue differently: instead of making the cache a property of the object\n>> backend, they instead make the cache itself pluggable.\n>>\n>> I think that's a worthwhile angle to explore. The cache would still sit\n>> on the repository level, and it wouldn't have to care at all whether we\n>> use loose objects/packfiles or any other backend. But in theory, we can\n>> still swap it out for a different representation as desired.\n> \n> The idea to allow these \"caches\" being pluggable to the system\n> independently from object store or reference store backends does\n> make quite a lot of sense.  If there is only one that is plugged,\n> that degenerates into the \"side data like commit-graph and\n> reachability bitmaps do not belong to a specific object store\n> backend, but are defined over the objects known to the repository\" I\n> was talking about in the message you are replying to, I think.\n> \n>> Which overall means that we can defer this to a later point in time, as\n>> we can make it pluggable independent from making the object database\n>> itself pluggable.\n>>\n>> So I'd propose to merge the first six patches, as everyone seemed to\n>> agree that they improve the status quo, but drop the last patch that\n>> moves the commit-graph into the ODB sources.\n>>\n>> Does that seem reasonable to everyone? If so, I don't really see a\n>> reason to reroll at this point. But please let me know in case I miss\n>> anything that needs addressing.\n> \n> I am fine with that, but let's hear from everyone ;-).\n\nI am also fine with that approach. Thanks!\n-Stolee\n"}]}