On Wed, Dec 22, 2021 at 8:00 AM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> This patch series includes a fix to the bug reported by Sean Allred [1] and
> diagnosed by Eric Sunshine [2].
>
> 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.
>
> This series fixes this, but also puts in place some helpers to prevent this
> from happening in the future. While here, some of the config paths are
> modified to take a repository struct.
>
> 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.
Based on the description I went and fetched the patch series and tried it out.
This feels like a bandaid to me. In addition to fixating on core.bare
(thus overlooking core.worktree), it also overlooks that people can
use worktrees without using sparse-checkout. What if they do
something like:
git clone --bare $URL myrepo
cd myrepo
git worktree add foo
git worktree add bar
git worktree add baz
... days/weeks later ...
cd foo
git config extensions.worktreeConfig true
git config status.showUntrackedFiles no # Or other config options
... hours/days later ..
cd ../bar
git status
At this point the user gets "fatal: this operation must be run in a
work tree". And it's much, much worse if the original clone was not
bare, but had core.worktree pointing somewhere else (because then the
`git status` invocation will show the differences between the *other*
worktree with the HEAD of *this* worktree). I think that "git
worktree add" should check if either core.bare is false or
core.worktree is set, and if so then set extensions.worktreeConfig and
migrate the relevant config.
While there may be some concern about non-git clients not
understanding extensions.worktreeConfig, I'd say that this type of
situation is one where we are just flat broken without it. Without
it, we're stuck in a situation that will: (a) confuse users ("Why does
core.bare/core.worktree seem to get ignored or have incorrect
values?"), (b) confuse non-git clients (are they really going to have
the tricky logic to overrule core.bare/core.worktree when in another
worktree?), (c) confuse git itself after subsequent configuration
tweaks, and (d) (related to item c) lead to ever more complicated
logic in core git to attempt to know when and how to overrule
core.bare and core.worktree as additional concerns enter the picture
(e.g. will someone contribute code to override core.bare/core.worktree
when run from a worktree with extensions.worktreeConfig=true, just as
someone originally wrote code to override core.bare/core.worktree when
the extensions.worktreeConfig setting wasn't present?)
I also think `git worktree add` should handle additional configuration
items related to sparse checkouts (as we've discussed elsewhere in the
past), but that's going a bit outside the scope of this series; I only
mention it so that we're aware the functionality added to `git
worktree add` will be getting some additions in the future.
