Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()

[Eric Sunshine <sunshine@sunshineco.com>]

On Sun, Dec 19, 2021 at 6:54 PM <rsbecker@nexbridge.com> wrote:
> On December 19, 2021 6:30 PM, Eric Sunshine wrote:
> > `is_bare_repository_cfg` is not exactly a boolean; it's a tristate, with -1
> > meaning "not yet determined". I didn't, at the time, closely follow the
> > discussion[1] of the particular bit of code you're questioning, but the `== 1`
> > was mentioned at least a couple times, once in review by Junio[2], and then
> > in the extra patch commentary by "jtan" when he submitted v2[3]. Anyhow,
> > if I'm following the original discussion correctly, then the usage, `== 1` (or the
> > equivalent `> 0`) is probably correct, and that treating it as a simple boolean
> > (where
> > -1 is true, too) would be undesirable. (Of course, I haven't traced through the
> > init code at all, so I don't even know if it can ever be
> > -1 at this point.) Five existing consumers of this global variable use `== 1`, and
> > only two use `> 0`, so this usage is at least reasonably consistent with other
> > parts of the project.
>
> Thanks for the clarification. It helps to understand the code. Could the variable type be changed to a new typedef like ConfigTriState instead of int to be clear about its semantics? Or perhaps an enum with -1, 0, 1 declared explicitly?

I'm not sure how much value an enum would add in this particular case
since this is a quite common idiom in this codebase. It might be
sufficient merely to add a comment to its declaration in cache.h
explaining its possible values.

What could be even more helpful is some documentation about the entire
setup sequence and how all the different "is bare" and "do we have a
worktree" computations interact since the general setup logic is quite
involved, and there are likely a number of pitfalls which are not
obvious by merely glancing at the code, but which people know about
who have dealt extensively with this code (such as Duy and probably
Peff).