Re: [PATCH v2 5/7] environment: split up concerns of `is_bare_repository_cfg`

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jun 12, 2026 at 09:59:42AM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/setup.c b/setup.c
> > index 71fc6b33da..32f14a8688 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -795,10 +795,22 @@ static int check_repository_format_gently(const char *gitdir,
> >  		has_common = 0;
> >  	}
> >  
> > -	if (!has_common) {
> > -		if (candidate->is_bare != -1)
> > -			is_bare_repository_cfg = candidate->is_bare;
> > -	} else {
> > +	if (startup_info->force_bare_repository) {
> > +		candidate->is_bare = 1;
> > +		FREE_AND_NULL(candidate->work_tree);
> > +	} else if (has_common) {
> > +		/*
> > +		 * When sharing a common dir with another repository (e.g. a
> > +		 * linked worktree), do not let this repository's config
> > +		 * dictate bareness; it is inherited from the main worktree.
> > +		 */
> > +		candidate->is_bare = -1;
> > +
> > +		/*
> > +		 * Furthermore, "core.worktree" is supposed to be ignored when
> > +		 * we have a commondir configured, unless it comes from the
> > +		 * per-worktree configuration.
> > +		 */
> >  		FREE_AND_NULL(candidate->work_tree);
> >  	}
> 
> Looking at the diff, this is really hard to understand. But your
> refactor makes sense and the after state is easier to comprehend.

Yeah, I'm in the same boat. Honestly, I really hope that our test suite
has enough coverage so that this refactoring won't cause any significant
regressions. Which isn't exactly a statement of confidence, but rather a
statement of "oh boy, this is awfully complex and has lots of weird edge
cases".

> > @@ -2571,7 +2584,7 @@ static int create_default_files(struct repository *repo,
> >  		repo_settings_set_shared_repository(repo,
> >  						    init_shared_repository);
> >  
> > -	is_bare_repository_cfg = !work_tree;
> > +	repo->bare_cfg = !work_tree;
> 
> I'm curious, do we still need this? If I'm not mistaken, this function
> is called after check_and_apply_repository_format(), which calls
> apply_repository_format() and sets repo->bare_cfg too (see the diff
> above). Or is it explained by what Justin said[1]?

We can't drop this because after we've applied the format we call
`repo_config()` with `git_default_core_config()`, which will potentially
set the `bare_cfg` variable if "core.bare" is set.

Patrick