[PATCH 00/10] commit-graph: several refactorings for writing commit graphs

[PATCH 00/10] commit-graph: several refactorings for writing commit graphs

[Patrick Steinhardt]

Hi,

this patch series includes several small cleanups for the interfaces for
writing commit graphs. Most importantly, the series refactors options so
that they all get passed via `struct commit_graph_write_options`. These
cleanups are split out of a bigger change to make reading and writing
commit-graphs pluggable via the object database bakcend.

The series is based on 750f5de7dfe (The 14th batch, 2026-11-03) with
ps/commit-graph-cleanups at cdc7d6b46c6 (builtin/commit-graph:
de-globalize options handling, 2026-10-28) merged into it.

Thanks!

Patrick

---
Patrick Steinhardt (10):
      commit-graph: rename `struct commit_graph_opts`
      commit-graph: pass writer flags via options
      commit-graph: drop `NULL` checks for write options
      commit-graph: drop redundant flags in write context
      commit-graph: merge the two commit graph writers
      commit-graph: move remaining write parameters into options
      commit-graph: fix mixup of write/verify flags
      builtin/commit-graph: set flags directly
      odb/source-files: verify commit-graphs as part of fsck
      commit-graph: modernize function names

 Documentation/gitformat-chunk.adoc |   2 +-
 builtin/commit-graph.c             |  82 ++++++-----------
 builtin/fetch.c                    |  23 ++---
 builtin/fsck.c                     |  20 ----
 builtin/gc.c                       |  15 ++-
 commit-graph.c                     | 183 +++++++++++++++++--------------------
 commit-graph.h                     |  40 ++++----
 odb.c                              |   2 +-
 odb/source-files.c                 |  25 +++++
 oss-fuzz/fuzz-commit-graph.c       |   4 +-
 t/helper/test-read-graph.c         |   2 +-
 11 files changed, 183 insertions(+), 215 deletions(-)


---
base-commit: d00aeacdaa9e157548863b0eecc019044126c621
change-id: 20261103-pks-commit-graph-write-refactorings-f68fb22efd9b

[PATCH 01/10] commit-graph: rename `struct commit_graph_opts`

[Patrick Steinhardt]

The `struct commit_graph_opts` is used by us to control how exactly a
commit graph gets written. Nothing really indicates that it's only
intended for use by `write_commit_graph()` though.

Rename it to `struct commit_graph_write_options` to fix that. Note that
in a subsequent commit we'll also rename `write_commit_graph()` to
`commit_graph_write()` to match our modern best practices for naming,
and that will align the naming for both of these.

While at it, also introduce an `INIT` macro.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/commit-graph.c |  7 ++-----
 commit-graph.c         |  6 +++---
 commit-graph.h         | 11 ++++++++---
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 4d28a9d51ec..0445302ba8b 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -141,7 +141,7 @@ static int cmd_commit_graph_verify(int argc,
 }
 
 struct commit_graph_write_config_data {
-	struct commit_graph_opts *write_opts;
+	struct commit_graph_write_options *write_opts;
 	struct opts_commit_graph *opts;
 };
 
