{"thread":{"id":"57117","subject":"Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","startedAt":"2021-12-18T16:47:12Z","lastAt":"2021-12-20T21:58:28Z","messageCount":19,"participants":["Sean Allred","rsbecker@nexbridge.com","Eric Sunshine","Derrick Stolee","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"444426","messageId":"CABceR4bZmtC4rCwgxZ1BBYZP69VOUca1f_moJoP989vTUZWu9Q@mail.gmail.com","threadId":"57117","inReplyTo":null,"subject":"Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Sean Allred","fromEmail":"allred.sean@gmail.com","sentAt":"2021-12-18T16:46:58Z","receivedAt":"2021-12-18T16:47:12Z","isPatch":false,"sender":{"key":"allred.sean@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2082195?v=4"},"body":"Hi folks! See the following bug report. Let me know if anything is\nunclear -- in all honesty, I neglectfully `git worktree remove\n--force`'d the first one I wrote...\n\nThank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\n    ~$ git clone --bare https://github.com/git/git.git\n    ---clip---\n\n    ~/gitbare$ git config --list --show-origin\n    file:config     core.repositoryformatversion=1\n    file:config     core.filemode=false\n    file:config     core.bare=true\n    file:config     core.ignorecase=true\n    file:config     remote.origin.url=https://github.com/git/git.git\n\n    ~/gitbare$ git worktree add --no-checkout ../next\n    Preparing worktree (checking out 'next')\n\n    ~/gitbare$ git config --list --show-origin\n    file:config     core.repositoryformatversion=1\n    file:config     core.filemode=false\n    file:config     core.bare=true\n    file:config     core.ignorecase=true\n    file:config     remote.origin.url=https://github.com/git/git.git\n\n    ~/gitbare$ cd ../next/\n\n    ~/next$ git config --list --show-origin\n    file:../gitbare/config    core.repositoryformatversion=1\n    file:../gitbare/config    core.filemode=false\n    file:../gitbare/config    core.bare=true\n    file:../gitbare/config    core.ignorecase=true\n    file:../gitbare/config    remote.origin.url=https://github.com/git/git.git\n\n    ~/next$ git rev-parse --is-bare-repository\n    false\n\n    ~/next$ git config extensions.worktreeconfig true\n    ~/next$ git rev-parse --is-bare-repository\n    true\n\n    ~/next$ git config --unset extensions.worktreeconfig\n    ~/next$ git rev-parse --is-bare-repository\n    false\n\nI actually found this situation (and narrowed it to the above) by\ntrying to perform a sparse-checkout in the worktree.  It appears\nsparse-checkout by default uses a worktree-specific config (which\ndoes make sense).\n\nWhat did you expect to happen? (Expected behavior)\n\n    I expected one of the following to happen:\n\n    1. I should have been blocked from creating a worktree from a bare\n    repository.\n\n    2. is_bare_repository() shouldn't be fooled by this situation,\n    assuming it's valid.\n\n    All things being equal, I would more expect to have been blocked\n    from creating a worktree from a bare repository.  Personally, this\n    bare repo + worktree setup doesn't not align with my experience so\n    far with how bare repos are normally used (i.e., as a convenience\n    for centralized remotes that will never be doing a checkout).\n\nWhat happened instead? (Actual behavior)\n\n    is_bare_repository() is fooled and I'm prevented from performing\n    any operation that requires a worktree (like performing a sparse\n    checkout).\n\nWhat's different between what you expected and what actually happened?\n\n    is_bare_repository() is fooled into thinking the worktree is not a\n    worktree / I'm able to create a worktree from a bare repo.\n\nAnything else you want to add:\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.34.1\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43\nUTC 2020 x86_64\ncompiler info: gnuc: 9.3\nlibc info: glibc: 2.31\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\nnot run from a git repository - no hooks to show\n\nBest,\n-Sean\n"},{"id":"444427","messageId":"014701d7f437$5ae45110$10acf330$@nexbridge.com","threadId":"57117","inReplyTo":"CABceR4bZmtC4rCwgxZ1BBYZP69VOUca1f_moJoP989vTUZWu9Q@mail.gmail.com","subject":"RE: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2021-12-18T17:47:36Z","receivedAt":"2021-12-18T17:47:46Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On December 18, 2021 11:47 AM, Sean Allred wrote:\n> Hi folks! See the following bug report. Let me know if anything is unclear -- in\n> all honesty, I neglectfully `git worktree remove --force`'d the first one I\n> wrote...\n> \n> Thank you for filling out a Git bug report!\n> Please answer the following questions to help us understand your issue.\n> \n> What did you do before the bug happened? (Steps to reproduce your issue)\n> \n>     ~$ git clone --bare https://github.com/git/git.git\n>     ---clip---\n> \n>     ~/gitbare$ git config --list --show-origin\n>     file:config     core.repositoryformatversion=1\n>     file:config     core.filemode=false\n>     file:config     core.bare=true\n>     file:config     core.ignorecase=true\n>     file:config     remote.origin.url=https://github.com/git/git.git\n> \n>     ~/gitbare$ git worktree add --no-checkout ../next\n>     Preparing worktree (checking out 'next')\n> \n>     ~/gitbare$ git config --list --show-origin\n>     file:config     core.repositoryformatversion=1\n>     file:config     core.filemode=false\n>     file:config     core.bare=true\n>     file:config     core.ignorecase=true\n>     file:config     remote.origin.url=https://github.com/git/git.git\n> \n>     ~/gitbare$ cd ../next/\n> \n>     ~/next$ git config --list --show-origin\n>     file:../gitbare/config    core.repositoryformatversion=1\n>     file:../gitbare/config    core.filemode=false\n>     file:../gitbare/config    core.bare=true\n>     file:../gitbare/config    core.ignorecase=true\n>     file:../gitbare/config    remote.origin.url=https://github.com/git/git.git\n> \n>     ~/next$ git rev-parse --is-bare-repository\n>     false\n> \n>     ~/next$ git config extensions.worktreeconfig true\n>     ~/next$ git rev-parse --is-bare-repository\n>     true\n> \n>     ~/next$ git config --unset extensions.worktreeconfig\n>     ~/next$ git rev-parse --is-bare-repository\n>     false\n> \n> I actually found this situation (and narrowed it to the above) by trying to\n> perform a sparse-checkout in the worktree.  It appears sparse-checkout by\n> default uses a worktree-specific config (which does make sense).\n> \n> What did you expect to happen? (Expected behavior)\n> \n>     I expected one of the following to happen:\n> \n>     1. I should have been blocked from creating a worktree from a bare\n>     repository.\n> \n>     2. is_bare_repository() shouldn't be fooled by this situation,\n>     assuming it's valid.\n> \n>     All things being equal, I would more expect to have been blocked\n>     from creating a worktree from a bare repository.  Personally, this\n>     bare repo + worktree setup doesn't not align with my experience so\n>     far with how bare repos are normally used (i.e., as a convenience\n>     for centralized remotes that will never be doing a checkout).\n> \n> What happened instead? (Actual behavior)\n> \n>     is_bare_repository() is fooled and I'm prevented from performing\n>     any operation that requires a worktree (like performing a sparse\n>     checkout).\n> \n> What's different between what you expected and what actually happened?\n> \n>     is_bare_repository() is fooled into thinking the worktree is not a\n>     worktree / I'm able to create a worktree from a bare repo.\n> \n> Anything else you want to add:\n> \n> Please review the rest of the bug report below.\n> You can delete any lines you don't wish to share.\n> \n> \n> [System Info]\n> git version:\n> git version 2.34.1\n> cpu: x86_64\n> no commit associated with this build\n> sizeof-long: 8\n> sizeof-size_t: 8\n> shell-path: /bin/sh\n> uname: Linux 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43\n> UTC 2020 x86_64 compiler info: gnuc: 9.3 libc info: glibc: 2.31 $SHELL (typically,\n> interactive shell): /bin/bash\n> \n> \n> [Enabled Hooks]\n> not run from a git repository - no hooks to show\n\nMy thoughts:\n\n1. 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.\n2. You ran is_bare_repository from next, which was in your worktree - not a bare repo, so that answer actually makes sense.\n\nI'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.\n\nJust my $0.02,\n-Randall\n\n"},{"id":"444428","messageId":"CABceR4Z+CoaUuGrJS+D1C9x+nR278S4ATWozz-ni2Y96FJc3cg@mail.gmail.com","threadId":"57117","inReplyTo":"014701d7f437$5ae45110$10acf330$@nexbridge.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Sean Allred","fromEmail":"allred.sean@gmail.com","sentAt":"2021-12-18T19:00:43Z","receivedAt":"2021-12-18T19:00:56Z","isPatch":false,"sender":{"key":"allred.sean@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2082195?v=4"},"body":"> You ran is_bare_repository from next, which was in your worktree - not a bare repo, so that answer actually makes sense.\n\nI'm not sure I follow. I did run is_bare_repository from the\nnext-worktree, but the return value was evidently dependent on the\nvalue of extensions.worktreeconfig. When true, is_bare_repository\nreturned true -- even within the next-worktree. Unless I'm missing\nsomething fairly fundamental here...\n\nOn Sat, Dec 18, 2021 at 11:47 AM <rsbecker@nexbridge.com> wrote:\n>\n> On December 18, 2021 11:47 AM, Sean Allred wrote:\n> > Hi folks! See the following bug report. Let me know if anything is unclear -- in\n> > all honesty, I neglectfully `git worktree remove --force`'d the first one I\n> > wrote...\n> >\n> > Thank you for filling out a Git bug report!\n> > Please answer the following questions to help us understand your issue.\n> >\n> > What did you do before the bug happened? (Steps to reproduce your issue)\n> >\n> >     ~$ git clone --bare https://github.com/git/git.git\n> >     ---clip---\n> >\n> >     ~/gitbare$ git config --list --show-origin\n> >     file:config     core.repositoryformatversion=1\n> >     file:config     core.filemode=false\n> >     file:config     core.bare=true\n> >     file:config     core.ignorecase=true\n> >     file:config     remote.origin.url=https://github.com/git/git.git\n> >\n> >     ~/gitbare$ git worktree add --no-checkout ../next\n> >     Preparing worktree (checking out 'next')\n> >\n> >     ~/gitbare$ git config --list --show-origin\n> >     file:config     core.repositoryformatversion=1\n> >     file:config     core.filemode=false\n> >     file:config     core.bare=true\n> >     file:config     core.ignorecase=true\n> >     file:config     remote.origin.url=https://github.com/git/git.git\n> >\n> >     ~/gitbare$ cd ../next/\n> >\n> >     ~/next$ git config --list --show-origin\n> >     file:../gitbare/config    core.repositoryformatversion=1\n> >     file:../gitbare/config    core.filemode=false\n> >     file:../gitbare/config    core.bare=true\n> >     file:../gitbare/config    core.ignorecase=true\n> >     file:../gitbare/config    remote.origin.url=https://github.com/git/git.git\n> >\n> >     ~/next$ git rev-parse --is-bare-repository\n> >     false\n> >\n> >     ~/next$ git config extensions.worktreeconfig true\n> >     ~/next$ git rev-parse --is-bare-repository\n> >     true\n> >\n> >     ~/next$ git config --unset extensions.worktreeconfig\n> >     ~/next$ git rev-parse --is-bare-repository\n> >     false\n> >\n> > I actually found this situation (and narrowed it to the above) by trying to\n> > perform a sparse-checkout in the worktree.  It appears sparse-checkout by\n> > default uses a worktree-specific config (which does make sense).\n> >\n> > What did you expect to happen? (Expected behavior)\n> >\n> >     I expected one of the following to happen:\n> >\n> >     1. I should have been blocked from creating a worktree from a bare\n> >     repository.\n> >\n> >     2. is_bare_repository() shouldn't be fooled by this situation,\n> >     assuming it's valid.\n> >\n> >     All things being equal, I would more expect to have been blocked\n> >     from creating a worktree from a bare repository.  Personally, this\n> >     bare repo + worktree setup doesn't not align with my experience so\n> >     far with how bare repos are normally used (i.e., as a convenience\n> >     for centralized remotes that will never be doing a checkout).\n> >\n> > What happened instead? (Actual behavior)\n> >\n> >     is_bare_repository() is fooled and I'm prevented from performing\n> >     any operation that requires a worktree (like performing a sparse\n> >     checkout).\n> >\n> > What's different between what you expected and what actually happened?\n> >\n> >     is_bare_repository() is fooled into thinking the worktree is not a\n> >     worktree / I'm able to create a worktree from a bare repo.\n> >\n> > Anything else you want to add:\n> >\n> > Please review the rest of the bug report below.\n> > You can delete any lines you don't wish to share.\n> >\n> >\n> > [System Info]\n> > git version:\n> > git version 2.34.1\n> > cpu: x86_64\n> > no commit associated with this build\n> > sizeof-long: 8\n> > sizeof-size_t: 8\n> > shell-path: /bin/sh\n> > uname: Linux 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43\n> > UTC 2020 x86_64 compiler info: gnuc: 9.3 libc info: glibc: 2.31 $SHELL (typically,\n> > interactive shell): /bin/bash\n> >\n> >\n> > [Enabled Hooks]\n> > not run from a git repository - no hooks to show\n>\n> My thoughts:\n>\n> 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.\n> 2. You ran is_bare_repository from next, which was in your worktree - not a bare repo, so that answer actually makes sense.\n>\n> 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.\n>\n> Just my $0.02,\n> -Randall\n>\n\n\n-- \n-Sean\n"},{"id":"444431","messageId":"000401d7f45a$005abea0$01103be0$@nexbridge.com","threadId":"57117","inReplyTo":"CABceR4Z+CoaUuGrJS+D1C9x+nR278S4ATWozz-ni2Y96FJc3cg@mail.gmail.com","subject":"RE: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2021-12-18T21:55:34Z","receivedAt":"2021-12-18T21:55:46Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On December 18, 2021 2:01 PM: Sean Allred wrote:\n> On Sat, Dec 18, 2021 at 11:47 AM <rsbecker@nexbridge.com> wrote:\n> >\n> > On December 18, 2021 11:47 AM, Sean Allred wrote:\n> > > Hi folks! See the following bug report. Let me know if anything is\n> > > unclear -- in all honesty, I neglectfully `git worktree remove\n> > > --force`'d the first one I wrote...\n> > >\n> > > Thank you for filling out a Git bug report!\n> > > Please answer the following questions to help us understand your issue.\n> > >\n> > > What did you do before the bug happened? (Steps to reproduce your\n> > > issue)\n> > >\n> > >     ~$ git clone --bare https://github.com/git/git.git\n> > >     ---clip---\n> > >\n> > >     ~/gitbare$ git config --list --show-origin\n> > >     file:config     core.repositoryformatversion=1\n> > >     file:config     core.filemode=false\n> > >     file:config     core.bare=true\n> > >     file:config     core.ignorecase=true\n> > >     file:config     remote.origin.url=https://github.com/git/git.git\n> > >\n> > >     ~/gitbare$ git worktree add --no-checkout ../next\n> > >     Preparing worktree (checking out 'next')\n> > >\n> > >     ~/gitbare$ git config --list --show-origin\n> > >     file:config     core.repositoryformatversion=1\n> > >     file:config     core.filemode=false\n> > >     file:config     core.bare=true\n> > >     file:config     core.ignorecase=true\n> > >     file:config     remote.origin.url=https://github.com/git/git.git\n> > >\n> > >     ~/gitbare$ cd ../next/\n> > >\n> > >     ~/next$ git config --list --show-origin\n> > >     file:../gitbare/config    core.repositoryformatversion=1\n> > >     file:../gitbare/config    core.filemode=false\n> > >     file:../gitbare/config    core.bare=true\n> > >     file:../gitbare/config    core.ignorecase=true\n> > >     file:../gitbare/config    remote.origin.url=https://github.com/git/git.git\n> > >\n> > >     ~/next$ git rev-parse --is-bare-repository\n> > >     false\n> > >\n> > >     ~/next$ git config extensions.worktreeconfig true\n> > >     ~/next$ git rev-parse --is-bare-repository\n> > >     true\n> > >\n> > >     ~/next$ git config --unset extensions.worktreeconfig\n> > >     ~/next$ git rev-parse --is-bare-repository\n> > >     false\n> > >\n> > > I actually found this situation (and narrowed it to the above) by\n> > > trying to perform a sparse-checkout in the worktree.  It appears\n> > > sparse-checkout by default uses a worktree-specific config (which does\n> make sense).\n> > >\n> > > What did you expect to happen? (Expected behavior)\n> > >\n> > >     I expected one of the following to happen:\n> > >\n> > >     1. I should have been blocked from creating a worktree from a bare\n> > >     repository.\n> > >\n> > >     2. is_bare_repository() shouldn't be fooled by this situation,\n> > >     assuming it's valid.\n> > >\n> > >     All things being equal, I would more expect to have been blocked\n> > >     from creating a worktree from a bare repository.  Personally, this\n> > >     bare repo + worktree setup doesn't not align with my experience so\n> > >     far with how bare repos are normally used (i.e., as a convenience\n> > >     for centralized remotes that will never be doing a checkout).\n> > >\n> > > What happened instead? (Actual behavior)\n> > >\n> > >     is_bare_repository() is fooled and I'm prevented from performing\n> > >     any operation that requires a worktree (like performing a sparse\n> > >     checkout).\n> > >\n> > > What's different between what you expected and what actually\n> happened?\n> > >\n> > >     is_bare_repository() is fooled into thinking the worktree is not a\n> > >     worktree / I'm able to create a worktree from a bare repo.\n> > >\n> > > Anything else you want to add:\n> > >\n> > > Please review the rest of the bug report below.\n> > > You can delete any lines you don't wish to share.\n> > >\n> > >\n> > > [System Info]\n> > > git version:\n> > > git version 2.34.1\n> > > cpu: x86_64\n> > > no commit associated with this build\n> > > sizeof-long: 8\n> > > sizeof-size_t: 8\n> > > shell-path: /bin/sh\n> > > uname: Linux 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28\n> > > 23:40:43 UTC 2020 x86_64 compiler info: gnuc: 9.3 libc info: glibc:\n> > > 2.31 $SHELL (typically, interactive shell): /bin/bash\n> > >\n> > >\n> > > [Enabled Hooks]\n> > > not run from a git repository - no hooks to show\n> >\n> > My thoughts:\n> >\n> > 1. I think it is legitimate to create a worktree from a bare repository. The\n> worktree is using its own working directory/index and does not require\n> anything from the bare repo.\n> > 2. You ran is_bare_repository from next, which was in your worktree - not\n> a bare repo, so that answer actually makes sense.\n\n> I'm not sure I follow. I did run is_bare_repository from the next-worktree,\n> but the return value was evidently dependent on the value of\n> extensions.worktreeconfig. When true, is_bare_repository returned true --\n> even within the next-worktree. Unless I'm missing something fairly\n> fundamental here...\n\nI 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.\n\nIt 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.\n\nHoping someone else can chime in on this.\n\n> > I'm not sure whether this is an expected use case but it does make sense\n> > to be one. If you typically work in worktrees and write scripts under that\n> > assumption, not having to worry about being in the non-worktree part of a\n> > clone makes sense. So creating a worktree off a bare repo is not a bad thing,\n> > assuming everything else is correct.\n\nThe essential part of create a worktree from a bare repo still makes sense to me.\n\n"},{"id":"444436","messageId":"e992d4b4-f9e2-a8f9-22da-e9d342c7bede@sunshineco.com","threadId":"57117","inReplyTo":"CABceR4bZmtC4rCwgxZ1BBYZP69VOUca1f_moJoP989vTUZWu9Q@mail.gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-19T20:16:40Z","receivedAt":"2021-12-19T20:16:45Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On 12/18/21 11:46 AM, Sean Allred wrote:\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> \n>      ~$ git clone --bare https://github.com/git/git.git\n>      ~/gitbare$ git config --list --show-origin\n>      file:config     core.bare=true\n>      ~/gitbare$ git worktree add --no-checkout ../next\n>      Preparing worktree (checking out 'next')\n>      ~/gitbare$ git config --list --show-origin\n>      file:config     core.bare=true\n>      ~/gitbare$ cd ../next/\n>      ~/next$ git config --list --show-origin\n>      file:../gitbare/config    core.bare=true\n>      ~/next$ git rev-parse --is-bare-repository\n>      false\n>      ~/next$ git config extensions.worktreeconfig true\n>      ~/next$ git rev-parse --is-bare-repository\n>      true\n>      ~/next$ git config --unset extensions.worktreeconfig\n>      ~/next$ git rev-parse --is-bare-repository\n>      false\n\nAs far as I can tell, this is working as designed and as documented[1]. \n`git rev-parse --is-bare-repository` is supposed to return \"true\" in the \nbare repository, and \"false\" in a worktree. However, you missed the step \n(discussed in [1]) in which it is your responsibility to move the \n`core.bare=true` setting from git.git/config to git.git/worktree.config \nmanually after setting `extensions.worktreeconfig=true`.\n\nWhen `extensions.worktreeconfig` is false, --is-bare-repository special \ncases `core.bare` to return \"false\" in a worktree regardless of the \nsetting in git.git/config. When `extensions.worktreeconfig` is true, \nhowever, that special-casing behavior is disabled, and \n--is-bare-repository returns whatever it pulls from the configuration. \nIn this case, it returned the value of `core.bare` from git.git/config \nsince you hadn't moved `core.bare=true` to git.git/worktree.config, \nwhich would be local only to the bare repository, and not seen by any \nworktrees.\n\n[1]: https://git-scm.com/docs/git-worktree#_configuration_file\n\n> I actually found this situation (and narrowed it to the above) by\n> trying to perform a sparse-checkout in the worktree.  It appears\n> sparse-checkout by default uses a worktree-specific config (which\n> does make sense).\n> \n> What did you expect to happen? (Expected behavior)\n> \n>      I expected one of the following to happen:\n> \n>      1. I should have been blocked from creating a worktree from a bare\n>      repository.\n> \n>      2. is_bare_repository() shouldn't be fooled by this situation,\n>      assuming it's valid.\n> \n>      All things being equal, I would more expect to have been blocked\n>      from creating a worktree from a bare repository.  Personally, this\n>      bare repo + worktree setup doesn't not align with my experience so\n>      far with how bare repos are normally used (i.e., as a convenience\n>      for centralized remotes that will never be doing a checkout).\n\nRegarding #1, creating worktrees from a bare repository is an \nexplicitly-supported use-case, and we've heard several times on the \nmailing list from people who take advantage of the ability in automation \nsituations (often in CI setups), as well as from people who use it as a \npart of their daily workflow (in which they don't consider any \nparticular worktree to be \"blessed\").\n\nRegarding #2, --is-bare-repository is not so much being fooled as it is \nbeing lied to by your configuration which wasn't updated to move \n`core.bare=true` to git.git/worktree.config. (Granted, it's painful that \nit's your responsibility to move the setting manually; automating would \navoid this sort of confusion.)\n\n> What happened instead? (Actual behavior)\n> \n>      is_bare_repository() is fooled and I'm prevented from performing\n>      any operation that requires a worktree (like performing a sparse\n>      checkout).\n> \n> What's different between what you expected and what actually happened?\n> \n>      is_bare_repository() is fooled into thinking the worktree is not a\n>      worktree / I'm able to create a worktree from a bare repo.\n"},{"id":"444437","messageId":"CABceR4YVd4remACJkxwSCTSYB2v3Zn1BsjKHbzeve8uHiZv1pA@mail.gmail.com","threadId":"57117","inReplyTo":"e992d4b4-f9e2-a8f9-22da-e9d342c7bede@sunshineco.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Sean Allred","fromEmail":"allred.sean@gmail.com","sentAt":"2021-12-19T20:46:59Z","receivedAt":"2021-12-19T20:47:13Z","isPatch":false,"sender":{"key":"allred.sean@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2082195?v=4"},"body":"> However, you missed the step (discussed in [1]) in which it is your\n> responsibility to move the `core.bare=true` setting from\n> git.git/config to git.git/worktree.config manually after setting\n> `extensions.worktreeconfig=true`.\n\nAhh, that makes sense!  I did notice the `core.bare` setting being\nrespected in source and figured this had a part to play (which is why\nI included git-config output).\n\nI think then that I was overzealous in trying to MWE-ify the issue: as\nI noted, I found this issue when I was trying to perform a\nsparse-checkout within the worktree.  To memory (I don't have my work\nsystem at the moment and don't have its `history`), I think it went\nsomething like this:\n\n    git worktree add --no-checkout ../next && cd ../next\n    git sparse-checkout init --cone # auto-created a worktree config\n    git sparse-checkout set t\n\nI think either the git-sparse-checkout-set command (or the\ngit-checkout I ran after) would fail complaining that I was not in a\nworktree.  Based on the above, it sounds like `init` is creating the\nworktree-specific config, but is not overriding `core.bare` in that\nconfig.  Would a patch to take this step this automatically be\nwell-received?  I see two options for when to set `core.bare=false` in\nworktree-specific config:\n\n  1. At git-worktree-add: This is probably the earliest time which\n     makes sense, but may be over-reach.  I'm not up-to-speed on how\n     worktree-specific configs are generally considered on this list.\n     If I were implementing a workaround, though, this is probably\n     where I'd make it.\n\n  2. At git-sparse-checkout-init: This is where the problem begins to\n     have an effect, so this might also make sense.\n\nI'm glad to learn about bare repositories + worktrees being a\nsupported use-case :-)\n"},{"id":"444438","messageId":"003f01d7f51f$f7bb0860$e7311920$@nexbridge.com","threadId":"57117","inReplyTo":"CABceR4YVd4remACJkxwSCTSYB2v3Zn1BsjKHbzeve8uHiZv1pA@mail.gmail.com","subject":"RE: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2021-12-19T21:32:42Z","receivedAt":"2021-12-19T21:32:53Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On December 19, 2021 3:47 PM, Sean Allred wrote:\n> To: Eric Sunshine <sunshine@sunshineco.com>\n> Cc: git@vger.kernel.org\n> Subject: Re: Bug report - Can create worktrees from bare repo / such\n> worktrees can fool is_bare_repository()\n> \n> > However, you missed the step (discussed in [1]) in which it is your\n> > responsibility to move the `core.bare=true` setting from\n> > git.git/config to git.git/worktree.config manually after setting\n> > `extensions.worktreeconfig=true`.\n> \n> Ahh, that makes sense!  I did notice the `core.bare` setting being respected\n> in source and figured this had a part to play (which is why I included git-config\n> output).\n> \n> I think then that I was overzealous in trying to MWE-ify the issue: as I noted, I\n> found this issue when I was trying to perform a sparse-checkout within the\n> worktree.  To memory (I don't have my work system at the moment and\n> don't have its `history`), I think it went something like this:\n> \n>     git worktree add --no-checkout ../next && cd ../next\n>     git sparse-checkout init --cone # auto-created a worktree config\n>     git sparse-checkout set t\n> \n> I think either the git-sparse-checkout-set command (or the git-checkout I ran\n> after) would fail complaining that I was not in a worktree.  Based on the\n> above, it sounds like `init` is creating the worktree-specific config, but is not\n> overriding `core.bare` in that config.  Would a patch to take this step this\n> automatically be well-received?  I see two options for when to set\n> `core.bare=false` in worktree-specific config:\n> \n>   1. At git-worktree-add: This is probably the earliest time which\n>      makes sense, but may be over-reach.  I'm not up-to-speed on how\n>      worktree-specific configs are generally considered on this list.\n>      If I were implementing a workaround, though, this is probably\n>      where I'd make it.\n> \n>   2. At git-sparse-checkout-init: This is where the problem begins to\n>      have an effect, so this might also make sense.\n> \n> I'm glad to learn about bare repositories + worktrees being a supported use-\n> case :-)\n\nFair enough, but 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.\n-Randall\n\n"},{"id":"444439","messageId":"CABceR4aP8BVr10RYS_0PFG7ExRTJ3tB541JGC4DJ7CUsMg9+Jw@mail.gmail.com","threadId":"57117","inReplyTo":"003f01d7f51f$f7bb0860$e7311920$@nexbridge.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Sean Allred","fromEmail":"allred.sean@gmail.com","sentAt":"2021-12-19T22:23:19Z","receivedAt":"2021-12-19T22:23:32Z","isPatch":false,"sender":{"key":"allred.sean@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2082195?v=4"},"body":"> what about the comparison code where is_bare_repository_cfg is\n> compared with 1 (it is a boolean and sometimes set to -1). This\n> would not generally pass a code review.\n\nI'm sorry, I'm afraid I don't completely follow.  Wouldn't the most\nstraightforward change be to simply follow the documented\nrecommendation when we create the worktree config in `git\nsparse-checkout init`?  Specifically,\n\n    @@ -374,6 +374,9 @@ static int set_config(enum sparse_checkout_mode mode)\n                           \"core.sparseCheckoutCone\",\n                           mode == MODE_CONE_PATTERNS ? \"true\" : NULL);\n\n    +    if (is_bare_repository())\n    +      git_config_set_in_file_gently(config_path, \"core.bare\", \"false\");\n    +\n         if (mode == MODE_NO_PATTERNS)\n             set_sparse_index_config(the_repository, 0);\n\nAre we saying the comparison within is_bare_repository() may not be\nappropriate in this case?\n\n---\n\nOr maybe it makes more sense to call set_git_work_tree() somewhere\nsensible such that get_git_work_tree() will return non-null.  Doing a\ndeeper dive into the code, that might be the *real* root cause\n(assuming the_repository->worktree is NULL in this scenario; I have\nnot run git through a debugger).  Such a change could have\nfarther-reaching consequences, though.\n"},{"id":"444440","messageId":"004001d7f52a$f0367010$d0a35030$@nexbridge.com","threadId":"57117","inReplyTo":"CABceR4aP8BVr10RYS_0PFG7ExRTJ3tB541JGC4DJ7CUsMg9+Jw@mail.gmail.com","subject":"RE: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2021-12-19T22:51:14Z","receivedAt":"2021-12-19T22:51:24Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On December 19, 2021 5:23 PM, Sean Allred wrote:\n> > what about the comparison code where is_bare_repository_cfg is\n> > compared with 1 (it is a boolean and sometimes set to -1). This would\n> > not generally pass a code review.\n> \n> I'm sorry, I'm afraid I don't completely follow.  Wouldn't the most\n> straightforward change be to simply follow the documented\n> recommendation when we create the worktree config in `git sparse-\n> checkout init`?  Specifically,\n> \n>     @@ -374,6 +374,9 @@ static int set_config(enum sparse_checkout_mode\n> mode)\n>                            \"core.sparseCheckoutCone\",\n>                            mode == MODE_CONE_PATTERNS ? \"true\" : NULL);\n> \n>     +    if (is_bare_repository())\n>     +      git_config_set_in_file_gently(config_path, \"core.bare\", \"false\");\n>     +\n>          if (mode == MODE_NO_PATTERNS)\n>              set_sparse_index_config(the_repository, 0);\n> \n> Are we saying the comparison within is_bare_repository() may not be\n> appropriate in this case?\n\nI'm suggesting that:\n\n        worktree->is_bare = (is_bare_repository_cfg == 1) ||\n                is_bare_repository();\n\nthe == 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.\n\n"},{"id":"444441","messageId":"CAPig+cTwmBViAGELF69mkYNoz+S-YW5-dcYuNfGeF-wob-=v9Q@mail.gmail.com","threadId":"57117","inReplyTo":"004001d7f52a$f0367010$d0a35030$@nexbridge.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-19T23:30:05Z","receivedAt":"2021-12-19T23:30:18Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 19, 2021 at 5:51 PM <rsbecker@nexbridge.com> wrote:\n> On December 19, 2021 5:23 PM, Sean Allred wrote:\n> > > what about the comparison code where is_bare_repository_cfg is\n> > > compared with 1 (it is a boolean and sometimes set to -1). This would\n> > > not generally pass a code review.\n> >\n> > I'm sorry, I'm afraid I don't completely follow.  Wouldn't the most\n> > straightforward change be to simply follow the documented\n> > recommendation when we create the worktree config in `git sparse-\n> > checkout init`?  Specifically,\n> >\n> >     +    if (is_bare_repository())\n> >     +      git_config_set_in_file_gently(config_path, \"core.bare\", \"false\");\n> >     +\n> >\n> > Are we saying the comparison within is_bare_repository() may not be\n> > appropriate in this case?\n>\n> I'm suggesting that:\n>\n>         worktree->is_bare = (is_bare_repository_cfg == 1) ||\n>                 is_bare_repository();\n>\n> 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.\n\n`is_bare_repository_cfg` is not exactly a boolean; it's a tristate,\nwith -1 meaning \"not yet determined\". I didn't, at the time, closely\nfollow the discussion[1] of the particular bit of code you're\nquestioning, but the `== 1` was mentioned at least a couple times,\nonce in review by Junio[2], and then in the extra patch commentary by\n\"jtan\" when he submitted v2[3]. Anyhow, if I'm following the original\ndiscussion correctly, then the usage, `== 1` (or the equivalent `> 0`)\nis probably correct, and that treating it as a simple boolean (where\n-1 is true, too) would be undesirable. (Of course, I haven't traced\nthrough the init code at all, so I don't even know if it can ever be\n-1 at this point.) Five existing consumers of this global variable use\n`== 1`, and only two use `> 0`, so this usage is at least reasonably\nconsistent with other parts of the project.\n\n[1]: https://lore.kernel.org/git/20190419172128.130170-1-jonathantanmy@google.com/T/\n[2]: https://lore.kernel.org/git/xmqqo954gira.fsf@gitster-ct.c.googlers.com/\n[3]: https://lore.kernel.org/git/20190419172128.130170-1-jonathantanmy@google.com/\n"},{"id":"444442","messageId":"CAPig+cSUT3B1OQLatvfnbyRezWm68AzcdhmkiFhnJ3T5Hs=x_w@mail.gmail.com","threadId":"57117","inReplyTo":"CAPig+cTwmBViAGELF69mkYNoz+S-YW5-dcYuNfGeF-wob-=v9Q@mail.gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-19T23:45:00Z","receivedAt":"2021-12-19T23:45:12Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 19, 2021 at 6:30 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Sun, Dec 19, 2021 at 5:51 PM <rsbecker@nexbridge.com> wrote:\n> >         worktree->is_bare = (is_bare_repository_cfg == 1) ||\n> >                 is_bare_repository();\n> >\n> > 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.\n>\n> `is_bare_repository_cfg` is not exactly a boolean; it's a tristate,\n> with -1 meaning \"not yet determined\". I didn't, at the time, closely\n> follow the discussion[1] of the particular bit of code you're\n> questioning, but the `== 1` was mentioned at least a couple times,\n> once in review by Junio[2], and then in the extra patch commentary by\n> \"jtan\" when he submitted v2[3]. Anyhow, if I'm following the original\n> discussion correctly, then the usage, `== 1` (or the equivalent `> 0`)\n> is probably correct, and that treating it as a simple boolean (where\n> -1 is true, too) would be undesirable. (Of course, I haven't traced\n> through the init code at all, so I don't even know if it can ever be\n> -1 at this point.) Five existing consumers of this global variable use\n> `== 1`, and only two use `> 0`, so this usage is at least reasonably\n> consistent with other parts of the project.\n\nThinking on it a bit more and re-reading jtan's commit message[1], it\nseems that it can be -1 at this point if `core.bare` is not set in\nconfiguration, as indicated at the end of his commit message:\n\n   In order to avoid that, also check core.bare when setting is_bare. If\n   core.bare=1, trust it, and otherwise, use is_bare_repository().\n\nIt does make me wonder if the code should have been:\n\n    if (is_bare_repository_cfg > 0)\n        worktree->is_bare = 1;\n    else if (is_bare_repository_cfg < 0)\n        worktree->is_bare = is_bare_repository();\n\nin order to respect `core.bare=0`, which the existing code doesn't\nseem to do, but perhaps I'm misunderstanding the case he was trying to\nsolve related to submodules. Anyhow, I think that's all a tangent from\nthe original issue raised in this thread by Sean (though I could be\nwrong about that too).\n\n[1]: https://lore.kernel.org/git/20190419172128.130170-1-jonathantanmy@google.com/\n"},{"id":"444443","messageId":"004301d7f533$b51c0a90$1f541fb0$@nexbridge.com","threadId":"57117","inReplyTo":"CAPig+cTwmBViAGELF69mkYNoz+S-YW5-dcYuNfGeF-wob-=v9Q@mail.gmail.com","subject":"RE: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2021-12-19T23:54:00Z","receivedAt":"2021-12-19T23:54:11Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On December 19, 2021 6:30 PM, Eric Sunshine wrote:\n> On Sun, Dec 19, 2021 at 5:51 PM <rsbecker@nexbridge.com> wrote:\n> > On December 19, 2021 5:23 PM, Sean Allred wrote:\n> > > > what about the comparison code where is_bare_repository_cfg is\n> > > > compared with 1 (it is a boolean and sometimes set to -1). This\n> > > > would not generally pass a code review.\n> > >\n> > > I'm sorry, I'm afraid I don't completely follow.  Wouldn't the most\n> > > straightforward change be to simply follow the documented\n> > > recommendation when we create the worktree config in `git sparse-\n> > > checkout init`?  Specifically,\n> > >\n> > >     +    if (is_bare_repository())\n> > >     +      git_config_set_in_file_gently(config_path, \"core.bare\", \"false\");\n> > >     +\n> > >\n> > > Are we saying the comparison within is_bare_repository() may not be\n> > > appropriate in this case?\n> >\n> > I'm suggesting that:\n> >\n> >         worktree->is_bare = (is_bare_repository_cfg == 1) ||\n> >                 is_bare_repository();\n> >\n> > the == 1 comparison should not be done for boolean-style variables. It is an\n> int, but initialized to -1. Unless -1 and 1 mean different things, but that is not\n> really documented.\n> \n> `is_bare_repository_cfg` is not exactly a boolean; it's a tristate, with -1\n> meaning \"not yet determined\". I didn't, at the time, closely follow the\n> discussion[1] of the particular bit of code you're questioning, but the `== 1`\n> was mentioned at least a couple times, once in review by Junio[2], and then\n> in the extra patch commentary by \"jtan\" when he submitted v2[3]. Anyhow,\n> if I'm following the original discussion correctly, then the usage, `== 1` (or the\n> equivalent `> 0`) is probably correct, and that treating it as a simple boolean\n> (where\n> -1 is true, too) would be undesirable. (Of course, I haven't traced through the\n> init code at all, so I don't even know if it can ever be\n> -1 at this point.) Five existing consumers of this global variable use `== 1`, and\n> only two use `> 0`, so this usage is at least reasonably consistent with other\n> parts of the project.\n> \n> [1]: https://lore.kernel.org/git/20190419172128.130170-1-\n> jonathantanmy@google.com/T/\n> [2]: https://lore.kernel.org/git/xmqqo954gira.fsf@gitster-ct.c.googlers.com/\n> [3]: https://lore.kernel.org/git/20190419172128.130170-1-\n> jonathantanmy@google.com/\n\nThanks 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?\n-Randall\n\n"},{"id":"444444","messageId":"CAPig+cQs0sp8St3KxrTsuFWU0wXteKHM1DBES0s1pzBpQ0Sv=g@mail.gmail.com","threadId":"57117","inReplyTo":"004301d7f533$b51c0a90$1f541fb0$@nexbridge.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-20T00:07:24Z","receivedAt":"2021-12-20T00:07:37Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 19, 2021 at 6:54 PM <rsbecker@nexbridge.com> wrote:\n> On December 19, 2021 6:30 PM, Eric Sunshine wrote:\n> > `is_bare_repository_cfg` is not exactly a boolean; it's a tristate, with -1\n> > meaning \"not yet determined\". I didn't, at the time, closely follow the\n> > discussion[1] of the particular bit of code you're questioning, but the `== 1`\n> > was mentioned at least a couple times, once in review by Junio[2], and then\n> > in the extra patch commentary by \"jtan\" when he submitted v2[3]. Anyhow,\n> > if I'm following the original discussion correctly, then the usage, `== 1` (or the\n> > equivalent `> 0`) is probably correct, and that treating it as a simple boolean\n> > (where\n> > -1 is true, too) would be undesirable. (Of course, I haven't traced through the\n> > init code at all, so I don't even know if it can ever be\n> > -1 at this point.) Five existing consumers of this global variable use `== 1`, and\n> > only two use `> 0`, so this usage is at least reasonably consistent with other\n> > parts of the project.\n>\n> 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?\n\nI'm not sure how much value an enum would add in this particular case\nsince this is a quite common idiom in this codebase. It might be\nsufficient merely to add a comment to its declaration in cache.h\nexplaining its possible values.\n\nWhat could be even more helpful is some documentation about the entire\nsetup sequence and how all the different \"is bare\" and \"do we have a\nworktree\" computations interact since the general setup logic is quite\ninvolved, and there are likely a number of pitfalls which are not\nobvious by merely glancing at the code, but which people know about\nwho have dealt extensively with this code (such as Duy and probably\nPeff).\n"},{"id":"444445","messageId":"CAPig+cQ6U_yFw-X2OWrizB1rbCvc4bNxuSzKFzmoLNnm0GH8Eg@mail.gmail.com","threadId":"57117","inReplyTo":"CABceR4YVd4remACJkxwSCTSYB2v3Zn1BsjKHbzeve8uHiZv1pA@mail.gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-20T00:58:14Z","receivedAt":"2021-12-20T00:58:28Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 19, 2021 at 3:47 PM Sean Allred <allred.sean@gmail.com> wrote:\n> > However, you missed the step (discussed in [1]) in which it is your\n> > responsibility to move the `core.bare=true` setting from\n> > git.git/config to git.git/worktree.config manually after setting\n> > `extensions.worktreeconfig=true`.\n>\n> Ahh, that makes sense!  I did notice the `core.bare` setting being\n> respected in source and figured this had a part to play (which is why\n> I included git-config output).\n>\n> I think then that I was overzealous in trying to MWE-ify the issue: as\n> I noted, I found this issue when I was trying to perform a\n> sparse-checkout within the worktree.  To memory (I don't have my work\n> system at the moment and don't have its `history`), I think it went\n> something like this:\n>\n>     git worktree add --no-checkout ../next && cd ../next\n>     git sparse-checkout init --cone # auto-created a worktree config\n>     git sparse-checkout set t\n\nThanks for this information. I haven't followed sparse-checkout mode\nat all and haven't used it, so I quickly scanned the man page for the\nworktree-relevant information, and indeed I was able to reproduce the\nproblem using the recipe (with the prerequisite that the repository is\nbare) which you present here.\n\n> I think either the git-sparse-checkout-set command (or the\n> git-checkout I ran after) would fail complaining that I was not in a\n> worktree.\n\nIt is indeed the `git sparse-checkout set` command which fails.\n\n> Based on the above, it sounds like `init` is creating the\n> worktree-specific config, but is not overriding `core.bare` in that\n> config.  Would a patch to take this step this automatically be\n> well-received?\n\nThis looks like a legitimate oversight, so some sort of patch to\nresolve it would likely be welcomed.\n\n> I see two options for when to set `core.bare=false` in\n> worktree-specific config:\n>\n>   1. At git-worktree-add: This is probably the earliest time which\n>      makes sense, but may be over-reach.  I'm not up-to-speed on how\n>      worktree-specific configs are generally considered on this list.\n>      If I were implementing a workaround, though, this is probably\n>      where I'd make it.\n\nMy knee-jerk reaction is that applying a \"fix\" to `git worktree add`\nto assign `core.bare=false` in the worktree-specific config may be\nmisguided, or at least feels too much like a band-aid. Although it's\ntrue that that would address the problem for worktrees created after\n`extensions.worktreeconfig=true` is set, it won't help\nalready-existing worktrees. This reason alone makes me hesitant to\nendorse this approach.\n\nWhat I could see, perhaps, is a new git-worktree subcommand which does\nall the necessary bookkeeping required when switching between\nworktree-specific configuration and the legacy configuration model.\nFor the sake of example, calling this fictitious command \"manage\",\nthen we might have \"git worktree manage --enable-worktree-config\"\nwhich both sets `extensions.worktreeconfig=true` _and_ moves\n`core.bare` and `core.worktree` from .git/config to\n.git/worktree.config if those config keys exist. The fictitious \"git\nworktree manage --disable-worktree-config\" would reverse the operation\nby moving all of .git/worktree.config into .git/config and disabling\n`extensions.worktreeconfig`. However, there's a question of what to do\nwith the worktree-specific worktree.config files when disabling\n`extensions.worktreeconfig`. Should they become orphaned or should\nthey somehow be incorporated into .git/config. Or, perhaps, there\nshouldn't even be a --disable-worktree-config switch since a human\nprobably needs to make decisions about merging the worktree-specific\nworktree.config files; or maybe --disable-worktree-config should exist\nbut error out if it can't figure out how to automatically deal with\nthe worktree-specific worktree.config files. Anyhow, this is not a\nsimple solution to the immediate problem and needs a lot more thought.\n\nAnother possibility is coming up with a migration plan to eventually\nmake worktree-specific configuration the default, and eventually drop\nsupport for the legacy configuration model. I don't know if Duy had\nany such migration plan in mind when he implemented worktree-specific\nconfiguration, but it seems a reasonable end goal. However, although\nthat would solve this problem in the long run (since `core.bare` would\nbe local to the repository and not bleed into worktrees), it doesn't\nhelp in the short term since any migration plan is likely to be\nyears-long.\n\n>   2. At git-sparse-checkout-init: This is where the problem begins to\n>      have an effect, so this might also make sense.\n\nYes, if I'm understanding everything correctly, this seems like the\nbest and most localized place to address the problem at this time. The\nsimple, but too minimal, approach would be for `git sparse-checkout\ninit` to simply add `core.bare=false` to the worktree-specific config,\nhowever, this only addresses the immediate problem and still leaves\nthings broken in general for any non-sparse worktrees.\n\nSo, the better and more correct approach, while still being localized\nto `git sparse-checkout init` is for it to respect the documented\nrequirement and automatically move `core.bare` and `core.worktree`\nfrom .git/config to .git/worktree.config if those keys exist. That\nshould leave everything in a nice Kosher state for all worktrees,\nexisting and newly-created.\n\n(I've cc:'d a few sparse checkout area experts, though I may have\nforgotten some.)\n"},{"id":"444468","messageId":"f39af047-d244-14be-4cd8-b6c3199545f3@gmail.com","threadId":"57117","inReplyTo":"CAPig+cQ6U_yFw-X2OWrizB1rbCvc4bNxuSzKFzmoLNnm0GH8Eg@mail.gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-12-20T14:11:38Z","receivedAt":"2021-12-20T14:11:44Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/19/2021 7:58 PM, Eric Sunshine wrote:\n> On Sun, Dec 19, 2021 at 3:47 PM Sean Allred <allred.sean@gmail.com> wrote:\n>>> However, you missed the step (discussed in [1]) in which it is your\n>>> responsibility to move the `core.bare=true` setting from\n>>> git.git/config to git.git/worktree.config manually after setting\n>>> `extensions.worktreeconfig=true`.\n\nThanks for this context of added responsibility. It seems a bit strange\nto require this, since it doesn't make any sense to have a bare worktree\n(at least not to me, feel free to elaborate on the need of such a\nsituation).\n\nI think the most defensive thing to do would be to always special case\ncore.bare to false when in a worktree. But if there is a reason to allow\nbare worktrees, then that isn't feasible.\n\n>> Ahh, that makes sense!  I did notice the `core.bare` setting being\n>> respected in source and figured this had a part to play (which is why\n>> I included git-config output).\n>>\n>> I think then that I was overzealous in trying to MWE-ify the issue: as\n>> I noted, I found this issue when I was trying to perform a\n>> sparse-checkout within the worktree.  To memory (I don't have my work\n>> system at the moment and don't have its `history`), I think it went\n>> something like this:\n>>\n>>     git worktree add --no-checkout ../next && cd ../next\n>>     git sparse-checkout init --cone # auto-created a worktree config\n>>     git sparse-checkout set t\n> \n> Thanks for this information. I haven't followed sparse-checkout mode\n> at all and haven't used it, so I quickly scanned the man page for the\n> worktree-relevant information, and indeed I was able to reproduce the\n> problem using the recipe (with the prerequisite that the repository is\n> bare) which you present here.\n> \n>> I think either the git-sparse-checkout-set command (or the\n>> git-checkout I ran after) would fail complaining that I was not in a\n>> worktree.\n> \n> It is indeed the `git sparse-checkout set` command which fails.\n\nRight, 'init' will set the sparse-checkout information in your worktree\nconfig and initialize it as needed, here:\n\nstatic int set_config(enum sparse_checkout_mode mode)\n{\n\tconst char *config_path;\n\n\tif (upgrade_repository_format(1) < 0)\n\t\tdie(_(\"unable to upgrade repository format to enable worktreeConfig\"));\n\tif (git_config_set_gently(\"extensions.worktreeConfig\", \"true\")) {\n\t\terror(_(\"failed to set extensions.worktreeConfig setting\"));\n\t\treturn 1;\n\t}\n\n\tconfig_path = git_path(\"config.worktree\");\n\tgit_config_set_in_file_gently(config_path,\n\t\t\t\t      \"core.sparseCheckout\",\n\t\t\t\t      mode ? \"true\" : NULL);\n\n\tgit_config_set_in_file_gently(config_path,\n\t\t\t\t      \"core.sparseCheckoutCone\",\n\t\t\t\t      mode == MODE_CONE_PATTERNS ? \"true\" : NULL);\n\n\tif (mode == MODE_NO_PATTERNS)\n\t\tset_sparse_index_config(the_repository, 0);\n\n\treturn 0;\n}\n\nSo, we are manually specifying \"put this in the config.worktree file\"\nand not going through some \"initialize worktree config\" helper. Such\na helper would be useful to avoid this issue in the future.\n\n>> Based on the above, it sounds like `init` is creating the\n>> worktree-specific config, but is not overriding `core.bare` in that\n>> config.  Would a patch to take this step this automatically be\n>> well-received?\n> \n> This looks like a legitimate oversight, so some sort of patch to\n> resolve it would likely be welcomed.\n> \n>> I see two options for when to set `core.bare=false` in\n>> worktree-specific config:\n>>\n>>   1. At git-worktree-add: This is probably the earliest time which\n>>      makes sense, but may be over-reach.  I'm not up-to-speed on how\n>>      worktree-specific configs are generally considered on this list.\n>>      If I were implementing a workaround, though, this is probably\n>>      where I'd make it.\n> \n> My knee-jerk reaction is that applying a \"fix\" to `git worktree add`\n> to assign `core.bare=false` in the worktree-specific config may be\n> misguided, or at least feels too much like a band-aid. Although it's\n> true that that would address the problem for worktrees created after\n> `extensions.worktreeconfig=true` is set, it won't help\n> already-existing worktrees. This reason alone makes me hesitant to\n> endorse this approach.\n\nYeah, my concern is that it requires the extension and could cause\nsome tools to stop working immediately. If we have the extension\nalready, it might make sense to initialize the file then.\n\n(...)\n>>   2. At git-sparse-checkout-init: This is where the problem begins to\n>>      have an effect, so this might also make sense.\n> \n> Yes, if I'm understanding everything correctly, this seems like the\n> best and most localized place to address the problem at this time. The\n> simple, but too minimal, approach would be for `git sparse-checkout\n> init` to simply add `core.bare=false` to the worktree-specific config,\n> however, this only addresses the immediate problem and still leaves\n> things broken in general for any non-sparse worktrees.\n> \n> So, the better and more correct approach, while still being localized\n> to `git sparse-checkout init` is for it to respect the documented\n> requirement and automatically move `core.bare` and `core.worktree`\n> from .git/config to .git/worktree.config if those keys exist. That\n> should leave everything in a nice Kosher state for all worktrees,\n> existing and newly-created.\n\nI agree that this is the only place that currently _writes_ to the\nworktree config. All other code paths that seem to care about the\nworktree config specifically only read from the config using a\nmodified scope.\n\nMy thought on the direction to go would be to extract some code\nfrom the set_config() in builtin/sparse-checkout.c into a config\nhelper, say \"ensure_worktree_config_exists()\", that adds the\nextension, creates the file, and then adds core.bare=false.\n\nEven better, we could create a config helper that writes to the\nworktree config, and _that_ could ensure the config is set\ncorrectly before writing the requested value.\n\nI'll take a stab at this today.\n\n> (I've cc:'d a few sparse checkout area experts, though I may have\n> forgotten some.)\n\nThank you for CC'ing me!\n-Stolee\n"},{"id":"444495","messageId":"CAPig+cQPUe9REf+wgVNjyak_nk3V361h-48rTFgk6TGC7vJgOA@mail.gmail.com","threadId":"57117","inReplyTo":"f39af047-d244-14be-4cd8-b6c3199545f3@gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-20T15:58:21Z","receivedAt":"2021-12-20T15:58:34Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"`On Mon, Dec 20, 2021 at 9:11 AM Derrick Stolee <stolee@gmail.com> wrote:\n> On 12/19/2021 7:58 PM, Eric Sunshine wrote:\n> > On Sun, Dec 19, 2021 at 3:47 PM Sean Allred <allred.sean@gmail.com> wrote:\n> >>> However, you missed the step (discussed in [1]) in which it is your\n> >>> responsibility to move the `core.bare=true` setting from\n> >>> git.git/config to git.git/worktree.config manually after setting\n> >>> `extensions.worktreeconfig=true`.\n>\n> Thanks for this context of added responsibility. It seems a bit strange\n> to require this, since it doesn't make any sense to have a bare worktree\n> (at least not to me, feel free to elaborate on the need of such a\n> situation).\n>\n> I think the most defensive thing to do would be to always special case\n> core.bare to false when in a worktree. But if there is a reason to allow\n> bare worktrees, then that isn't feasible.\n\nI suppose one of Duy's motivations when adding worktree-specific\nconfiguration -- and, importantly, configuration specific to the main\nworktree -- was to get rid of the ugly special cases (such as\nhard-coding overrides for `core.bare` and `core.worktree`) required by\nthe original (mis-)design. Importantly, those special-cases need to be\nimplemented by all third-party tools too, so it's not a winning\napproach, whereas worktree-specific configuration gets by without\nspecial cases and could easily be implemented by foreign tools. In the\nlong run, rather than re-introducing such overrides, a better solution\nprobably would be to make worktree-specific configuration the default,\nbut that's a decision for some other day.\n\n> >> I think either the git-sparse-checkout-set command (or the\n> >> git-checkout I ran after) would fail complaining that I was not in a\n> >> worktree.\n> >\n> > It is indeed the `git sparse-checkout set` command which fails.\n>\n> Right, 'init' will set the sparse-checkout information in your worktree\n> config and initialize it as needed, here:\n>\n>         if (upgrade_repository_format(1) < 0)\n>                 die(_(\"unable to upgrade repository format to enable worktreeConfig\"));\n>         if (git_config_set_gently(\"extensions.worktreeConfig\", \"true\")) {\n>                 error(_(\"failed to set extensions.worktreeConfig setting\"));\n>                 return 1;\n>         }\n>\n> So, we are manually specifying \"put this in the config.worktree file\"\n> and not going through some \"initialize worktree config\" helper. Such\n> a helper would be useful to avoid this issue in the future.\n\nYes, I was planning to suggest this in a follow-up message.\nSpecifically, I think top-level worktree.[hc] (not builtin/worktree.c)\nshould publish a function which enables worktree-specific\nconfiguration _and_ does all the necessary bookkeeping, such as moving\n`core.bare` and `core.worktree` from .git/config to\n.git/worktree.config. That way, not only can git-sparse-checkout take\nadvantage of it, but so can any command which needs the functionality\nin the future, as well as the fictitious \"git worktree manage\" command\nI mentioned earlier if it ever materializes.\n\n> >> I see two options for when to set `core.bare=false` in\n> >> worktree-specific config:\n> >>\n> >>   1. At git-worktree-add: This is probably the earliest time which\n> >>      makes sense, but may be over-reach.  I'm not up-to-speed on how\n> >>      worktree-specific configs are generally considered on this list.\n> >>      If I were implementing a workaround, though, this is probably\n> >>      where I'd make it.\n> >\n> > My knee-jerk reaction is that applying a \"fix\" to `git worktree add`\n> > to assign `core.bare=false` in the worktree-specific config may be\n> > misguided, or at least feels too much like a band-aid. Although it's\n> > true that that would address the problem for worktrees created after\n> > `extensions.worktreeconfig=true` is set, it won't help\n> > already-existing worktrees. This reason alone makes me hesitant to\n> > endorse this approach.\n>\n> Yeah, my concern is that it requires the extension and could cause\n> some tools to stop working immediately. If we have the extension\n> already, it might make sense to initialize the file then.\n\nI'm not following what you're saying. Initialize which file? When?\n\nAnyhow, this does bring up a good point and it makes me wonder if\ngit-sparse-checkout (or whatever helper function is implemented)\nshould warn the user that setting this extension could break foreign\ntools and that the repository format is being upgraded.\n\n> >>   2. At git-sparse-checkout-init: This is where the problem begins to\n> >>      have an effect, so this might also make sense.\n> >\n> > Yes, if I'm understanding everything correctly, this seems like the\n> > best and most localized place to address the problem at this time. The\n> > simple, but too minimal, approach would be for `git sparse-checkout\n> > init` to simply add `core.bare=false` to the worktree-specific config,\n> > however, this only addresses the immediate problem and still leaves\n> > things broken in general for any non-sparse worktrees.\n> >\n> > So, the better and more correct approach, while still being localized\n> > to `git sparse-checkout init` is for it to respect the documented\n> > requirement and automatically move `core.bare` and `core.worktree`\n> > from .git/config to .git/worktree.config if those keys exist. That\n> > should leave everything in a nice Kosher state for all worktrees,\n> > existing and newly-created.\n>\n> I agree that this is the only place that currently _writes_ to the\n> worktree config. All other code paths that seem to care about the\n> worktree config specifically only read from the config using a\n> modified scope.\n>\n> My thought on the direction to go would be to extract some code\n> from the set_config() in builtin/sparse-checkout.c into a config\n> helper, say \"ensure_worktree_config_exists()\", that adds the\n> extension, creates the file, and then adds core.bare=false.\n>\n> Even better, we could create a config helper that writes to the\n> worktree config, and _that_ could ensure the config is set\n> correctly before writing the requested value.\n\nPlease do not take the approach of setting `core.bare=false` in the\nworktree-specific config file. As I mentioned in the quoted text just\nabove, that only resolves the problem for the worktree in question but\nleaves all other potentially worktrees broken (both existing worktrees\nand all new worktrees which are not being used for sparse checkout).\nThe _only_ correct thing to do, as far as I can see, is for the new\nhelper function to precisely implement the requirements as laid out by\ngit-worktree.txt: specifically, enable\n`extensions.worktreeConfig=true` _and_ relocate `core.bare` and\n`core.worktree` from .git/config to .git/worktree.config.\n"},{"id":"444498","messageId":"xmqq5yrjxn79.fsf@gitster.g","threadId":"57117","inReplyTo":"f39af047-d244-14be-4cd8-b6c3199545f3@gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-20T16:20:26Z","receivedAt":"2021-12-20T16:20:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> Thanks for this context of added responsibility. It seems a bit strange\n> to require this, since it doesn't make any sense to have a bare worktree\n> (at least not to me, feel free to elaborate on the need of such a\n> situation).\n\nStepping back a bit, those who want to have two new worktrees\nattached to a single bare repository justify the wish by saying that\nneither of these two new worktrees should be the primary thing that\nthey can lose to make the other inoperable, and having a dedicated\n\"shared object and ref store\" repository makes it more symmetric and\nsafer by making it obvious which one is the precious thing to keep.\n\nWanting to create two new \"bare repository lookalike\" attached to a\nsingle bare repository might be defensible the same way.\n\nNot that the current \"git worktree\" has readily-available features\nto create such a layout.  If people who have worked on \"worktree\"\ndid not see the possibility of needing such a layout, it is\nunderstandable that such features wouldn't have been designed yet.\n\nAlso not that I think that such a layout necessarily makes sense.\n"},{"id":"444507","messageId":"0f98d3e5-f2c2-526a-0476-a89778e5b2b5@gmail.com","threadId":"57117","inReplyTo":"CAPig+cQPUe9REf+wgVNjyak_nk3V361h-48rTFgk6TGC7vJgOA@mail.gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-12-20T17:29:03Z","receivedAt":"2021-12-20T17:29:11Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/20/2021 10:58 AM, Eric Sunshine wrote:\n> `On Mon, Dec 20, 2021 at 9:11 AM Derrick Stolee <stolee@gmail.com> wrote:\n>> On 12/19/2021 7:58 PM, Eric Sunshine wrote:\n>>> On Sun, Dec 19, 2021 at 3:47 PM Sean Allred <allred.sean@gmail.com> wrote:\n>>>> I think either the git-sparse-checkout-set command (or the\n>>>> git-checkout I ran after) would fail complaining that I was not in a\n>>>> worktree.\n>>>\n>>> It is indeed the `git sparse-checkout set` command which fails.\n>>\n>> Right, 'init' will set the sparse-checkout information in your worktree\n>> config and initialize it as needed, here:\n>>\n>>         if (upgrade_repository_format(1) < 0)\n>>                 die(_(\"unable to upgrade repository format to enable worktreeConfig\"));\n>>         if (git_config_set_gently(\"extensions.worktreeConfig\", \"true\")) {\n>>                 error(_(\"failed to set extensions.worktreeConfig setting\"));\n>>                 return 1;\n>>         }\n>>\n>> So, we are manually specifying \"put this in the config.worktree file\"\n>> and not going through some \"initialize worktree config\" helper. Such\n>> a helper would be useful to avoid this issue in the future.\n> \n> Yes, I was planning to suggest this in a follow-up message.\n> Specifically, I think top-level worktree.[hc] (not builtin/worktree.c)\n> should publish a function which enables worktree-specific\n> configuration _and_ does all the necessary bookkeeping, such as moving\n> `core.bare` and `core.worktree` from .git/config to\n> .git/worktree.config. That way, not only can git-sparse-checkout take\n> advantage of it, but so can any command which needs the functionality\n> in the future, as well as the fictitious \"git worktree manage\" command\n> I mentioned earlier if it ever materializes.\n\nAh. I put my change in config.[hc], but let's discuss that in the\npatch series [1].\n\n[1] https://lore.kernel.org/git/pull.1101.git.1640015844.gitgitgadget@gmail.com\n\n>>>> I see two options for when to set `core.bare=false` in\n>>>> worktree-specific config:\n>>>>\n>>>>   1. At git-worktree-add: This is probably the earliest time which\n>>>>      makes sense, but may be over-reach.  I'm not up-to-speed on how\n>>>>      worktree-specific configs are generally considered on this list.\n>>>>      If I were implementing a workaround, though, this is probably\n>>>>      where I'd make it.\n>>>\n>>> My knee-jerk reaction is that applying a \"fix\" to `git worktree add`\n>>> to assign `core.bare=false` in the worktree-specific config may be\n>>> misguided, or at least feels too much like a band-aid. Although it's\n>>> true that that would address the problem for worktrees created after\n>>> `extensions.worktreeconfig=true` is set, it won't help\n>>> already-existing worktrees. This reason alone makes me hesitant to\n>>> endorse this approach.\n>>\n>> Yeah, my concern is that it requires the extension and could cause\n>> some tools to stop working immediately. If we have the extension\n>> already, it might make sense to initialize the file then.\n> \n> I'm not following what you're saying. Initialize which file? When?\n\nI mean when initializing the config.worktree file.\n\n> Anyhow, this does bring up a good point and it makes me wonder if\n> git-sparse-checkout (or whatever helper function is implemented)\n> should warn the user that setting this extension could break foreign\n> tools and that the repository format is being upgraded.\n\nYes, this was a concern when doing this change. I at least have not\nseen anyone complain about it. This is not critical to the sparse-checkout\nfeature, but is currently how the sparse-checkout builtin works, so is\npart of the paved path for most users getting started.\n\n>>>>   2. At git-sparse-checkout-init: This is where the problem begins to\n>>>>      have an effect, so this might also make sense.\n>>>\n>>> Yes, if I'm understanding everything correctly, this seems like the\n>>> best and most localized place to address the problem at this time. The\n>>> simple, but too minimal, approach would be for `git sparse-checkout\n>>> init` to simply add `core.bare=false` to the worktree-specific config,\n>>> however, this only addresses the immediate problem and still leaves\n>>> things broken in general for any non-sparse worktrees.\n>>>\n>>> So, the better and more correct approach, while still being localized\n>>> to `git sparse-checkout init` is for it to respect the documented\n>>> requirement and automatically move `core.bare` and `core.worktree`\n>>> from .git/config to .git/worktree.config if those keys exist. That\n>>> should leave everything in a nice Kosher state for all worktrees,\n>>> existing and newly-created.\n>>\n>> I agree that this is the only place that currently _writes_ to the\n>> worktree config. All other code paths that seem to care about the\n>> worktree config specifically only read from the config using a\n>> modified scope.\n>>\n>> My thought on the direction to go would be to extract some code\n>> from the set_config() in builtin/sparse-checkout.c into a config\n>> helper, say \"ensure_worktree_config_exists()\", that adds the\n>> extension, creates the file, and then adds core.bare=false.\n>>\n>> Even better, we could create a config helper that writes to the\n>> worktree config, and _that_ could ensure the config is set\n>> correctly before writing the requested value.\n> \n> Please do not take the approach of setting `core.bare=false` in the\n> worktree-specific config file. As I mentioned in the quoted text just\n> above, that only resolves the problem for the worktree in question but\n> leaves all other potentially worktrees broken (both existing worktrees\n> and all new worktrees which are not being used for sparse checkout).\n> The _only_ correct thing to do, as far as I can see, is for the new\n> helper function to precisely implement the requirements as laid out by\n> git-worktree.txt: specifically, enable\n> `extensions.worktreeConfig=true` _and_ relocate `core.bare` and\n> `core.worktree` from .git/config to .git/worktree.config.\n\nSorry I either misread or missed the part about moving core.bare over\nto core.worktree.\n\nPutting this in the helper that writes to config.worktree is the best\nlocation, because we don't want to force it during 'git worktree add'\nsince that would cause compatibility issues for tools that don't\nunderstand extensions.worktreeConfig.\n\nThanks for diligence in communicating what I missed.\n\n-Stolee\n"},{"id":"444547","messageId":"CAPig+cTzshfTeNDEbGTaCvZCAjnH-mmzi0i67Wu169wXG2XN2w@mail.gmail.com","threadId":"57117","inReplyTo":"0f98d3e5-f2c2-526a-0476-a89778e5b2b5@gmail.com","subject":"Re: Bug report - Can create worktrees from bare repo / such worktrees can fool is_bare_repository()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-20T21:58:15Z","receivedAt":"2021-12-20T21:58:28Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Dec 20, 2021 at 12:29 PM Derrick Stolee <stolee@gmail.com> wrote:\n> On 12/20/2021 10:58 AM, Eric Sunshine wrote:\n> > `On Mon, Dec 20, 2021 at 9:11 AM Derrick Stolee <stolee@gmail.com> wrote:\n> >> So, we are manually specifying \"put this in the config.worktree file\"\n> >> and not going through some \"initialize worktree config\" helper. Such\n> >> a helper would be useful to avoid this issue in the future.\n> >\n> > Yes, I was planning to suggest this in a follow-up message.\n> > Specifically, I think top-level worktree.[hc] (not builtin/worktree.c)\n> > should publish a function which enables worktree-specific\n> > configuration _and_ does all the necessary bookkeeping, such as moving\n> > `core.bare` and `core.worktree` from .git/config to\n> > .git/worktree.config. That way, not only can git-sparse-checkout take\n> > advantage of it, but so can any command which needs the functionality\n> > in the future, as well as the fictitious \"git worktree manage\" command\n> > I mentioned earlier if it ever materializes.\n>\n> Ah. I put my change in config.[hc], but let's discuss that in the\n> patch series [1].\n\nMy concern and sole reason for bringing it up is that this new\nfunction (which should be generally useful) should not end up in\nbuiltin/sparse-checkout.c. I had suggested worktree.c because its\nfunctionality is closely related to worktrees, however, since config.c\nhas intimate knowledge of the location of worktree configuration, that\nalso is a reasonable home for the new function. Either location should\nbe fine; I don't feel strongly either way and don't think it needs a\nlot (or any) discussion.\n"}]}