Re: [PATCH v6 2/2] config: add "worktree" and "worktree/i" includeIf conditions
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jul 7, 2026, 15:26 UTC
- Message-ID
- <ak0am-pEdtOvyBp4@pks.im>
- In-Reply-To
- <CAC1kPDNBecLbmZwjfR5-CsNheF3rcbZ5=SQ+cwjzpFMjFr9KGQ@mail.gmail.com>
On Mon, Jul 06, 2026 at 08:18:39PM +0800, Chen Linxuan wrote:
Show 86 quoted lines
> On Fri, Jul 3, 2026 at 7:03 PM Patrick Steinhardt <ps@pks.im> wrote: > > > > On Fri, Jul 03, 2026 at 11:13:18AM +0800, Chen Linxuan via B4 Relay wrote: > > > diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh > > > index f3892578e4ff..4e840dfdb35b 100755 > > > --- a/t/t1305-config-include.sh > > > +++ b/t/t1305-config-include.sh > > > @@ -396,4 +396,132 @@ test_expect_success 'onbranch without repository but explicit nonexistent Git di > > [snip] > > > +test_expect_success SYMLINKS 'conditional include, worktree resolves symlinks' ' > > > + mkdir real-wt && > > > + ln -s real-wt link-wt && > > > + git init link-wt/repo && > > > + ( > > > + cd link-wt/repo && > > > + # repo->worktree resolves symlinks, so use real path in pattern > > > + echo "[includeIf \"worktree:**/real-wt/repo\"]path=bar-link" >>.git/config && > > > + echo "[test]wtlink=2" >.git/bar-link && > > > + echo 2 >expect && > > > + git config test.wtlink >actual && > > > + test_cmp expect actual > > > + ) > > > +' > > > > Okay, this covers one scenario. But with "gitdir:" we're actually able > > to use both the symlinked and the real location: > > > > test_expect_success SYMLINKS 'conditional include, worktree matching symlink' ' > > mkdir sym-real && > > ln -s sym-real sym-link && > > git init sym-link/repo && > > ( > > cd sym-link/repo && > > link_path="$(pwd)" && > > real_path="$(test-tool path-utils real_path "$link_path")" && > > cat >>.git/config <<-EOF && > > [includeIf "gitdir:$link_path/.git"] > > path = gitdir-link > > [includeIf "gitdir:$real_path/.git"] > > path = gitdir-real > > [includeIf "worktree:$link_path"] > > path = worktree-link > > [includeIf "worktree:$real_path"] > > path = worktree-real > > EOF > > echo "[test]gitdirlink=1" >.git/gitdir-link && > > echo "[test]gitdirreal=1" >.git/gitdir-real && > > echo "[test]worktreelink=1" >.git/worktree-link && > > echo "[test]worktreereal=1" >.git/worktree-real && > > > > git config get test.gitdirlink && > > git config get test.gitdirreal && > > git config get test.worktreereal && > > test_must_fail git config test.worktreelink > > ) > > ' > > > > The last call to git-config(1) fails, which is inconsistent with how > > resolve the path for "gitdir". > > > > I investigated the symlink mismatch. > > `gitdir:` works because `opts->git_dir` still preserves the discovered or > user-provided spelling, and `include_by_path()` matches both its realpath > and its absolute non-realpath form. > > `worktree:` is different: `repo_get_work_tree()` returns > `repo->worktree`, which is stored by `repo_set_worktree()` via > `real_pathdup(path, 1)`. So the symlink spelling is already lost before > we evaluate includeIf conditions. > > Changing `repo->worktree` itself to preserve the original spelling looks > risky, because several users access `repo->worktree` directly, and setup > code appears to rely on it being canonical. > > My current possible v7 approach is to keep `repo->worktree` canonical, > but store an additional absolute, normalized, non-realpath worktree path > for `includeIf.worktree`. For the ordinary discovered-repository case, > this has to be derived in `setup_discovered_git_dir()` from physical > `cwd`, the worktree-root offset, and a validated `$PWD`, because > `set_git_work_tree()` is otherwise only called with `"."`. > > This makes your suggested test pass, but the plumbing is less trivial > than the original patch. Does this approach sound reasonable, or would > you prefer different semantics for symlinked worktree paths?
It certainly sounds a bit ugly, but I'd rather have something that's ugly than something that's inconsistent for our users *shrug*
Thanks!
Patrick