Subject: Re: [PATCH 2/2] setup: make bareRepository=explicit work in GIT_DIR of a secondary worktree

From: Junio C Hamano <[email-masked]>
Date: Fri, 08 Mar 2024 15:32:11 -0800

Kyle Lippincott <[email-masked]> writes:

>>     $ cat ../seconary/.git
>
> Nit: typo, should be `secondary` (missing the `d`)

Good eyes.  Thanks.

>> +       /*
>> +        * We should be a subdirectory of /.git/worktrees inside
>> +        * the $GIT_DIR of the primary worktree.
>> +        *
>> +        * NEEDSWORK: some folks create secondary worktrees out of a
>> +        * bare repository; they don't count ;-), at least not yet.
>> +        */
>> +       if (!strstr(path, "/.git/worktrees/"))
>
> Do we need to be concerned about path separators being different on
> Windows? Or have they already been normalized here?

I am not certain offhand, but if they need to tolerate different
separators, they can send in patches.

>> +               goto out;
>> +
>> +       /*
>> +        * Does gitdir that points at the ".git" file at the root of
>> +        * the secondary worktree roundtrip here?
>> +        */
>
> What loss of security do we have if we don't have as stringent of a
> check? i.e. if we just did `return !!strstr(path, "/.git/worktrees/)`?

No loss of security.

These "keep result the status we want to return if we want to return
immediately here, and always jump to the out label instead of
returning" patterns is mere a disciplined way to make it easier to
write code that does not leak.  The very first one may not have to
do the "goto out" and instead immediately return, but by writing
this way, I do not need to be always looking out to shoot down
patches that adds new check and/or allocations before this
condition and early "out".

> Or maybe we even combine the existing ends_with(.git) check with this

At the mechanical level, perhaps, but I'd want logically separate
things treated as distinct cases.  One is about being inside
$GIT_DIR of the primary worktrees (where more than majority of users
will encounter) and the new one is about being inside $GIT_DIR of
secondaries.