Re: [PATCH v2 0/5] Sparse checkout: fix bug with worktree of bare repo
From: Eric Sunshine <hidden>
Date: 2021-12-22 06:05:12
On Tue, Dec 21, 2021 at 2:14 PM Derrick Stolee via GitGitGadget [off-list ref] 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.