Re: [PATCH v3 0/6] Sparse checkout: fix bug with worktree of bare repo

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Dec 30, 2021 at 2:29 PM Elijah Newren <newren@gmail.com> wrote:
> On Thu, Dec 30, 2021 at 9:41 AM Derrick Stolee <stolee@gmail.com> wrote:
> > On 12/30/2021 2:40 AM, Eric Sunshine wrote:
> > > On Wed, Dec 29, 2021 at 4:40 AM Elijah Newren <newren@gmail.com> wrote:>
> > >>   * If core.bare=true or core.worktree is set, then at `git worktree
> > >> add` time, automatically run the logic you have here for
> > >> init-worktree-config.  Having either of those config settings with
> > >> multiple worktrees is currently broken in all git versions and likely
> > >> in most all external tools.  As such, being aggressive in the new
> > >> config settings to allow new versions of git to work seems totally
> > >> safe to me -- we can't be any more broken than we already were.
> >
> > I'm not sure I agree with the "currently broken in all git versions"
> > because when extensions.worktreeConfig is not enabled, the core.bare
> > and core.worktree settings are ignored by the worktrees. This upgrade
> > during 'add' is the only thing I am not so sure about.
>
> Oh, you're right; I mis-spoke.  If someone has core.bare=true and has
> multiple worktrees, AND never attempts to use sparse-checkouts OR
> otherwise set extensions.worktreeConfig, then git still works due to
> git's special-case logic that will override core.bare in this
> configuration.  It's just setting them up for a ticking time bomb,
> waiting for them to either use an external tool that doesn't share
> that special case override-core.bare logic, or for the user to decide
> to set extensions.worktreeConfig directly or use sparse-checkouts.

So, how does this alter the proposed logic? Or does it? Does the above
condition get revised to:

    if extensions.worktreeConfig=true and
        (.git/config contains core.bare=true or core.worktree):
            relocate core.bare/core.worktree to .git/config.worktree

That is, we need to relocate core.bare and core.worktree from
.git/config to .git/config.worktree if and only if
extensions.worktreeConfig=true (because, due to the special-case
handling, those two keys don't interfere with anything when
extensions.worktreeConfig is not true).

This, of course, doesn't help the case if someone has existing
worktrees and decides to flip extensions.worktreeConfig to true
without doing the manual bookkeeping, but that case has always been
broken (and is documented, though not necessarily where people will
look). The new `git worktree add` logic, however, will fix that
brokenness automatically when a new worktree is added.

> > >>   * If core.bare=false and core.worktree is not set, then:
> >
> > >>     * `git sparse-checkout {init,set}` should set
> > >> extensions.worktreeConfig if not already set, and always set the
> > >> core.sparse* and index.sparse settings in worktree-specific files.
> >
> > This should happen no matter the case of core.bare and core.worktree
> > existing, right?
>
> Hmm.  I think that's safe for people who cloned and used `git worktree
> add` with newer git versions, since `git worktree add` will have moved
> core.bare and core.worktree to the config.worktree file when those
> have non-default values.
>
> But, we might want to help out the folks who have existing repos with
> which they have used older git versions.  So, we could have `git
> sparse-checkout {init,set}` check for non-default values of
> core.bare/core.worktree in the shared config file, and, if found, exit
> with an error which point users at some relevant documentation (which
> may just suggest 'git worktree add temporary && git worktree remove
> temporary' as a workaround for those caught in such a state.)

I'm probably missing something obvious, but rather than error out,
can't we just automatically relocate core.bare and core.worktree from
.git/config to .git/config.worktree in this case?