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

[Elijah Newren <newren@gmail.com>]

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:>
>
> Taking time to focus only on this outline here:
>
> >> So, I'd like to reiterate my earlier suggestion which would avoid
> >> these regressions while also fixing the reported bug:
>
> >>   * 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.

> >>   * 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.)

> >>     * `git worktree add`, if extensions.worktreeConfig is already set,
> >> will copy both the info/sparse-checkout file and the config.worktree
> >> settings (module core.bare and core.worktree, if present) to the new
> >> worktree
>
> and here, 'git worktree add' should always copy the info/sparse-checkout
> file (if core.sparseCheckout is enabled) and copy the config.worktree
> settings if extensions.worktreeConfig is enabled (and filter out
> core.bare and core.worktree in the process).

Right.

> > Thanks for the clearly written enumeration of how you expect this to
> > work. This summary pretty well (or entirely) captures the conclusions
> > I arrived at, as well, after devoting a chunk of time today thinking
> > through the cases. If I'm understanding everything correctly, the
> > approach outlined here solves the bare-worktree problem in the least
> > invasive and least dangerous way (for older Git versions and foreign
> > tools). And we don't even need the `git worktree init-worktree-config`
> > subcommand (though we need the underlying functionality).
>
> I'm happy to drop the subcommand in favor of some documentation in
> git-config.txt (Documentation/config/extensions.txt to be exact).

You may also want to make the two existing references to
extensions.worktreeConfig within Documentation/git-config.txt point
users at the extended documentation you add to
Documentation/config/extensions.txt.  (Remember, I found a reference
to extensions.worktreeConfig, tried it, and started using and
recommending it without it ever occurring to me that there was a more
detailed explanation elsewhere, so only adding to
Documentation/config/extensions.txt might run into the same
discoverability issues.)