Re: [PATCH v3 5/6] sparse-checkout: use repo_config_set_worktree_gently()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 30, 2021, 09:01 UTC
Message-ID: <CAPig+cTGdmzfY5r3gdewdEwcRMDdnS9FtZxZNLSCNQYHeQq=wA@mail.gmail.com>
In-Reply-To: <b200819c1bb564695b68d8ed2caebfca0302f93f.1640727143.git.gitgitgadget@gmail.com>

On Tue, Dec 28, 2021 at 4:32 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

> The previous change added repo_config_set_worktree_gently() to assist
> writing config values into the worktree.config file, if enabled.

s/worktree.config/config.worktree/

(I made the same mistake several times earlier in this discussion.)

> Let the sparse-checkout builtin use this helper instead of attempting to
> initialize the worktree config on its own. This changes behavior of 'git
> sparse-checkout set' in a few important ways:

"worktree config" is a bit ambiguous here. It might be clearer to say "enable per-worktree configuration" or "enable worktree-specific configuration".

Show 10 quoted lines

>  1. Git will no longer upgrade the repository format and add the
>     worktree config extension. The user should run 'git worktree
>     init-worktree-config' to enable this feature.
>
>  2. If worktree config is disabled, then this command will set the
>     core.sparseCheckout (and possibly core.sparseCheckoutCone and
>     index.sparse) values in the common config file.
>
>  3. If the main worktree is bare, then this command will not put the
>     worktree in a broken state.

There are a few other cases of ambiguity in this enumerated list, as well, and the need for `s/main worktree is bare/repository is bare/`, however, if you end up restructuring this series the to follow the recipe Elijah set forth at the bottom of [1], then most of this patch's commit message will be rewritten anyhow, so my mention of these minor issues will be moot.

[1]: https://lore.kernel.org/git/CABPp-BHuO3B366uJuODMQo-y449p8cAMVn0g2MTcO5di3Xa7Zg@mail.gmail.com/

Show 29 quoted lines

> The main reason to use worktree-specific config for the sparse-checkout
> builtin was to avoid enabling sparse-checkout patterns in one and
> causing a loss of files in another. If a worktree does not have a
> sparse-checkout patterns file, then the sparse-checkout logic will not
> kick in on that worktree.
>
> This new logic introduces a new user pattern that could lead to some
> confusion. Suppose a user has not upgraded to worktree config and
> follows these steps in order:
>
>  1. Enable sparse-checkout in a worktree.
>
>  2. Disable sparse-checkout in that worktree without deleting that
>     worktree's sparse-checkout file.
>
>  3. Enable sparse-checkout in another worktree.
>
> After these steps, the first worktree will have sparse-checkout enabled
> with whatever patterns exist. The worktree does not immediately have
> those patterns applied, but a variety of Git commands would apply the
> sparse-checkout patterns and update the worktree state to reflect those
> patterns. This situation is likely very rare and the workaround is to
> upgrade to worktree specific config on purpose. Users already in this
> state used the sparse-checkout builtin with a version that upgraded to
> worktree config, anyway.
>
> Reported-by: Sean Allred <allred.sean@gmail.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>