@@ -229,10 +229,7 @@ static int cmd_commit_graph_write(int argc,
 				  const char *prefix,
 				  struct repository *repo)
 {
-	struct commit_graph_opts write_opts = {
-		.size_multiple = 2,
-		.max_new_filters = -1,
-	};
+	struct commit_graph_write_options write_opts = COMMIT_GRAPH_WRITE_OPTIONS_INIT;
 	struct opts_commit_graph opts = {
 		.progress = isatty(2),
 		.enable_changed_paths = -1,
diff --git a/commit-graph.c b/commit-graph.c
index 6c141a6555f..2f9bf7a44fd 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1166,7 +1166,7 @@ struct write_commit_graph_context {
 		 trust_generation_numbers:1;
 
 	struct topo_level_slab *topo_levels;
-	const struct commit_graph_opts *opts;
+	const struct commit_graph_write_options *opts;
 	size_t total_bloom_filter_data_size;
 	const struct bloom_filter_settings *bloom_settings;
 
@@ -1916,7 +1916,7 @@ static int add_ref_to_set(const struct reference *ref, void *cb_data)
 int write_commit_graph_reachable(struct repository *repo,
 				 const char *dir,
 				 enum commit_graph_write_flags flags,
-				 const struct commit_graph_opts *opts)
+				 const struct commit_graph_write_options *opts)
 {
 	struct oidset commits = OIDSET_INIT;
 	struct refs_cb_data data;
@@ -2565,7 +2565,7 @@ int write_commit_graph(struct repository *r,
 		       const struct string_list *const pack_indexes,
 		       struct oidset *commits,
 		       enum commit_graph_write_flags flags,
-		       const struct commit_graph_opts *opts)
+		       const struct commit_graph_write_options *opts)
 {
 	struct write_commit_graph_context ctx = {
 		.r = r,
diff --git a/commit-graph.h b/commit-graph.h
index 3b55192c2a1..513713ecb62 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -168,7 +168,7 @@ enum commit_graph_split_flags {
 	COMMIT_GRAPH_SPLIT_REPLACE          = 2
 };
 
-struct commit_graph_opts {
+struct commit_graph_write_options {
 	int size_multiple;
 	int max_commits;
 	timestamp_t expire_time;
@@ -176,6 +176,11 @@ struct commit_graph_opts {
 	int max_new_filters;
 };
 
+#define COMMIT_GRAPH_WRITE_OPTIONS_INIT { \
+	.size_multiple = 2, \
+	.max_new_filters = -1, \
+}
+
 /*
  * The write_commit_graph* methods return zero on success
  * and a negative value on failure. Note that if the repository
@@ -189,13 +194,13 @@ struct commit_graph_opts {
 int write_commit_graph_reachable(struct repository *repo,
 				 const char *dir,
 				 enum commit_graph_write_flags flags,
-				 const struct commit_graph_opts *opts);
+				 const struct commit_graph_write_options *opts);
 int write_commit_graph(struct repository *r,
 		       const char *dir,
 		       const struct string_list *pack_indexes,
 		       struct oidset *commits,
 		       enum commit_graph_write_flags flags,
-		       const struct commit_graph_opts *opts);
+		       const struct commit_graph_write_options *opts);
 
 #define COMMIT_GRAPH_VERIFY_SHALLOW	(1 << 0)
 

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 02/10] commit-graph: pass writer flags via options

[Patrick Steinhardt]

While we already accept an options structure when writing commit graphs,
we expect the caller to also pass a couple of other optional parameters
that control how exactly we perform the write. This mix is somewhat
arbitrary though and makes for an awkward calling convention, where it
is not immediately obvious which parameters are required and which are
options.

As a first step, move flags into the options structure and adapt callers
accordingly. The other parameters will follow in subsequent commits.

Note that with this change, passing the options structure now becomes
mandatory. This isn't much of an issue though because all callers do in
fact pass one now.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/commit-graph.c | 14 ++++++--------
 builtin/fetch.c        | 11 +++++------
 builtin/gc.c           | 12 ++++++++----
 commit-graph.c         | 24 +++++++++++-------------
 commit-graph.h         |  3 +--
 5 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 0445302ba8b..d49faf543b4 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -243,7 +243,6 @@ static int cmd_commit_graph_write(int argc,
 	struct oidset commits = OIDSET_INIT;
 	struct odb_files_dir *dir = NULL;
 	int result = 0;
-	enum commit_graph_write_flags flags = 0;
 	struct progress *progress = NULL;
 	struct option options[] = {
 		OPT_BOOL(0, "reachable", &opts.reachable,
@@ -290,23 +289,23 @@ static int cmd_commit_graph_write(int argc,
 	if (!opts.obj_dir)
 		opts.obj_dir = repo_get_object_directory(repo);
 	if (opts.append)
-		flags |= COMMIT_GRAPH_WRITE_APPEND;
+		write_opts.flags |= COMMIT_GRAPH_WRITE_APPEND;
 	if (opts.split)
-		flags |= COMMIT_GRAPH_WRITE_SPLIT;
+		write_opts.flags |= COMMIT_GRAPH_WRITE_SPLIT;
 	if (opts.progress)
-		flags |= COMMIT_GRAPH_WRITE_PROGRESS;
+		write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
 	if (!opts.enable_changed_paths)
-		flags |= COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS;
+		write_opts.flags |= COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS;
 	if (opts.enable_changed_paths == 1 ||
 	    git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
-		flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
+		write_opts.flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
 
 	dir = odb_source_files_find_dir(repo->objects, opts.obj_dir);
 	if (!dir)
 		die(_("could not find object directory matching %s"), opts.obj_dir);
 
 	if (opts.reachable) {
-		if (write_commit_graph_reachable(repo, dir->abspath, flags, &write_opts))
+		if (write_commit_graph_reachable(repo, dir->abspath, &write_opts))
 			result = 1;
 		goto cleanup;
 	}
@@ -346,7 +345,6 @@ static int cmd_commit_graph_write(int argc,
 	if (write_commit_graph(repo, dir->abspath,
 			       opts.stdin_packs ? &pack_indexes : NULL,
 			       opts.stdin_commits ? &commits : NULL,
-			       flags,
 			       &write_opts))
 		result = 1;
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 09966e5ce8d..b76e7725728 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2936,10 +2936,11 @@ int cmd_fetch(int argc,
 	if (fetch_write_commit_graph > 0 ||
 	    (fetch_write_commit_graph < 0 &&
 	     the_repository->settings.fetch_write_commit_graph)) {
-		int commit_graph_flags = COMMIT_GRAPH_WRITE_SPLIT;
+		struct commit_graph_write_options write_opts = COMMIT_GRAPH_WRITE_OPTIONS_INIT;
 
+		write_opts.flags |= COMMIT_GRAPH_WRITE_SPLIT;
 		if (progress)
-			commit_graph_flags |= COMMIT_GRAPH_WRITE_PROGRESS;
+			write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
 
 		if (graph_write_mode != GRAPH_WRITE_SKIP) {
 			trace2_region_enter("fetch", "write-commit-graph",
@@ -2948,13 +2949,11 @@ int cmd_fetch(int argc,
 				write_commit_graph(
 					the_repository,
 					the_repository->objects->source->path,
-					NULL, &updated_tips,
-					commit_graph_flags, NULL);
+					NULL, &updated_tips, &write_opts);
 			else
 				write_commit_graph_reachable(
 					the_repository,
-					the_repository->objects->source->path,
-					commit_graph_flags, NULL);
+					the_repository->objects->source->path, &write_opts);
 			trace2_region_leave("fetch", "write-commit-graph",
 					    the_repository);
 		}
diff --git a/builtin/gc.c b/builtin/gc.c
index 81cecf6b6c3..a157909ed54 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -734,11 +734,15 @@ int cmd_gc(int argc,
 		clean_pack_garbage();
 	}
 
-	if (the_repository->settings.gc_write_commit_graph == 1)
+	if (the_repository->settings.gc_write_commit_graph == 1) {
+		struct commit_graph_write_options write_opts = COMMIT_GRAPH_WRITE_OPTIONS_INIT;
+
+		if (!opts.quiet && !daemonized)
+			write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
+
 		write_commit_graph_reachable(the_repository,
-					     the_repository->objects->source->path,
-					     !opts.quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,
-					     NULL);
+					     the_repository->objects->source->path, &write_opts);
+	}
 
 	if (!daemonized) {
 		char *path = repo_git_path(the_repository, "gc.log");
diff --git a/commit-graph.c b/commit-graph.c
index 2f9bf7a44fd..9216d051c1f 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -32,14 +32,15 @@
 
 void git_test_write_commit_graph_or_die(struct repository *repo)
 {
-	int flags = 0;
+	struct commit_graph_write_options opts = COMMIT_GRAPH_WRITE_OPTIONS_INIT;
+
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0))
 		return;
 
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
-		flags = COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
+		opts.flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
 
-	if (write_commit_graph_reachable(repo, repo->objects->source->path, flags, NULL))
+	if (write_commit_graph_reachable(repo, repo->objects->source->path, &opts))
 		die("failed to write commit-graph under GIT_TEST_COMMIT_GRAPH");
 }
 
@@ -1915,7 +1916,6 @@ static int add_ref_to_set(const struct reference *ref, void *cb_data)
 
 int write_commit_graph_reachable(struct repository *repo,
 				 const char *dir,
-				 enum commit_graph_write_flags flags,
 				 const struct commit_graph_write_options *opts)
 {
 	struct oidset commits = OIDSET_INIT;
@@ -1926,7 +1926,7 @@ int write_commit_graph_reachable(struct repository *repo,
 	data.repo = repo;
 	data.commits = &commits;
 
-	if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
+	if (opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		data.progress = start_delayed_progress(
 			repo,
 			_("Collecting referenced commits"), 0);
@@ -1936,8 +1936,7 @@ int write_commit_graph_reachable(struct repository *repo,
 
 	stop_progress(&data.progress);
 
-	result = write_commit_graph(repo, dir, NULL, &commits,
-				    flags, opts);
+	result = write_commit_graph(repo, dir, NULL, &commits, opts);
 
 	oidset_clear(&commits);
 	return result;
@@ -2564,14 +2563,13 @@ int write_commit_graph(struct repository *r,
 		       const char *dir,
 		       const struct string_list *const pack_indexes,
 		       struct oidset *commits,
-		       enum commit_graph_write_flags flags,
 		       const struct commit_graph_write_options *opts)
 {
 	struct write_commit_graph_context ctx = {
 		.r = r,
-		.append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0,
-		.report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0,
-		.split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0,
+		.append = opts->flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0,
+		.report_progress = opts->flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0,
+		.split = opts->flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0,
 		.opts = opts,
 		.total_bloom_filter_data_size = 0,
 		.write_generation_data = (get_configured_generation_version(r) == 2),
@@ -2617,9 +2615,9 @@ int write_commit_graph(struct repository *r,
 	for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
 		chain->topo_levels = &topo_levels;
 
-	if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
+	if (opts->flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
 		ctx.changed_paths = 1;
-	if (!(flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) {
+	if (!(opts->flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) {
 		/* We have changed-paths already. Keep them in the next graph */
 		if (g && g->bloom_filter_settings) {
 			ctx.changed_paths = 1;
diff --git a/commit-graph.h b/commit-graph.h
index 513713ecb62..7461093a990 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -169,6 +169,7 @@ enum commit_graph_split_flags {
 };
 
 struct commit_graph_write_options {
+	enum commit_graph_write_flags flags;
 	int size_multiple;
 	int max_commits;
 	timestamp_t expire_time;
@@ -193,13 +194,11 @@ struct commit_graph_write_options {
  */
 int write_commit_graph_reachable(struct repository *repo,
 				 const char *dir,
-				 enum commit_graph_write_flags flags,
 				 const struct commit_graph_write_options *opts);
 int write_commit_graph(struct repository *r,
 		       const char *dir,
 		       const struct string_list *pack_indexes,
 		       struct oidset *commits,
-		       enum commit_graph_write_flags flags,
 		       const struct commit_graph_write_options *opts);
 
 #define COMMIT_GRAPH_VERIFY_SHALLOW	(1 << 0)

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 03/10] commit-graph: drop `NULL` checks for write options

[Patrick Steinhardt]

The commit-graph writer still handles the case where `ctx->opts` is not
set by falling back to defaults sprinkled across several functions. But
since we have moved the write flags into the options structure in the
preceding commit all callers pass it, and we can thus start to treat it
as a mandatory parameter now.

Drop the checks and use the options unconditionally.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 commit-graph.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index 9216d051c1f..e86569b7ced 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1568,8 +1568,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
 {
 	int i;
 	struct commit *commit;
-	enum commit_graph_split_flags flags = ctx->opts ?
-		ctx->opts->split_flags : COMMIT_GRAPH_SPLIT_UNSPECIFIED;
+	enum commit_graph_split_flags flags = ctx->opts->split_flags;
 
 	if (ctx->report_progress)
 		ctx->progress = start_delayed_progress(
@@ -1858,7 +1857,7 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx)
 	else
 		QSORT(sorted_commits, ctx->commits.nr, commit_gen_cmp);
 
-	max_new_filters = ctx->opts && ctx->opts->max_new_filters >= 0 ?
+	max_new_filters = ctx->opts->max_new_filters >= 0 ?
 		ctx->opts->max_new_filters : ctx->commits.nr;
 
 	for (i = 0; i < ctx->commits.nr; i++) {
@@ -2035,8 +2034,7 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
 static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
 {
 	uint32_t i;
-	enum commit_graph_split_flags flags = ctx->opts ?
-		ctx->opts->split_flags : COMMIT_GRAPH_SPLIT_UNSPECIFIED;
+	enum commit_graph_split_flags flags = ctx->opts->split_flags;
 
 	ctx->num_extra_edges = 0;
 	if (ctx->report_progress)
@@ -2290,20 +2288,13 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx,
 {
 	struct commit_graph *g;
 	uint32_t num_commits;
-	enum commit_graph_split_flags flags = COMMIT_GRAPH_SPLIT_UNSPECIFIED;
-	uint32_t i;
-
-	int max_commits = 0;
+	enum commit_graph_split_flags flags = ctx->opts->split_flags;
+	int max_commits = ctx->opts->max_commits;
 	int size_mult = 2;
+	uint32_t i;
 
-	if (ctx->opts) {
-		max_commits = ctx->opts->max_commits;
-
-		if (ctx->opts->size_multiple)
-			size_mult = ctx->opts->size_multiple;
-
-		flags = ctx->opts->split_flags;
-	}
+	if (ctx->opts->size_multiple)
+		size_mult = ctx->opts->size_multiple;
 
 	g = graph_to_merge;
 	num_commits = ctx->commits.nr;
@@ -2508,7 +2499,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 	size_t dirnamelen;
 	timestamp_t expire_time = time(NULL);
 
-	if (ctx->opts && ctx->opts->expire_time)
+	if (ctx->opts->expire_time)
 		expire_time = ctx->opts->expire_time;
 	if (!ctx->split) {
 		char *chain_file_name = get_commit_graph_chain_filename(ctx->dir);
@@ -2645,8 +2636,7 @@ int write_commit_graph(struct repository *r,
 				ctx.commit_graph_filenames_before[--i] = xstrdup(chain->filename);
 		}
 
-		if (ctx.opts)
-			replace = ctx.opts->split_flags & COMMIT_GRAPH_SPLIT_REPLACE;
+		replace = ctx.opts->split_flags & COMMIT_GRAPH_SPLIT_REPLACE;
 	}
 
 	if (odb_count_objects(r->objects, ODB_COUNT_OBJECTS_APPROXIMATE, &ctx.approx_nr_objects) < 0)

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 04/10] commit-graph: drop redundant flags in write context

[Patrick Steinhardt]

In the preceding commit we have moved the flags that control how we
write a commit graph into its options structure. As a consequence we
have those flags implicitly available in the write context, too, as it
already contains a pointer to the options struct. This makes a couple of
individual flags that we track in the write context redundant as they
are a simple alias of the flags.

Drop these redundant flags.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 commit-graph.c | 61 ++++++++++++++++++++++++++++------------------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index e86569b7ced..4f3563f4d0b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1158,10 +1158,7 @@ struct write_commit_graph_context {
 	uint32_t new_num_commits_in_base;
 	struct commit_graph *new_base_graph;
 
-	unsigned append:1,
-		 report_progress:1,
-		 split:1,
-		 changed_paths:1,
+	unsigned changed_paths:1,
 		 order_by_pack:1,
 		 write_generation_data:1,
 		 trust_generation_numbers:1;
@@ -1570,7 +1567,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
 	struct commit *commit;
 	enum commit_graph_split_flags flags = ctx->opts->split_flags;
 
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		ctx->progress = start_delayed_progress(
 					ctx->r,
 					_("Loading known commits in commit graph"),
@@ -1588,7 +1585,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
 	 * than the number of missing commits in the reachable
 	 * closure.
 	 */
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		ctx->progress = start_delayed_progress(
 					ctx->r,
 					_("Expanding reachable commits in commit graph"),
@@ -1599,7 +1596,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
 
 		if (!commit)
 			continue;
-		if (ctx->split) {
+		if (ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT) {
 			if ((!repo_parse_commit(ctx->r, commit) &&
 			     commit_graph_position(commit) == COMMIT_NOT_FROM_GRAPH) ||
 			    flags == COMMIT_GRAPH_SPLIT_REPLACE)
@@ -1609,7 +1606,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
 	}
 	stop_progress(&ctx->progress);
 
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		ctx->progress = start_delayed_progress(
 					ctx->r,
 					_("Clearing commit marks in commit graph"),
@@ -1731,7 +1728,7 @@ static void compute_topological_levels(struct write_commit_graph_context *ctx)
 		.data = ctx,
 	};
 
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		info.progress = ctx->progress
 			      = start_delayed_progress(
 					ctx->r,
@@ -1766,7 +1763,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 		.set_generation = set_generation_v2,
 	};
 
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		info.progress = ctx->progress
 			      = start_delayed_progress(
 					ctx->r,
@@ -1844,7 +1841,7 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx)
 
 	init_bloom_filters();
 
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		progress = start_delayed_progress(
 			ctx->r,
 			_("Computing commit changed paths Bloom filters"),
@@ -1948,7 +1945,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
 	struct strbuf progress_title = STRBUF_INIT;
 	int ret = 0;
 
-	if (ctx->report_progress) {
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS) {
 		strbuf_addf(&progress_title,
 			    Q_("Finding commits for commit graph in %"PRIuMAX" pack",
 			       "Finding commits for commit graph in %"PRIuMAX" packs",
@@ -2014,7 +2011,7 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
 		.typep = &type,
 	};
 
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		ctx->progress = start_delayed_progress(
 			ctx->r,
 			_("Finding commits for commit graph among packed objects"),
@@ -2037,7 +2034,7 @@ static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
 	enum commit_graph_split_flags flags = ctx->opts->split_flags;
 
 	ctx->num_extra_edges = 0;
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		ctx->progress = start_delayed_progress(
 			ctx->r,
 			_("Finding extra edges in commit graph"),
@@ -2051,11 +2048,13 @@ static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
 
 		commit = lookup_commit(ctx->r, &ctx->oids.oid[i]);
 
-		if (ctx->split && flags != COMMIT_GRAPH_SPLIT_REPLACE &&
+		if ((ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT) &&
+		    flags != COMMIT_GRAPH_SPLIT_REPLACE &&
 		    commit_graph_position(commit) != COMMIT_NOT_FROM_GRAPH)
 			continue;
 
-		if (ctx->split && flags == COMMIT_GRAPH_SPLIT_REPLACE)
+		if ((ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT) &&
+		    flags == COMMIT_GRAPH_SPLIT_REPLACE)
 			repo_parse_commit(ctx->r, commit);
 		else
 			repo_parse_commit_no_graph(ctx->r, commit);
@@ -2100,14 +2099,14 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 {
 	uint32_t i;
 	struct hashfile *f;
-	struct tempfile *graph_layer; /* when ctx->split is non-zero */
+	struct tempfile *graph_layer; /* when COMMIT_GRAPH_WRITE_SPLIT is non-zero */
 	struct lock_file lk = LOCK_INIT;
 	const unsigned hashsz = ctx->r->hash_algo->rawsz;
 	struct strbuf progress_title = STRBUF_INIT;
 	struct chunkfile *cf;
 	unsigned char file_hash[GIT_MAX_RAWSZ];
 
-	if (ctx->split) {
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT) {
 		struct strbuf tmp_file = STRBUF_INIT;
 
 		strbuf_addf(&tmp_file,
@@ -2124,7 +2123,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 		return -1;
 	}
 
-	if (ctx->split) {
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT) {
 		char *lock_name = get_commit_graph_chain_filename(ctx->dir);
 
 		repo_hold_lock_file_for_update_mode(ctx->r, &lk, lock_name,
@@ -2195,7 +2194,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 	hashwrite_u8(f, get_num_chunks(cf));
 	hashwrite_u8(f, ctx->num_commit_graphs_after - 1);
 
-	if (ctx->report_progress) {
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS) {
 		strbuf_addf(&progress_title,
 			    Q_("Writing out commit graph in %d pass",
 			       "Writing out commit graph in %d passes",
@@ -2212,7 +2211,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 	stop_progress(&ctx->progress);
 	strbuf_release(&progress_title);
 
-	if (ctx->split && ctx->base_graph_name && ctx->num_commit_graphs_after > 1) {
+	if ((ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT) &&
+	    ctx->base_graph_name && ctx->num_commit_graphs_after > 1) {
 		char *new_base_hash = xstrdup(oid_to_hex(&ctx->new_base_graph->oid));
 		char *new_base_name = get_split_graph_filename(ctx->new_base_graph->dir, new_base_hash);
 
@@ -2227,7 +2227,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 			  CSUM_HASH_IN_STREAM | CSUM_FSYNC);
 	free_chunkfile(cf);
 
-	if (ctx->split) {
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT) {
 		FILE *chainf = fdopen_lock_file(&lk, "w");
 		char *final_graph_name;
 		int result;
@@ -2406,7 +2406,7 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx)
 {
 	uint32_t i, dedup_i = 0;
 
-	if (ctx->report_progress)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		ctx->progress = start_delayed_progress(
 					ctx->r,
 					_("Scanning merged commits"),
@@ -2452,7 +2452,7 @@ static void merge_commit_graphs(struct write_commit_graph_context *ctx,
 	while (g && current_graph_number >= ctx->num_commit_graphs_after) {
 		current_graph_number--;
 
-		if (ctx->report_progress)
+		if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 			ctx->progress = start_delayed_progress(ctx->r,
 							       _("Merging commit-graph"), 0);
 
@@ -2501,7 +2501,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 
 	if (ctx->opts->expire_time)
 		expire_time = ctx->opts->expire_time;
-	if (!ctx->split) {
+	if (!(ctx->opts->flags & COMMIT_GRAPH_WRITE_SPLIT)) {
 		char *chain_file_name = get_commit_graph_chain_filename(ctx->dir);
 		unlink(chain_file_name);
 		free(chain_file_name);
@@ -2558,9 +2558,6 @@ int write_commit_graph(struct repository *r,
 {
 	struct write_commit_graph_context ctx = {
 		.r = r,
-		.append = opts->flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0,
-		.report_progress = opts->flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0,
-		.split = opts->flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0,
 		.opts = opts,
 		.total_bloom_filter_data_size = 0,
 		.write_generation_data = (get_configured_generation_version(r) == 2),
@@ -2624,7 +2621,7 @@ int write_commit_graph(struct repository *r,
 
 	bloom_settings.hash_version = bloom_settings.hash_version == 2 ? 2 : 1;
 
-	if (ctx.split) {
+	if (ctx.opts->flags & COMMIT_GRAPH_WRITE_SPLIT) {
 		for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
 			ctx.num_commit_graphs_before++;
 
@@ -2642,7 +2639,7 @@ int write_commit_graph(struct repository *r,
 	if (odb_count_objects(r->objects, ODB_COUNT_OBJECTS_APPROXIMATE, &ctx.approx_nr_objects) < 0)
 		ctx.approx_nr_objects = 0;
 
-	if (ctx.append && g) {
+	if ((ctx.opts->flags & COMMIT_GRAPH_WRITE_APPEND) && g) {
 		for (i = 0; i < g->num_commits; i++) {
 			struct object_id oid;
 			oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_algo->rawsz, i),
@@ -2680,7 +2677,7 @@ int write_commit_graph(struct repository *r,
 	if (!ctx.commits.nr && !replace)
 		goto cleanup;
 
-	if (ctx.split) {
+	if (ctx.opts->flags & COMMIT_GRAPH_WRITE_SPLIT) {
 		split_graph_merge_strategy(&ctx, g);
 
 		if (!replace)
@@ -2703,7 +2700,7 @@ int write_commit_graph(struct repository *r,
 	if (ctx.changed_paths)
 		deinit_bloom_filters();
 
-	if (ctx.split)
+	if (ctx.opts->flags & COMMIT_GRAPH_WRITE_SPLIT)
 		mark_commit_graphs(&ctx);
 
 	expire_commit_graphs(&ctx);

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 05/10] commit-graph: merge the two commit graph writers

[Patrick Steinhardt]

We have two entry points for writing commit graphs:

  - `write_commit_graph()` either writes a commit graph for all commits
    in the object database, or for the commits specified via its options
    struct.

  - `write_commit_graph_reachable()` writes a commit graph for all
    reachable commits.

It's somewhat pointless though to have a separate entry point for
writing a commit graph for reachable commits, as this use case can be
trivially satisfied via a flag.

Introduce that flag and drop the separate entry point in favor of
`write_commit_graph()`. This ensures we have a single entry point for
writing commit graphs, only, which will make it easier to eventually
introduce pluggable commit graphs via the object database.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/commit-graph.c |  8 ++------
 builtin/fetch.c        |  7 +++++--
 builtin/gc.c           |  5 +++--
 commit-graph.c         | 47 ++++++++++++++++++++++-------------------------
 commit-graph.h         |  4 +---
 5 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index d49faf543b4..644f7736c91 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -299,17 +299,13 @@ static int cmd_commit_graph_write(int argc,
 	if (opts.enable_changed_paths == 1 ||
 	    git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
 		write_opts.flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
+	if (opts.reachable)
+		write_opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
 
 	dir = odb_source_files_find_dir(repo->objects, opts.obj_dir);
 	if (!dir)
 		die(_("could not find object directory matching %s"), opts.obj_dir);
 
-	if (opts.reachable) {
-		if (write_commit_graph_reachable(repo, dir->abspath, &write_opts))
-			result = 1;
-		goto cleanup;
-	}
-
 	if (opts.stdin_packs) {
 		struct strbuf packname = STRBUF_INIT;
 		size_t dirlen;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b76e7725728..637d7ddaa06 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2941,6 +2941,8 @@ int cmd_fetch(int argc,
 		write_opts.flags |= COMMIT_GRAPH_WRITE_SPLIT;
 		if (progress)
 			write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
+		if (graph_write_mode != GRAPH_WRITE_TIPS)
+			write_opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
 
 		if (graph_write_mode != GRAPH_WRITE_SKIP) {
 			trace2_region_enter("fetch", "write-commit-graph",
@@ -2951,9 +2953,10 @@ int cmd_fetch(int argc,
 					the_repository->objects->source->path,
 					NULL, &updated_tips, &write_opts);
 			else
-				write_commit_graph_reachable(
+				write_commit_graph(
 					the_repository,
-					the_repository->objects->source->path, &write_opts);
+					the_repository->objects->source->path,
+					NULL, NULL, &write_opts);
 			trace2_region_leave("fetch", "write-commit-graph",
 					    the_repository);
 		}
diff --git a/builtin/gc.c b/builtin/gc.c
index a157909ed54..709789dbf2c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -737,11 +737,12 @@ int cmd_gc(int argc,
 	if (the_repository->settings.gc_write_commit_graph == 1) {
 		struct commit_graph_write_options write_opts = COMMIT_GRAPH_WRITE_OPTIONS_INIT;
 
+		write_opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
 		if (!opts.quiet && !daemonized)
 			write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
 
-		write_commit_graph_reachable(the_repository,
-					     the_repository->objects->source->path, &write_opts);
+		write_commit_graph(the_repository, the_repository->objects->source->path,
+				   NULL, NULL, &write_opts);
 	}
 
 	if (!daemonized) {
diff --git a/commit-graph.c b/commit-graph.c
index 4f3563f4d0b..97be0f71484 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -37,10 +37,11 @@ void git_test_write_commit_graph_or_die(struct repository *repo)
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0))
 		return;
 
+	opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
 		opts.flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
 
-	if (write_commit_graph_reachable(repo, repo->objects->source->path, &opts))
+	if (write_commit_graph(repo, repo->objects->source->path, NULL, NULL, &opts))
 		die("failed to write commit-graph under GIT_TEST_COMMIT_GRAPH");
 }
 
@@ -1910,32 +1911,23 @@ static int add_ref_to_set(const struct reference *ref, void *cb_data)
 	return 0;
 }
 
-int write_commit_graph_reachable(struct repository *repo,
-				 const char *dir,
-				 const struct commit_graph_write_options *opts)
+static void fill_oids_from_reachable(struct write_commit_graph_context *ctx,
+				     struct oidset *commits)
 {
-	struct oidset commits = OIDSET_INIT;
-	struct refs_cb_data data;
-	int result;
-
-	memset(&data, 0, sizeof(data));
-	data.repo = repo;
-	data.commits = &commits;
+	struct refs_cb_data data = {
+		.repo = ctx->r,
+		.commits = commits,
+	};
 
-	if (opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
+	if (ctx->opts->flags & COMMIT_GRAPH_WRITE_PROGRESS)
 		data.progress = start_delayed_progress(
-			repo,
+			ctx->r,
 			_("Collecting referenced commits"), 0);
 
-	refs_for_each_ref(get_main_ref_store(repo), add_ref_to_set,
+	refs_for_each_ref(get_main_ref_store(ctx->r), add_ref_to_set,
 			  &data);
 
 	stop_progress(&data.progress);
-
-	result = write_commit_graph(repo, dir, NULL, &commits, opts);
-
-	oidset_clear(&commits);
-	return result;
 }
 
 static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
@@ -2567,9 +2559,13 @@ int write_commit_graph(struct repository *r,
 	int res = 0;
 	int replace = 0;
 	struct bloom_filter_settings bloom_settings = DEFAULT_BLOOM_FILTER_SETTINGS;
+	struct oidset reachable_commits = OIDSET_INIT;
 	struct topo_level_slab topo_levels;
 	struct commit_graph *g;
 
+	if (!!pack_indexes + !!commits + !!(opts->flags & COMMIT_GRAPH_WRITE_REACHABLE) > 1)
+		BUG("attempting to write commit-graph with mutually incompatible options");
+
 	prepare_repo_settings(r);
 	if (!r->settings.core_commit_graph) {
 		warning(_("attempting to write a commit-graph, but 'core.commitGraph' is disabled"));
@@ -2652,14 +2648,14 @@ int write_commit_graph(struct repository *r,
 		ctx.order_by_pack = 1;
 		if ((res = fill_oids_from_packs(&ctx, pack_indexes)))
 			goto cleanup;
-	}
-
-	if (commits) {
+	} else if (commits) {
 		if ((res = fill_oids_from_commits(&ctx, commits)))
 			goto cleanup;
-	}
-
-	if (!pack_indexes && !commits) {
+	} else if (opts->flags & COMMIT_GRAPH_WRITE_REACHABLE) {
+		fill_oids_from_reachable(&ctx, &reachable_commits);
+		if ((res = fill_oids_from_commits(&ctx, &reachable_commits)))
+			goto cleanup;
+	} else {
 		ctx.order_by_pack = 1;
 		fill_oids_from_all_packs(&ctx);
 	}
@@ -2710,6 +2706,7 @@ int write_commit_graph(struct repository *r,
 	free(ctx.base_graph_name);
 	free(ctx.dir);
 	commit_stack_clear(&ctx.commits);
+	oidset_clear(&reachable_commits);
 	oid_array_clear(&ctx.oids);
 	clear_topo_level_slab(&topo_levels);
 
diff --git a/commit-graph.h b/commit-graph.h
index 7461093a990..4adbd5f18d9 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -160,6 +160,7 @@ enum commit_graph_write_flags {
 	COMMIT_GRAPH_WRITE_SPLIT      = (1 << 2),
 	COMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 3),
 	COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS = (1 << 4),
+	COMMIT_GRAPH_WRITE_REACHABLE  = (1 << 5),
 };
 
 enum commit_graph_split_flags {
@@ -192,9 +193,6 @@ struct commit_graph_write_options {
  * canonicalized internally so that it compares equal to the directory of
  * graphs that have already been loaded.
  */
-int write_commit_graph_reachable(struct repository *repo,
-				 const char *dir,
-				 const struct commit_graph_write_options *opts);
 int write_commit_graph(struct repository *r,
 		       const char *dir,
 		       const struct string_list *pack_indexes,

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 06/10] commit-graph: move remaining write parameters into options

[Patrick Steinhardt]

Move the remaining optional parameters that can be passed to
`write_commit_graph()` into its options structure to simplify the
function's calling convention.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/commit-graph.c |  7 +++----
 builtin/fetch.c        | 15 +++++----------
 builtin/gc.c           |  2 +-
 commit-graph.c         | 14 ++++++--------
 commit-graph.h         |  4 ++--
 5 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 644f7736c91..f2c374d0176 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -319,6 +319,7 @@ static int cmd_commit_graph_write(int argc,
 			string_list_append(&pack_indexes, packname.buf);
 		}
 
+		write_opts.pack_indexes = &pack_indexes;
 		strbuf_release(&packname);
 	} else if (opts.stdin_commits) {
 		oidset_init(&commits, 0);
@@ -335,13 +336,11 @@ static int cmd_commit_graph_write(int argc,
 			}
 		}
 
+		write_opts.commits = &commits;
 		stop_progress(&progress);
 	}
 
-	if (write_commit_graph(repo, dir->abspath,
-			       opts.stdin_packs ? &pack_indexes : NULL,
-			       opts.stdin_commits ? &commits : NULL,
-			       &write_opts))
+	if (write_commit_graph(repo, dir->abspath, &write_opts))
 		result = 1;
 
 cleanup:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 637d7ddaa06..7278cee8ca7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2943,20 +2943,15 @@ int cmd_fetch(int argc,
 			write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
 		if (graph_write_mode != GRAPH_WRITE_TIPS)
 			write_opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
+		else
+			write_opts.commits = &updated_tips;
 
 		if (graph_write_mode != GRAPH_WRITE_SKIP) {
 			trace2_region_enter("fetch", "write-commit-graph",
 					    the_repository);
-			if (graph_write_mode == GRAPH_WRITE_TIPS)
-				write_commit_graph(
-					the_repository,
-					the_repository->objects->source->path,
-					NULL, &updated_tips, &write_opts);
-			else
-				write_commit_graph(
-					the_repository,
-					the_repository->objects->source->path,
-					NULL, NULL, &write_opts);
+			write_commit_graph(the_repository,
+					   the_repository->objects->source->path,
+					   &write_opts);
 			trace2_region_leave("fetch", "write-commit-graph",
 					    the_repository);
 		}
diff --git a/builtin/gc.c b/builtin/gc.c
index 709789dbf2c..7bf88861df0 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -742,7 +742,7 @@ int cmd_gc(int argc,
 			write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
 
 		write_commit_graph(the_repository, the_repository->objects->source->path,
-				   NULL, NULL, &write_opts);
+				   &write_opts);
 	}
 
 	if (!daemonized) {
diff --git a/commit-graph.c b/commit-graph.c
index 97be0f71484..077213b5336 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -41,7 +41,7 @@ void git_test_write_commit_graph_or_die(struct repository *repo)
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
 		opts.flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
 
-	if (write_commit_graph(repo, repo->objects->source->path, NULL, NULL, &opts))
+	if (write_commit_graph(repo, repo->objects->source->path, &opts))
 		die("failed to write commit-graph under GIT_TEST_COMMIT_GRAPH");
 }
 
@@ -2544,8 +2544,6 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 
 int write_commit_graph(struct repository *r,
 		       const char *dir,
-		       const struct string_list *const pack_indexes,
-		       struct oidset *commits,
 		       const struct commit_graph_write_options *opts)
 {
 	struct write_commit_graph_context ctx = {
@@ -2563,7 +2561,7 @@ int write_commit_graph(struct repository *r,
 	struct topo_level_slab topo_levels;
 	struct commit_graph *g;
 
-	if (!!pack_indexes + !!commits + !!(opts->flags & COMMIT_GRAPH_WRITE_REACHABLE) > 1)
+	if (!!opts->pack_indexes + !!opts->commits + !!(opts->flags & COMMIT_GRAPH_WRITE_REACHABLE) > 1)
 		BUG("attempting to write commit-graph with mutually incompatible options");
 
 	prepare_repo_settings(r);
@@ -2644,12 +2642,12 @@ int write_commit_graph(struct repository *r,
 		}
 	}
 
-	if (pack_indexes) {
+	if (opts->pack_indexes) {
 		ctx.order_by_pack = 1;
-		if ((res = fill_oids_from_packs(&ctx, pack_indexes)))
+		if ((res = fill_oids_from_packs(&ctx, opts->pack_indexes)))
 			goto cleanup;
-	} else if (commits) {
-		if ((res = fill_oids_from_commits(&ctx, commits)))
+	} else if (opts->commits) {
+		if ((res = fill_oids_from_commits(&ctx, opts->commits)))
 			goto cleanup;
 	} else if (opts->flags & COMMIT_GRAPH_WRITE_REACHABLE) {
 		fill_oids_from_reachable(&ctx, &reachable_commits);
diff --git a/commit-graph.h b/commit-graph.h
index 4adbd5f18d9..0e03e82a04a 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -171,6 +171,8 @@ enum commit_graph_split_flags {
 
 struct commit_graph_write_options {
 	enum commit_graph_write_flags flags;
+	const struct string_list *pack_indexes;
+	struct oidset *commits;
 	int size_multiple;
 	int max_commits;
 	timestamp_t expire_time;
@@ -195,8 +197,6 @@ struct commit_graph_write_options {
  */
 int write_commit_graph(struct repository *r,
 		       const char *dir,
-		       const struct string_list *pack_indexes,
-		       struct oidset *commits,
 		       const struct commit_graph_write_options *opts);
 
 #define COMMIT_GRAPH_VERIFY_SHALLOW	(1 << 0)

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 07/10] commit-graph: fix mixup of write/verify flags

[Patrick Steinhardt]

While we already have flags specific to `verify_commit_graph()`, the
function also uses `COMMIT_GRAPH_WRITE_PROGRESS`. This mixup is a bit of
a footgun though, as it may easily happen that the value of that flag
is the exact same value as a future `COMMIT_GRAPH_VERIFY_*` flag.

Fix this mixup by introducing a new `COMMIT_GRAPH_VERIFY_PROGRESS` flag.
While at it, convert the verify flags into an enum.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/commit-graph.c | 4 ++--
 commit-graph.c         | 5 +++--
 commit-graph.h         | 8 ++++++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index f2c374d0176..09b09d6ac5c 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -70,9 +70,9 @@ static int cmd_commit_graph_verify(int argc,
 	char *graph_name;
 	char *chain_name;
 	enum { OPENED_NONE, OPENED_GRAPH, OPENED_CHAIN } opened = OPENED_NONE;
+	enum commit_graph_verify_flags flags = 0;
 	int fd;
 	struct stat st;
-	int flags = 0;
 	int incomplete_chain = 0;
 	int ret;
 	struct option options[] = {
@@ -97,7 +97,7 @@ static int cmd_commit_graph_verify(int argc,
 	if (opts.shallow)
 		flags |= COMMIT_GRAPH_VERIFY_SHALLOW;
 	if (opts.progress)
-		flags |= COMMIT_GRAPH_WRITE_PROGRESS;
+		flags |= COMMIT_GRAPH_VERIFY_PROGRESS;
 
 	dir = odb_source_files_find_dir(repo->objects, opts.obj_dir);
 	if (!dir)
diff --git a/commit-graph.c b/commit-graph.c
index 077213b5336..c82776b0667 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2904,7 +2904,8 @@ static int verify_one_commit_graph(struct commit_graph *g,
 	return verify_commit_graph_error;
 }
 
-int verify_commit_graph(struct commit_graph *g, int flags)
+int verify_commit_graph(struct commit_graph *g,
+			enum commit_graph_verify_flags flags)
 {
 	struct progress *progress = NULL;
 	int local_error = 0;
@@ -2915,7 +2916,7 @@ int verify_commit_graph(struct commit_graph *g, int flags)
 		return 1;
 	}
 
-	if (flags & COMMIT_GRAPH_WRITE_PROGRESS) {
+	if (flags & COMMIT_GRAPH_VERIFY_PROGRESS) {
 		uint64_t total = g->num_commits;
 		if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW))
 			total += g->num_commits_in_base;
diff --git a/commit-graph.h b/commit-graph.h
index 0e03e82a04a..80d59bd23bd 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -199,9 +199,13 @@ int write_commit_graph(struct repository *r,
 		       const char *dir,
 		       const struct commit_graph_write_options *opts);
 
-#define COMMIT_GRAPH_VERIFY_SHALLOW	(1 << 0)
+enum commit_graph_verify_flags {
+	COMMIT_GRAPH_VERIFY_SHALLOW = (1 << 0),
+	COMMIT_GRAPH_VERIFY_PROGRESS = (1 << 1),
+};
 
-int verify_commit_graph(struct commit_graph *g, int flags);
+int verify_commit_graph(struct commit_graph *g,
+			enum commit_graph_verify_flags flags);
 
 void close_commit_graph(struct object_database *);
 void free_commit_graph(struct commit_graph *);

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 08/10] builtin/commit-graph: set flags directly

[Patrick Steinhardt]

We have a bit of an unnecessary indirection when parsing flags for both
the "write" and "verify" subcommands: instead of setting the flags
directly via a local flags variable, we instead populate them into a
`struct opts_commit_graph` only to then set the flag. This is awkward
and unnecessarily verbose.

Drop the separate flags in `struct opts_commit_graph` and set them
directly via `OPT_BIT()` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/commit-graph.c | 54 ++++++++++++++++++--------------------------------
 1 file changed, 19 insertions(+), 35 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 09b09d6ac5c..218c30ed3ed 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -42,13 +42,8 @@ static char const * const builtin_commit_graph_usage[] = {
 
 struct opts_commit_graph {
 	const char *obj_dir;
-	int reachable;
 	int stdin_packs;
 	int stdin_commits;
-	int append;
-	int split;
-	int shallow;
-	int progress;
 	int enable_changed_paths;
 };
 
@@ -62,24 +57,23 @@ static int cmd_commit_graph_verify(int argc,
 				   const char *prefix,
 				   struct repository *repo)
 {
-	struct opts_commit_graph opts = {
-		.progress = isatty(2),
-	};
+	struct opts_commit_graph opts = { 0 };
 	struct commit_graph *graph = NULL;
 	struct odb_files_dir *dir = NULL;
 	char *graph_name;
 	char *chain_name;
 	enum { OPENED_NONE, OPENED_GRAPH, OPENED_CHAIN } opened = OPENED_NONE;
-	enum commit_graph_verify_flags flags = 0;
+	enum commit_graph_verify_flags flags = isatty(2) ? COMMIT_GRAPH_VERIFY_PROGRESS : 0;
 	int fd;
 	struct stat st;
 	int incomplete_chain = 0;
 	int ret;
 	struct option options[] = {
-		OPT_BOOL(0, "shallow", &opts.shallow,
-			 N_("if the commit-graph is split, only verify the tip file")),
-		OPT_BOOL(0, "progress", &opts.progress,
-			 N_("force progress reporting")),
+		OPT_BIT(0, "shallow", &flags,
+			N_("if the commit-graph is split, only verify the tip file"),
+			COMMIT_GRAPH_VERIFY_SHALLOW),
+		OPT_BIT(0, "progress", &flags,
+			N_("force progress reporting"), COMMIT_GRAPH_VERIFY_PROGRESS),
 		COMMIT_GRAPH_COMMON_OPTIONS(&opts),
 		OPT_END(),
 	};
@@ -94,10 +88,6 @@ static int cmd_commit_graph_verify(int argc,
 
 	if (!opts.obj_dir)
 		opts.obj_dir = repo_get_object_directory(repo);
-	if (opts.shallow)
-		flags |= COMMIT_GRAPH_VERIFY_SHALLOW;
-	if (opts.progress)
-		flags |= COMMIT_GRAPH_VERIFY_PROGRESS;
 
 	dir = odb_source_files_find_dir(repo->objects, opts.obj_dir);
 	if (!dir)
@@ -152,7 +142,7 @@ static int write_option_parse_split(const struct option *opt, const char *arg,
 
 	BUG_ON_OPT_NEG(unset);
 
-	data->opts->split = 1;
+	data->write_opts->flags |= COMMIT_GRAPH_WRITE_SPLIT;
 	if (!arg)
 		return 0;
 
@@ -231,7 +221,6 @@ static int cmd_commit_graph_write(int argc,
 {
 	struct commit_graph_write_options write_opts = COMMIT_GRAPH_WRITE_OPTIONS_INIT;
 	struct opts_commit_graph opts = {
-		.progress = isatty(2),
 		.enable_changed_paths = -1,
 	};
 	struct commit_graph_write_config_data config_data = {
@@ -245,14 +234,15 @@ static int cmd_commit_graph_write(int argc,
 	int result = 0;
 	struct progress *progress = NULL;
 	struct option options[] = {
-		OPT_BOOL(0, "reachable", &opts.reachable,
-			N_("start walk at all refs")),
+		OPT_BIT(0, "reachable", &write_opts.flags,
+			N_("start walk at all refs"), COMMIT_GRAPH_WRITE_REACHABLE),
 		OPT_BOOL(0, "stdin-packs", &opts.stdin_packs,
 			N_("scan pack-indexes listed by stdin for commits")),
 		OPT_BOOL(0, "stdin-commits", &opts.stdin_commits,
 			N_("start walk at commits listed by stdin")),
-		OPT_BOOL(0, "append", &opts.append,
-			N_("include all commits already in the commit-graph file")),
+		OPT_BIT(0, "append", &write_opts.flags,
+			N_("include all commits already in the commit-graph file"),
+			COMMIT_GRAPH_WRITE_APPEND),
 		OPT_BOOL(0, "changed-paths", &opts.enable_changed_paths,
 			N_("enable computation for changed paths")),
 		OPT_CALLBACK_F(0, "split", &config_data, NULL,
@@ -268,14 +258,16 @@ static int cmd_commit_graph_write(int argc,
 		OPT_CALLBACK_F(0, "max-new-filters", &write_opts.max_new_filters,
 			NULL, N_("maximum number of changed-path Bloom filters to compute"),
 			0, write_option_max_new_filters),
-		OPT_BOOL(0, "progress", &opts.progress,
-			 N_("force progress reporting")),
+		OPT_BIT(0, "progress", &write_opts.flags,
+			N_("force progress reporting"), COMMIT_GRAPH_WRITE_PROGRESS),
 		COMMIT_GRAPH_COMMON_OPTIONS(&opts),
 		OPT_END(),
 	};
 
 	trace2_cmd_mode("write");
 
+	write_opts.flags |= isatty(2) ? COMMIT_GRAPH_WRITE_PROGRESS : 0;
+
 	repo_config(repo, git_commit_graph_write_config, &config_data);
 
 	argc = parse_options(argc, argv, prefix,
@@ -284,23 +276,15 @@ static int cmd_commit_graph_write(int argc,
 	if (argc)
 		usage_with_options(builtin_commit_graph_write_usage, options);
 
-	if (opts.reachable + opts.stdin_packs + opts.stdin_commits > 1)
+	if (!!(write_opts.flags & COMMIT_GRAPH_WRITE_REACHABLE) + opts.stdin_packs + opts.stdin_commits > 1)
 		die(_("use at most one of --reachable, --stdin-commits, or --stdin-packs"));
 	if (!opts.obj_dir)
 		opts.obj_dir = repo_get_object_directory(repo);
-	if (opts.append)
-		write_opts.flags |= COMMIT_GRAPH_WRITE_APPEND;
-	if (opts.split)
-		write_opts.flags |= COMMIT_GRAPH_WRITE_SPLIT;
-	if (opts.progress)
-		write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
 	if (!opts.enable_changed_paths)
 		write_opts.flags |= COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS;
 	if (opts.enable_changed_paths == 1 ||
 	    git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
 		write_opts.flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
-	if (opts.reachable)
-		write_opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
 
 	dir = odb_source_files_find_dir(repo->objects, opts.obj_dir);
 	if (!dir)
@@ -323,7 +307,7 @@ static int cmd_commit_graph_write(int argc,
 		strbuf_release(&packname);
 	} else if (opts.stdin_commits) {
 		oidset_init(&commits, 0);
-		if (opts.progress)
+		if (write_opts.flags & COMMIT_GRAPH_WRITE_PROGRESS)
 			progress = start_delayed_progress(
 				repo,
 				_("Collecting commits from input"), 0);

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 09/10] odb/source-files: verify commit-graphs as part of fsck

[Patrick Steinhardt]

git-fsck(1) verifies commit-graphs by downcasting the object database's
source to the "files" backend and spawning `git commit-graph verify
--object-dir` for each of its object directories. But starting with
14bf3a43f8b (Merge branch 'ps/odb-pluggable-fsck', 2026-09-15) we have
adapted git-fsck(1) so that the verification of the object database is
pluggable.

In a subsequent patch series we're also about to make commit graphs
themselves pluggable so that a backend can choose where exactly a commit
graph is stored. As part of that, we'll also have to make verification
of commit graphs pluggable in the same way.

Move the verification into `odb_source_files_fsck()`, same as we did for
multi-pack indices, bitmaps and all the other auxiliary data structures.
Same as when we adapted consistency checks for those, there are a couple
of consequences here:

  - Commit graphs stored in alternates are only verified when
    `ODB_FSCK_FULL` is set, which is the default for git-fsck(1).

  - Commit-graphs are not verified with `--connectivity-only` anymore,
    as that mode skips the object database checks altogether.

  - Failures are reported via `ERROR_OBJECT` instead of a dedicated
    exit code bit, same as we have dropped `ERROR_MIDX` et al. Those
    error codes were never documented or exercised anywhere.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fsck.c     | 20 --------------------
 odb/source-files.c | 25 +++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 710780e35a8..c4558efcf18 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -46,7 +46,6 @@ static timestamp_t now;
 #define ERROR_OBJECT 01
 #define ERROR_REACHABLE 02
 #define ERROR_REFS 010
-#define ERROR_COMMIT_GRAPH 020
 
 static const char *describe_object(const struct object_id *oid)
 {
@@ -913,7 +912,6 @@ int cmd_fsck(int argc,
 		fsck_enable_object_names(&fsck_walk_options);
 
 	repo_config(repo, git_fsck_config, &fsck_obj_options);
-	prepare_repo_settings(repo);
 
 	if (check_references)
 		fsck_refs(repo);
@@ -984,24 +982,6 @@ int cmd_fsck(int argc,
 
 	check_connectivity(repo);
 
-	if (repo->settings.core_commit_graph) {
-		struct odb_source_files *files = odb_source_files_downcast(repo->objects->source);
-		struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
-
-		for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
-			child_process_init(&commit_graph_verify);
-			commit_graph_verify.git_cmd = 1;
-			strvec_pushl(&commit_graph_verify.args, "commit-graph",
-				     "verify", "--object-dir", dir->abspath, NULL);
-			if (show_progress)
-				strvec_push(&commit_graph_verify.args, "--progress");
-			else
-				strvec_push(&commit_graph_verify.args, "--no-progress");
-			if (run_command(&commit_graph_verify))
-				errors_found |= ERROR_COMMIT_GRAPH;
-		}
-	}
-
 	free_snapshot_refs(&snap);
 	return errors_found;
 }
diff --git a/odb/source-files.c b/odb/source-files.c
index 5b4277fbfd3..224b47e38b1 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -1215,6 +1215,30 @@ static int odb_source_files_generate_pack(struct odb_source *source UNUSED,
 	return 0;
 }
 
+static int fsck_commit_graph(struct odb_source_files *files,
+			     struct odb_files_dir *dir,
+			     struct odb_fsck_options *opts)
+{
+	struct repository *repo = files->base.odb->repo;
+	struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
+
+	prepare_repo_settings(repo);
+	if (!repo->settings.core_commit_graph)
+		return 0;
+
+	commit_graph_verify.git_cmd = 1;
+	strvec_pushl(&commit_graph_verify.args, "commit-graph", "verify",
+		     "--object-dir", dir->abspath, NULL);
+	if (opts->flags & ODB_FSCK_PROGRESS)
+		strvec_push(&commit_graph_verify.args, "--progress");
+	else
+		strvec_push(&commit_graph_verify.args, "--no-progress");
+	if (run_command(&commit_graph_verify))
+		return -1;
+
+	return 0;
+}
+
 static int odb_source_files_fsck(struct odb_source *source,
 				 struct odb_fsck_options *opts)
 {
@@ -1227,6 +1251,7 @@ static int odb_source_files_fsck(struct odb_source *source,
 
 		ret |= odb_source_fsck(&dir->loose->base, opts);
 		ret |= odb_source_fsck(&dir->packed->base, opts);
+		ret |= fsck_commit_graph(files, dir, opts);
 	}
 
 	return ret;

-- 
2.56.0.500.g5d06dc02cd.dirty

[PATCH 10/10] commit-graph: modernize function names

[Patrick Steinhardt]

Modernize a couple of function names of the commit-graph subsystem so
that they follow our modern `<subsystem>_<verb>` naming schema.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/gitformat-chunk.adoc |  2 +-
 builtin/commit-graph.c             |  6 +++---
 builtin/fetch.c                    |  2 +-
 builtin/gc.c                       |  2 +-
 commit-graph.c                     | 20 ++++++++++----------
 commit-graph.h                     | 14 +++++++-------
 odb.c                              |  2 +-
 oss-fuzz/fuzz-commit-graph.c       |  4 ++--
 t/helper/test-read-graph.c         |  2 +-
 9 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/gitformat-chunk.adoc b/Documentation/gitformat-chunk.adoc
index 3315df6201d..061803932b4 100644
--- a/Documentation/gitformat-chunk.adoc
+++ b/Documentation/gitformat-chunk.adoc
@@ -118,7 +118,7 @@ Examples
 These file formats use the chunk-format API, and can be used as examples
 for future formats:
 
-* *commit-graph:* see `write_commit_graph_file()` and `parse_commit_graph()`
+* *commit-graph:* see `write_commit_graph_file()` and `commit_graph_parse()`
   in `commit-graph.c` for how the chunk-format API is used to write and
   parse the commit-graph file format documented in
   the commit-graph file format in linkgit:gitformat-commit-graph[5].
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 218c30ed3ed..8194a528406 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -119,8 +119,8 @@ static int cmd_commit_graph_verify(int argc,
 	if (!graph)
 		return 1;
 
-	ret = verify_commit_graph(graph, flags);
-	free_commit_graph(graph);
+	ret = commit_graph_verify(graph, flags);
+	commit_graph_free(graph);
 
 	if (incomplete_chain) {
 		error("one or more commit-graph chain files could not be loaded");
@@ -324,7 +324,7 @@ static int cmd_commit_graph_write(int argc,
 		stop_progress(&progress);
 	}
 
-	if (write_commit_graph(repo, dir->abspath, &write_opts))
+	if (commit_graph_write(repo, dir->abspath, &write_opts))
 		result = 1;
 
 cleanup:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7278cee8ca7..cf8cafc4520 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2949,7 +2949,7 @@ int cmd_fetch(int argc,
 		if (graph_write_mode != GRAPH_WRITE_SKIP) {
 			trace2_region_enter("fetch", "write-commit-graph",
 					    the_repository);
-			write_commit_graph(the_repository,
+			commit_graph_write(the_repository,
 					   the_repository->objects->source->path,
 					   &write_opts);
 			trace2_region_leave("fetch", "write-commit-graph",
diff --git a/builtin/gc.c b/builtin/gc.c
index 7bf88861df0..68cb04a672d 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -741,7 +741,7 @@ int cmd_gc(int argc,
 		if (!opts.quiet && !daemonized)
 			write_opts.flags |= COMMIT_GRAPH_WRITE_PROGRESS;
 
-		write_commit_graph(the_repository, the_repository->objects->source->path,
+		commit_graph_write(the_repository, the_repository->objects->source->path,
 				   &write_opts);
 	}
 
diff --git a/commit-graph.c b/commit-graph.c
index c82776b0667..da65cd3deb9 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -41,7 +41,7 @@ void git_test_write_commit_graph_or_die(struct repository *repo)
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
 		opts.flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
 
-	if (write_commit_graph(repo, repo->objects->source->path, &opts))
+	if (commit_graph_write(repo, repo->objects->source->path, &opts))
 		die("failed to write commit-graph under GIT_TEST_COMMIT_GRAPH");
 }
 
@@ -275,7 +275,7 @@ struct commit_graph *load_commit_graph_one_fd_st(struct repository *repo,
 	graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	close(fd);
 
-	ret = parse_commit_graph(repo, graph_map, graph_size);
+	ret = commit_graph_parse(repo, graph_map, graph_size);
 	if (ret)
 		ret->dir = absolute_pathdup(dir);
 	else
@@ -375,7 +375,7 @@ static int graph_read_bloom_data(const unsigned char *chunk_start,
 	return 0;
 }
 
-struct commit_graph *parse_commit_graph(struct repository *r,
+struct commit_graph *commit_graph_parse(struct repository *r,
 					void *graph_map, size_t graph_size)
 {
 	const unsigned char *data;
@@ -686,7 +686,7 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb,
 					graph_chain = g;
 					valid = 1;
 				} else {
-					free_commit_graph(g);
+					commit_graph_free(g);
 				}
 
 				break;
@@ -830,14 +830,14 @@ struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)
 	return NULL;
 }
 
-void close_commit_graph(struct object_database *o)
+void commit_graph_close(struct object_database *o)
 {
 	if (!o->commit_graph)
 		return;
 
 	clear_commit_graph_data_slab(&commit_graph_data_slab);
 	deinit_bloom_filters();
-	free_commit_graph(o->commit_graph);
+	commit_graph_free(o->commit_graph);
 	o->commit_graph = NULL;
 }
 
@@ -2214,7 +2214,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 		ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 2] = new_base_hash;
 	}
 
-	close_commit_graph(ctx->r->objects);
+	commit_graph_close(ctx->r->objects);
 	finalize_hashfile(f, file_hash, FSYNC_COMPONENT_COMMIT_GRAPH,
 			  CSUM_HASH_IN_STREAM | CSUM_FSYNC);
 	free_chunkfile(cf);
@@ -2542,7 +2542,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 	strbuf_release(&path);
 }
 
-int write_commit_graph(struct repository *r,
+int commit_graph_write(struct repository *r,
 		       const char *dir,
 		       const struct commit_graph_write_options *opts)
 {
@@ -2904,7 +2904,7 @@ static int verify_one_commit_graph(struct commit_graph *g,
 	return verify_commit_graph_error;
 }
 
-int verify_commit_graph(struct commit_graph *g,
+int commit_graph_verify(struct commit_graph *g,
 			enum commit_graph_verify_flags flags)
 {
 	struct progress *progress = NULL;
@@ -2937,7 +2937,7 @@ int verify_commit_graph(struct commit_graph *g,
 	return local_error;
 }
 
-void free_commit_graph(struct commit_graph *g)
+void commit_graph_free(struct commit_graph *g)
 {
 	while (g) {
 		struct commit_graph *next = g->base_graph;
diff --git a/commit-graph.h b/commit-graph.h
index 80d59bd23bd..2156499a688 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -135,9 +135,9 @@ struct repo_settings;
 
 /*
  * Callers should initialize the repo_settings with prepare_repo_settings()
- * prior to calling parse_commit_graph().
+ * prior to calling commit_graph_parse().
  */
-struct commit_graph *parse_commit_graph(struct repository *r,
+struct commit_graph *commit_graph_parse(struct repository *r,
 					void *graph_map, size_t graph_size);
 
 /*
@@ -186,7 +186,7 @@ struct commit_graph_write_options {
 }
 
 /*
- * The write_commit_graph* methods return zero on success
+ * The commit_graph_write* methods return zero on success
  * and a negative value on failure. Note that if the repository
  * is not compatible with the commit-graph feature, then the
  * methods will return 0 without writing a commit-graph.
@@ -195,7 +195,7 @@ struct commit_graph_write_options {
  * canonicalized internally so that it compares equal to the directory of
  * graphs that have already been loaded.
  */
-int write_commit_graph(struct repository *r,
+int commit_graph_write(struct repository *r,
 		       const char *dir,
 		       const struct commit_graph_write_options *opts);
 
@@ -204,11 +204,11 @@ enum commit_graph_verify_flags {
 	COMMIT_GRAPH_VERIFY_PROGRESS = (1 << 1),
 };
 
-int verify_commit_graph(struct commit_graph *g,
+int commit_graph_verify(struct commit_graph *g,
 			enum commit_graph_verify_flags flags);
 
-void close_commit_graph(struct object_database *);
-void free_commit_graph(struct commit_graph *);
+void commit_graph_close(struct object_database *);
+void commit_graph_free(struct commit_graph *);
 
 /*
  * Disable further use of the commit graph in this process when parsing a
diff --git a/odb.c b/odb.c
index 2c93a9490c6..b89457d0c91 100644
--- a/odb.c
+++ b/odb.c
@@ -739,7 +739,7 @@ struct object_database *odb_new(struct repository *repo,
 void odb_close(struct object_database *o)
 {
 	odb_source_close(o->source);
-	close_commit_graph(o);
+	commit_graph_close(o);
 }
 
 static void odb_free_sources(struct object_database *o)
diff --git a/oss-fuzz/fuzz-commit-graph.c b/oss-fuzz/fuzz-commit-graph.c
index 59bbb849d1e..e941cd9fc56 100644
--- a/oss-fuzz/fuzz-commit-graph.c
+++ b/oss-fuzz/fuzz-commit-graph.c
@@ -23,9 +23,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 	the_repository->settings.initialized = 1;
 	the_repository->settings.commit_graph_generation_version = 2;
 	the_repository->settings.commit_graph_changed_paths_version = 1;
-	g = parse_commit_graph(the_repository, (void *)data, size);
+	g = commit_graph_parse(the_repository, (void *)data, size);
 	repo_clear(the_repository);
-	free_commit_graph(g);
+	commit_graph_free(g);
 
 	return 0;
 }
diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c
index aceac4f6995..9642e55b8c0 100644
--- a/t/helper/test-read-graph.c
+++ b/t/helper/test-read-graph.c
@@ -119,6 +119,6 @@ int cmd__read_graph(int argc, const char **argv)
 	}
 
 done:
-	free_commit_graph(graph);
+	commit_graph_free(graph);
 	return ret;
 }

-- 
2.56.0.500.g5d06dc02cd.dirty

Re: [PATCH 05/10] commit-graph: merge the two commit graph writers

[Derrick Stolee]

On 11/4/2026 10:57 AM, Patrick Steinhardt wrote:
> We have two entry points for writing commit graphs:
> 
>   - `write_commit_graph()` either writes a commit graph for all commits
>     in the object database, or for the commits specified via its options
>     struct.
> 
>   - `write_commit_graph_reachable()` writes a commit graph for all
>     reachable commits.
> 
> It's somewhat pointless though to have a separate entry point for
> writing a commit graph for reachable commits, as this use case can be
> trivially satisfied via a flag.
> 
> Introduce that flag and drop the separate entry point in favor of
> `write_commit_graph()`. This ensures we have a single entry point for
> writing commit graphs, only, which will make it easier to eventually
> introduce pluggable commit graphs via the object database.
The previous four patches were immediately obvious to be correct. This
one takes a little bit more work, but is a valuable simplification
that will lead to better code reuse in the future.

Thanks,
-Stolee

Re: [PATCH 06/10] commit-graph: move remaining write parameters into options

[Derrick Stolee]

On 11/4/2026 10:57 AM, Patrick Steinhardt wrote:
> Move the remaining optional parameters that can be passed to
> `write_commit_graph()` into its options structure to simplify the
> function's calling convention.

> -	if (write_commit_graph(repo, dir->abspath,
> -			       opts.stdin_packs ? &pack_indexes : NULL,
> -			       opts.stdin_commits ? &commits : NULL,
> -			       &write_opts))
> +	if (write_commit_graph(repo, dir->abspath, &write_opts))
>  		result = 1;

So much cleaner! (Even better in other hunks.)

Thanks,
-Stolee

Re: [PATCH 00/10] commit-graph: several refactorings for writing commit graphs

[Derrick Stolee]

On 11/4/2026 10:57 AM, Patrick Steinhardt wrote:
> Hi,
> 
> this patch series includes several small cleanups for the interfaces for
> writing commit graphs. Most importantly, the series refactors options so
> that they all get passed via `struct commit_graph_write_options`. These
> cleanups are split out of a bigger change to make reading and writing
> commit-graphs pluggable via the object database bakcend.
These cleanups look great. They are carefully constructed to be as
simple as possible at each step. It's good to bring some of these
conventions into a more modern style and to simplify things in the
process.

LGTM.

Thanks,
-Stolee

Re: [PATCH 06/10] commit-graph: move remaining write parameters into options

[Kristofer Karlsson]

On Wed, 4 Nov 2026 at 17:15, Patrick Steinhardt <ps@pks.im> wrote:
>
>                 if (graph_write_mode != GRAPH_WRITE_TIPS)
>                         write_opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
> +               else
> +                       write_opts.commits = &updated_tips;

Ah, this touches on my previous patch cef2d8c4cb
(fetch: write commit-graph using updated refs only, 2026-10-07).

My only nit would be that the if/else also covers the skip case,
so one of the branches always fires for it.  A switch would make
each mode set exactly what it needs:

  switch (graph_write_mode) {
  case GRAPH_WRITE_SKIP:
      break;
  case GRAPH_WRITE_REACHABLE:
      write_opts.flags |= COMMIT_GRAPH_WRITE_REACHABLE;
      break;
  case GRAPH_WRITE_TIPS:
      write_opts.commits = &updated_tips;
      break;
  }

It doesn't matter for correctness since the skip case never calls
the writer, but it looks easier to reason about to me.

But that's just a preference, I don't feel strongly about it.

>
>                 if (graph_write_mode != GRAPH_WRITE_SKIP) {
>                         trace2_region_enter("fetch", "write-commit-graph",
>                                             the_repository);
> -                       if (graph_write_mode == GRAPH_WRITE_TIPS)
> -                               write_commit_graph(
> -                                       the_repository,
> -                                       the_repository->objects->source->path,
> -                                       NULL, &updated_tips, &write_opts);
> -                       else
> -                               write_commit_graph(
> -                                       the_repository,
> -                                       the_repository->objects->source->path,
> -                                       NULL, NULL, &write_opts);
> +                       write_commit_graph(the_repository,
> +                                          the_repository->objects->source->path,
> +                                          &write_opts);
>                         trace2_region_leave("fetch", "write-commit-graph",
>                                             the_repository);
>                 }

I really like this unification of the call sites and the patch
looks great overall!

Thanks,
Kristofer

Re: [PATCH 04/10] commit-graph: drop redundant flags in write context

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> In the preceding commit we have moved the flags that control how we
> write a commit graph into its options structure. As a consequence we
> have those flags implicitly available in the write context, too, as it
> already contains a pointer to the options struct. This makes a couple of
> individual flags that we track in the write context redundant as they
> are a simple alias of the flags.
>
> Drop these redundant flags.
> ...
>  	struct write_commit_graph_context ctx = {
>  		.r = r,
> -		.append = opts->flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0,
> -		.report_progress = opts->flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0,
> -		.split = opts->flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0,

OK, so these are the most important lines in this patch; everything
else is there as a fallout of this change, and as a result, the
remaining code has become much easier to follow.

Nicely done.