On Thu, Dec 30, 2021 at 9:41 AM Derrick Stolee <stolee@xxxxxxxxx> wrote: > > On 12/30/2021 2:40 AM, Eric Sunshine wrote: > > On Wed, Dec 29, 2021 at 4:40 AM Elijah Newren <newren@xxxxxxxxx> 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.)