On Tue, Dec 21, 2021 at 2:14 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> The previous change added repo_config_set_worktree_gently() to assist
> writing config values into the worktree.config file, especially when
> that may not have been initialized.
>
> When the base repo is bare, running 'git sparse-checkout init' in a
> worktree will create the config.worktree file for the worktree, but that
> will start causing the worktree to parse the bare repo's core.bare=true
> value and start treating the worktree as bare. This causes more problems
> as other commands are run in that worktree.
>
> The fix is to have this assignment into config.worktree be handled by
> the repo_config_set_worktree_gently() helper.
>
> Reported-by: Sean Allred <allred.sean@xxxxxxxxx>
> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> @@ -71,6 +71,20 @@ test_expect_success 'git sparse-checkout init' '
> +test_expect_success 'init in a worktree of a bare repo' '
Nit: Although `init` doing an incomplete job of enabling per-worktree
config was indeed the source of the problem, it actually manifested
when invoking other commands, such as `set`. Consequently, it may be
slightly misleading to talk about `init` in the test title. A title
such as "worktree of a bare repo" might be good enough. Anyhow, just a
nit.
> + test_when_finished rm -rf bare worktree &&
> + git clone --bare repo bare &&
> + git -C bare worktree add ../worktree &&
> + (
> + cd worktree &&
> + git sparse-checkout init &&
> + test_must_fail git config core.bare &&
Nit: I'm rather "meh" on explicitly checking `core.bare` here since
it's not particularly relevant to the test: just testing `init + set`
alone is enough to trigger the bug which begat this patch series.
Future readers of this test might even be confused by the presence of
this `core.bare` check.
> + git sparse-checkout set /*
The `/*` is expanding to all entries in the root of the filesystem,
which probably isn't what you intended. I suspect you want literal
"/*", in which case you need to quote it:
git sparse-checkout set "/*"
> + ) &&
> + git -C bare config --list --show-origin >actual &&
> + grep "file:config.worktree core.bare=true" actual
As mentioned above, I'm fairly meh on this part (and perhaps leaning
toward the negative) since it places too much emphasis on a low-level
detail. I _could_ see this as a test of the new function which
upgrades the repo to per-worktree config, but that's not what _this_
test is about.
> +'
