Re: [PATCH v6 2/2] config: add "worktree" and "worktree/i" includeIf conditions

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jul 06, 2026 at 08:18:39PM +0800, Chen Linxuan wrote:
> 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