Re: [PATCH v5 4/5] sparse-checkout: set worktree-config correctly

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 6, 2022, 10:21 UTC
Message-ID: <CAPig+cTqmjrciKZa7DPA8B5Em6bNPprrJGaO6xCkRvmxvzQgHw@mail.gmail.com>
In-Reply-To: <08b89d17ccfab93546c9e84accaea84ce93ab932.1643641259.git.gitgitgadget@gmail.com>

On Mon, Jan 31, 2022 at 10:01 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 14 quoted lines

> The previous change added repo_config_set_worktree_gently() to assist
> writing config values into the config.worktree file, if enabled. An
> earlier change added init_worktree_config() as a helper to initialize
> extensions.worktreeConfig if not already enabled.
>
> Let the sparse-checkout builtin use these helpers instead of attempting to
> initialize the worktree config on its own. This changes behavior of 'git
> sparse-checkout set' in a few important ways:
>
>  1. Git will no longer upgrade the repository format, since this is not
>     a requirement for understanding extensions.worktreeConfig.
>
>  2. If the main worktree is bare, then this command will not put the
>     worktree in a broken state.

Although the three of four of us involved in this discussion understand what "broken state" means, such a description may be too vague for future readers. To help them out, perhaps we can do a better job of conveying the nature of the actual breakage by rewriting all of the above like this:

    `git sparse-checkout set/init` enables worktree-specific
    configuration[*] by setting extensions.worktreeConfig=true, but
    neglects to perform the additional necessary bookkeeping of
    relocating `core.bare=true` and `core.worktree` from
    $GIT_COMMON_DIR/config to $GIT_COMMON_DIR/config.worktree, as
    documented in git-worktree.txt. As a result of this oversight,
    these settings, which are nonsensical for secondary worktrees, can
    cause Git commands to incorrectly consider a worktree bare (in the
    case of `core.bare`) or operate on the wrong worktree (in the case
    of `core.worktree`). Fix this problem by taking advantage of the
    recently-added init_worktree_config() which enables
    `extensions.worktreeConfig` and takes care of necessary
    bookkeeping.

    While at it, for backward-compatibility reasons, also stop
    upgrading the repository format to "1" since doing so is
    (unintentionally) not required to take advantage of
    `extensions.worktreeConfig`, as explained by 11664196ac ("Revert
    "check_repository_format_gently(): refuse extensions for old
    repositories"", 2020-07-15).

Show 5 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.

Perhaps this paragraph can become the "[*]" footnote I referenced in the above rewrite.

> Reported-by: Sean Allred <allred.sean@gmail.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>