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

[Justin Tobler <jltobler@gmail.com>]

On 26/06/11 11:19AM, Patrick Steinhardt wrote:
> On Wed, Jun 10, 2026 at 05:22:04PM -0500, Justin Tobler wrote:
> > On 26/06/10 08:56AM, Patrick Steinhardt wrote:
> > > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > > index 52aa92fb0a..566732c9f4 100644
> > > --- a/builtin/init-db.c
> > > +++ b/builtin/init-db.c
> > > @@ -81,7 +81,7 @@ int cmd_init_db(int argc,
> > >  	const char *template_dir = NULL;
> > >  	char *template_dir_to_free = NULL;
> > >  	unsigned int flags = 0;
> > > -	int bare = is_bare_repository_cfg;
> > > +	int bare = startup_info->force_bare_repository ? 1 : -1;
> > 
> > Any particular reason to continue mapping `force_bare_repository=false`
> > to -1? Or was this to just minimize changes?
> 
> The `-1` value doesn't mean "false", but it rather means "undecided".
> The effect of this is that "core.bare" will eventually override this.

Ok, so `bare` here is still ultimately a combination of
`force_bare_repository` and `bare_cfg`.

> > > diff --git a/setup.c b/setup.c
> > > index 71fc6b33da..2b690da8ca 100644
> > > --- a/setup.c
> > > +++ b/setup.c
> > > @@ -795,10 +795,16 @@ 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;
> > >  		FREE_AND_NULL(candidate->work_tree);
> > 
> > Previously, when there was a common dir, `candidate->work_tree` was left
> > untouched, but now we are expclicitly setting it. I'm not sure I fully
> > understand this change.
> 
> I cannot blame you. All of this logic is so unbelievably tangled and
> hard to follow.
> 
> In any case, I think you might have missed the fact that we `else if`
> branch is now `has_common` as compared to `!has_common`? To explain the
> different cases a bit:
> 
>   - When we have `force_bare_repository` we are being told that the
>     repository should be treated as bare. So we set `is_bare` and also
>     clear the work tree that may have been discovered.
> 
>   - When we have a commondir we know that we're in a worktree.
>     Previously we did nothing in this case, and that had the implicit
>     effect that `is_bare_repository_cfg` would remain at `-1`. So to
>     match that behaviour we have to also reset the candidate's bareness
>     to `-1`, so that we parse it via the repository's configuration at a
>     later point in time.
> 
>     The other part here is that we also reset `candidate->work_tree`.
>     This is because the expectation is that `$GIT_DIR/common` should
>     override any "core.worktree" settings. Quoting git-config(1):
> 
>         If GIT_COMMON_DIR environment variable is set, core.worktree is
>         ignored and not used for determining the root of working tree.
> 
>   - When we don't have a commondir we previosuly had to also adapt the
>     global `is_bare_repository_cfg` variable. This part is not necessary
>     anymore, so we basically just drop this case altogether.

Thanks for explaination. This make more sense now.

-Justin