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

On December 18, 2021 2:01 PM: Sean Allred wrote:

On Sat, Dec 18, 2021 at 11:47 AM [off-list ref] wrote:

quoted

On December 18, 2021 11:47 AM, Sean Allred wrote:

quoted

Hi folks! See the following bug report. Let me know if anything is
unclear -- in all honesty, I neglectfully `git worktree remove
--force`'d the first one I wrote...

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your
issue)

    ~$ git clone --bare https://github.com/git/git.git
    ---clip---

    ~/gitbare$ git config --list --show-origin
    file:config     core.repositoryformatversion=1
    file:config     core.filemode=false
    file:config     core.bare=true
    file:config     core.ignorecase=true
    file:config     remote.origin.url=https://github.com/git/git.git

    ~/gitbare$ git worktree add --no-checkout ../next
    Preparing worktree (checking out 'next')

    ~/gitbare$ git config --list --show-origin
    file:config     core.repositoryformatversion=1
    file:config     core.filemode=false
    file:config     core.bare=true
    file:config     core.ignorecase=true
    file:config     remote.origin.url=https://github.com/git/git.git

    ~/gitbare$ cd ../next/

    ~/next$ git config --list --show-origin
    file:../gitbare/config    core.repositoryformatversion=1
    file:../gitbare/config    core.filemode=false
    file:../gitbare/config    core.bare=true
    file:../gitbare/config    core.ignorecase=true
    file:../gitbare/config    remote.origin.url=https://github.com/git/git.git

    ~/next$ git rev-parse --is-bare-repository
    false

    ~/next$ git config extensions.worktreeconfig true
    ~/next$ git rev-parse --is-bare-repository
    true

    ~/next$ git config --unset extensions.worktreeconfig
    ~/next$ git rev-parse --is-bare-repository
    false

I actually found this situation (and narrowed it to the above) by
trying to perform a sparse-checkout in the worktree.  It appears
sparse-checkout by default uses a worktree-specific config (which does

make sense).

quoted

quoted

What did you expect to happen? (Expected behavior)

    I expected one of the following to happen:

    1. I should have been blocked from creating a worktree from a bare
    repository.

    2. is_bare_repository() shouldn't be fooled by this situation,
    assuming it's valid.

    All things being equal, I would more expect to have been blocked
    from creating a worktree from a bare repository.  Personally, this
    bare repo + worktree setup doesn't not align with my experience so
    far with how bare repos are normally used (i.e., as a convenience
    for centralized remotes that will never be doing a checkout).

What happened instead? (Actual behavior)

    is_bare_repository() is fooled and I'm prevented from performing
    any operation that requires a worktree (like performing a sparse
    checkout).

What's different between what you expected and what actually

happened?

quoted

quoted

    is_bare_repository() is fooled into thinking the worktree is not a
    worktree / I'm able to create a worktree from a bare repo.

Anything else you want to add:

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.34.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28
23:40:43 UTC 2020 x86_64 compiler info: gnuc: 9.3 libc info: glibc:
2.31 $SHELL (typically, interactive shell): /bin/bash


[Enabled Hooks]
not run from a git repository - no hooks to show

My thoughts:

1. I think it is legitimate to create a worktree from a bare repository. The

worktree is using its own working directory/index and does not require
anything from the bare repo.

quoted

2. You ran is_bare_repository from next, which was in your worktree - not

a bare repo, so that answer actually makes sense.

I'm not sure I follow. I did run is_bare_repository from the next-worktree,
but the return value was evidently dependent on the value of
extensions.worktreeconfig. When true, is_bare_repository returned true --
even within the next-worktree. Unless I'm missing something fairly
fundamental here...

I agree that this interpretation *may* be incorrect. Worktreeconfig allows a configuration associated with worktrees but does not mean that there is one. It seems like worktreeconfig=true is causing git to check the worktree-specific configuration, finds out that you are in a worktree but there is in fact no worktree configuration specified, so the main repo is checked, which is bare, so reports true. When worktreeconfig=false, it looks like a quick decision is made that because you are in a worktree, obviously you are not bare (this may be an incorrect interpretation). I can somewhat see both sides of this. Perhaps some clarification on the interpretation is required.

It does seem like is_bare_repository_cfg is false in is_bare_repository, which seems to be wrong in context. However, there is a strange comparison in worktree.c that bothers me - is_bare_repository_cfg == 1 around line 67 which is a numeric comparison to a boolean. I think that may be incorrect. There is a NEEDSWORK comment in the code immediately before that line.

Hoping someone else can chime in on this.

quoted

I'm not sure whether this is an expected use case but it does make sense
to be one. If you typically work in worktrees and write scripts under that
assumption, not having to worry about being in the non-worktree part of a
clone makes sense. So creating a worktree off a bare repo is not a bad thing,
assuming everything else is correct.

The essential part of create a worktree from a bare repo still makes sense to me.