Re: [PATCH v2 3/5] setup: defer object database creation

Patrick Steinhardt <[email protected]> Wed, 5 Aug 2026 09:27:59 +0200
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
On Tue, Aug 04, 2026 at 08:48:42PM +0200, Toon Claes wrote:
> Patrick Steinhardt <[email protected]> writes:
> 
> > In a subsequent commit we'll make the creation of the on-disk data
> > structures of an object database pluggable. This will lead to an
> > in-between state where we have already configured the repository's
> > object database, but it's not usable yet until we eventually call
> > `create_object_directory()`.
> >
> > Defer the object database creation so that we handle both steps in the
> > same function.
> >
> > Signed-off-by: Patrick Steinhardt <[email protected]>
> > ---
> >  setup.c | 35 +++++++++++++++++++++++++++--------
> >  setup.h |  9 +++++++++
> >  2 files changed, 36 insertions(+), 8 deletions(-)
> >
> > diff --git a/setup.c b/setup.c
> > index 825572f5f1..a7b1b9eaef 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -1760,6 +1760,13 @@ enum discovery_result discover_git_directory_reason(struct strbuf *commondir,
> >  	return result;
> >  }
> >  
> > +static void get_object_directories(char **object_directory,
> > +				   char **alternate_object_directories)
> > +{
> > +	*object_directory = xstrdup_or_null(getenv(DB_ENVIRONMENT));
> > +	*alternate_object_directories = xstrdup_or_null(getenv(ALTERNATE_DB_ENVIRONMENT));
> > +}
> 
> Would it make sense to wrap these in a APPLY_REPOSITORY_FORMAT_HONOR_ENV
> guard?
> 
> I mean, below we call this function *only* when flags has that bit set.
> But the return values of that function are used at the bottom of
> apply_repository_format(), that's a bit awkard.
> 
> So can I suggest the following patch instead? That would remove the
> weird double pointer passing around, which feels a bit unneeded.

You're right, this is somewhat awkward. I have a different proposal
though: instead of creating a separate function, we can move handling of
environment variables into `odb_new()` itself. This also paves the way
for moving handling of these environment variables into the backend,
which is something I want to do soonish [1].

Patrick

[1]: https://lore.kernel.org/git/[email protected]/