On Mon, Aug 04, 2025 at 06:10:21PM -0400, Taylor Blau wrote: > On Mon, Aug 04, 2025 at 10:17:23AM +0200, Patrick Steinhardt wrote: > > Stop using `the_hash_algo` as it implicitly relies on `the_repository`. > > Instead, we either use the hash algo provided via the context or, if > > there is no such hash algo, we use `the_repository` explicitly. Such > > uses will be removed in subsequent commits. > > Seems reasonable, and the implementation looks straightforward to me, > however I wonder... > > > @@ -129,6 +130,7 @@ struct repo_settings; > > * prior to calling parse_commit_graph(). > > */ > > struct commit_graph *parse_commit_graph(struct repo_settings *s, > > + const struct git_hash_algo *hash_algo, > > void *graph_map, size_t graph_size); > > ...does it make more sense to take a 'struct repository *' here instead > of passing both its settings and hash_algo separately? Is there a > scenario where we would want to parse a commit graph with a (settings, > hash_algo) pair that does not match that of any single repository? Fair. That'd also allow us to move the call of `prepare_repo_settings()` into this function. There's one catch though: in "oss-fuzz/fuzz-commit-graph.c" we manually stub out both the repository's hash function and its settings. But we can appease it by also setting `the_repository->settings.initialized`, which ensures that we won't try to populate the settings anymore. Will amend. Patrick
