Re: [PATCH 7/9] commit-graph: stop using `the_hash_algo`

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 6, 2025, 06:53 UTC
Message-ID: <aJL7z2kHXCW1exWH@pks.im>
In-Reply-To: <aJEvzR4wuve/9ltU@nand.local>

On Mon, Aug 04, 2025 at 06:10:21PM -0400, Taylor Blau wrote:

Show 20 quoted lines

> 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