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

From: rsbecker@nexbridge.com <rsbecker@nexbridge.com>
Date: Dec 19, 2021, 22:51 UTC
Message-ID: <004001d7f52a$f0367010$d0a35030$@nexbridge.com>
In-Reply-To: <CABceR4aP8BVr10RYS_0PFG7ExRTJ3tB541JGC4DJ7CUsMg9+Jw@mail.gmail.com>

On December 19, 2021 5:23 PM, Sean Allred wrote:

Show 22 quoted lines

> > what about the comparison code where is_bare_repository_cfg is
> > compared with 1 (it is a boolean and sometimes set to -1). This would
> > not generally pass a code review.
> 
> I'm sorry, I'm afraid I don't completely follow.  Wouldn't the most
> straightforward change be to simply follow the documented
> recommendation when we create the worktree config in `git sparse-
> checkout init`?  Specifically,
> 
>     @@ -374,6 +374,9 @@ static int set_config(enum sparse_checkout_mode
> mode)
>                            "core.sparseCheckoutCone",
>                            mode == MODE_CONE_PATTERNS ? "true" : NULL);
> 
>     +    if (is_bare_repository())
>     +      git_config_set_in_file_gently(config_path, "core.bare", "false");
>     +
>          if (mode == MODE_NO_PATTERNS)
>              set_sparse_index_config(the_repository, 0);
> 
> Are we saying the comparison within is_bare_repository() may not be
> appropriate in this case?

I'm suggesting that:

        worktree->is_bare = (is_bare_repository_cfg == 1) ||
                is_bare_repository();

the == 1 comparison should not be done for boolean-style variables. It is an int, but initialized to -1. Unless -1 and 1 mean different things, but that is not really documented.