From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 21 Nov 2025 08:12:48 GMT
Subject: Re: [PATCH 12/13] odb: handle changing a repository's commondir
Message-ID: <aSAfAJscjYHAg3Mc@pks.im>
In-Reply-To: <xmqq34687414.fsf@gitster.g>

On Thu, Nov 20, 2025 at 02:06:15PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > -static void set_git_dir_1(const char *path)
> > +static void setup_git_env_internal(const char *git_dir,
> > +				   bool skip_initializing_odb)
> > +{
> 
> Hopefully we won't gain too many callers of this function, and ...
> 
> > +static void set_git_dir_1(const char *path, bool skip_initializing_odb)
> >  {
> 
> ... this function, as ...
> 
> > -	set_git_dir_1(path);
> > +	set_git_dir_1(path, true);
> > ...
> > -	set_git_dir_1(path);
> > +	set_git_dir_1(path, false);
> 
> ... it is almost impossible to tell from the call site which one is
> for initializing the ODB (hint: "true" does initialize the ODB, oh,
> no it is the other way around, or is it correct?  now everybody is
> confused).
> 
> We could do "enum { INIT_DB, NO_INIT_DB }" instead of bool and the
> calling sites would become self-describing, but as long as we won't
> have too many calling sites, the current code should be OK.

Quite frankly, the whole "setup.c" file could use a makeover. I would
claim it's almost impossible to understand the different flows we have
here, and the setup of a repository (or `the_reposiory`) is awfully
complex and non-obvious.

Some ICs in my team might tackle this in the Git 2.53 release cycle,
hopefully.

Patrick