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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Dec 19, 2021 at 3:47 PM Sean Allred <allred.sean@xxxxxxxxx> wrote:
> > However, you missed the step (discussed in [1]) in which it is your
> > responsibility to move the `core.bare=true` setting from
> > git.git/config to git.git/worktree.config manually after setting
> > `extensions.worktreeconfig=true`.
>
> Ahh, that makes sense!  I did notice the `core.bare` setting being
> respected in source and figured this had a part to play (which is why
> I included git-config output).
>
> I think then that I was overzealous in trying to MWE-ify the issue: as
> I noted, I found this issue when I was trying to perform a
> sparse-checkout within the worktree.  To memory (I don't have my work
> system at the moment and don't have its `history`), I think it went
> something like this:
>
>     git worktree add --no-checkout ../next && cd ../next
>     git sparse-checkout init --cone # auto-created a worktree config
>     git sparse-checkout set t

Thanks for this information. I haven't followed sparse-checkout mode
at all and haven't used it, so I quickly scanned the man page for the
worktree-relevant information, and indeed I was able to reproduce the
problem using the recipe (with the prerequisite that the repository is
bare) which you present here.

> I think either the git-sparse-checkout-set command (or the
> git-checkout I ran after) would fail complaining that I was not in a
> worktree.

It is indeed the `git sparse-checkout set` command which fails.

> Based on the above, it sounds like `init` is creating the
> worktree-specific config, but is not overriding `core.bare` in that
> config.  Would a patch to take this step this automatically be
> well-received?

This looks like a legitimate oversight, so some sort of patch to
resolve it would likely be welcomed.

> I see two options for when to set `core.bare=false` in
> worktree-specific config:
>
>   1. At git-worktree-add: This is probably the earliest time which
>      makes sense, but may be over-reach.  I'm not up-to-speed on how
>      worktree-specific configs are generally considered on this list.
>      If I were implementing a workaround, though, this is probably
>      where I'd make it.

My knee-jerk reaction is that applying a "fix" to `git worktree add`
to assign `core.bare=false` in the worktree-specific config may be
misguided, or at least feels too much like a band-aid. Although it's
true that that would address the problem for worktrees created after
`extensions.worktreeconfig=true` is set, it won't help
already-existing worktrees. This reason alone makes me hesitant to
endorse this approach.

What I could see, perhaps, is a new git-worktree subcommand which does
all the necessary bookkeeping required when switching between
worktree-specific configuration and the legacy configuration model.
For the sake of example, calling this fictitious command "manage",
then we might have "git worktree manage --enable-worktree-config"
which both sets `extensions.worktreeconfig=true` _and_ moves
`core.bare` and `core.worktree` from .git/config to
.git/worktree.config if those config keys exist. The fictitious "git
worktree manage --disable-worktree-config" would reverse the operation
by moving all of .git/worktree.config into .git/config and disabling
`extensions.worktreeconfig`. However, there's a question of what to do
with the worktree-specific worktree.config files when disabling
`extensions.worktreeconfig`. Should they become orphaned or should
they somehow be incorporated into .git/config. Or, perhaps, there
shouldn't even be a --disable-worktree-config switch since a human
probably needs to make decisions about merging the worktree-specific
worktree.config files; or maybe --disable-worktree-config should exist
but error out if it can't figure out how to automatically deal with
the worktree-specific worktree.config files. Anyhow, this is not a
simple solution to the immediate problem and needs a lot more thought.

Another possibility is coming up with a migration plan to eventually
make worktree-specific configuration the default, and eventually drop
support for the legacy configuration model. I don't know if Duy had
any such migration plan in mind when he implemented worktree-specific
configuration, but it seems a reasonable end goal. However, although
that would solve this problem in the long run (since `core.bare` would
be local to the repository and not bleed into worktrees), it doesn't
help in the short term since any migration plan is likely to be
years-long.

>   2. At git-sparse-checkout-init: This is where the problem begins to
>      have an effect, so this might also make sense.

Yes, if I'm understanding everything correctly, this seems like the
best and most localized place to address the problem at this time. The
simple, but too minimal, approach would be for `git sparse-checkout
init` to simply add `core.bare=false` to the worktree-specific config,
however, this only addresses the immediate problem and still leaves
things broken in general for any non-sparse worktrees.

So, the better and more correct approach, while still being localized
to `git sparse-checkout init` is for it to respect the documented
requirement and automatically move `core.bare` and `core.worktree`
from .git/config to .git/worktree.config if those keys exist. That
should leave everything in a nice Kosher state for all worktrees,
existing and newly-created.

(I've cc:'d a few sparse checkout area experts, though I may have
forgotten some.)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux