Re: [PATCH v2 5/5] sparse-checkout: use repo_config_set_worktree_gently()
From: Eric Sunshine <hidden>
Date: 2021-12-22 05:48:34
On Tue, Dec 21, 2021 at 2:14 PM Derrick Stolee via GitGitGadget [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted> Helped-by: Eric Sunshine [off-list ref] Signed-off-by: Derrick Stolee <redacted> ---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.
+'