Re: [PATCH 05/17] odb: get rid of `the_repository` in `find_odb()`

From: Patrick Steinhardt <ps@pks.im>
Date: May 9, 2025, 11:25 UTC
Message-ID: <aB3mHVX0A_ADYLKM@pks.im>
In-Reply-To: <d731678d-bda2-474d-81e8-39da85bd9038@gmail.com>

On Tue, May 06, 2025 at 09:10:06PM -0400, Derrick Stolee wrote:

Show 15 quoted lines

> On 5/6/25 7:09 AM, Patrick Steinhardt wrote:
> 
> > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
> > index 0a184d39720..28550d3bcc9 100644
> > --- a/builtin/commit-graph.c
> > +++ b/builtin/commit-graph.c
> > @@ -66,7 +66,7 @@ static int graph_verify(int argc, const char **argv, const char *prefix,
> >   			struct repository *repo UNUSED)
> >   {
> >   	struct commit_graph *graph = NULL;
> > -	struct odb_backend *odb = NULL;
> > +	struct odb_backend *backend = NULL;
> 
> nit: this looks like a misplaced variable rename that should be in
> an earlier patch.

Fixed.

Show 8 quoted lines

> > @@ -221,7 +221,7 @@ static int graph_write(int argc, const char **argv, const char *prefix,
> >   	struct string_list pack_indexes = STRING_LIST_INIT_DUP;
> >   	struct strbuf buf = STRBUF_INIT;
> >   	struct oidset commits = OIDSET_INIT;
> > -	struct odb_backend *odb = NULL;
> > +	struct odb_backend *backend = NULL;
> 
> Here's another of those delayed renames.

Fixed.

Show 23 quoted lines

> > -struct odb_backend *find_odb(struct repository *r, const char *obj_dir)
> > +struct odb_backend *odb_find_backend(struct object_database *odb, const char *obj_dir)
> 
> I was looking at this implementation and wondering why it wasn't
> renamed earlier, but upon closer inspection I agree that the
> rename is worthwhile _and_ the method shouldn't have been
> changed until now.
> 
> >   {
> > -	struct odb_backend *odb;
> > +	struct odb_backend *backend;
> >   	char *obj_dir_real = real_pathdup(obj_dir, 1);
> >   	struct strbuf odb_path_real = STRBUF_INIT;
> > -	prepare_alt_odb(r);
> > -	for (odb = r->objects->backends; odb; odb = odb->next) {
> > -		strbuf_realpath(&odb_path_real, odb->path, 1);
> > +	prepare_alt_odb(odb->repo);
> This does make me wonder if we should be able to prepare the
> alternates for a given odb without using an owning repo. Yes,
> we'd need to assign the repo parent when creating new odbs, but
> the info/alternates file is in the object dir.
> 
> Perhaps this is waiting for me in a later patch...

Things are still somewhat mixed at the end of this patch series, but we do reduce the dependency over time. Eventually, the goal is that we only need a repository to read its configuration, which may even happen once and then never again for the object database so that we can stop tracking the parent repository.

Patrick