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

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Dec 21, 2021 at 2:14 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> The root cause is that 'git sparse-checkout init' writes to the worktree
> config without checking that core.bare might need to be set. This only
> matters when the base repository is bare, since creating the config.worktree
> file and enabling extensions.worktreeConfig will cause Git to treat the base
> repo's core.bare=false as important for this worktree.

I'm having trouble understanding what this is trying to say. Did you
mean "true" rather than "false" in the final sentence? Anyhow, I think
this description is somewhat stale. A more succinct way to describe
the issue is that `git sparse-checkout init` wasn't correctly
upgrading the repo to per-worktree configuration, with the result that
the `core.bare=true` setting of a bare repo bled into
worktree-specific configuration, which caused a bit of havoc. This
patch series fixes `init` to upgrade the repo properly.

> The critical bits are in Patches 3, 4, and 5 which introduce a helper for
> upgrading to worktree config, a helper to write to worktree config, and then
> consume that config helper in builtin/sparse-checkout.c and sparse-index.c.
>
> Update in v2
> ============
>
>  * Eric correctly pointed out that I was writing core.bare incorrectly. It
>    should move out of the core config and into the core repository's
>    worktree config.
>  * Patch 3 is new, separating the "upgrade" logic out of config.c, but it is
>    still called by the config helper to make it painless to write worktree
>    config.

It's good to see the "upgrade to per-worktree config" split out to a
standalone single-purpose utility function. I left several review
comments in that patch, the most important of which is that the
implementation is incomplete (because it doesn't relocate
`core.worktree`), thus it can leave the repo in an inconsistent and
broken state. Several of the other review comments are actionable, as
well.

I'm still concerned about and uncomfortable with the implementation of
the new repo_config_set_worktree_gently(), but I've left ample
comments about that elsewhere in this discussion, so I needn't go into
it here.

Thanks for working on this.