Re: [PATCH v4 2/2] config: add "worktree" and "worktree/i" includeIf conditions
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 21, 2026, 12:09 UTC
- Message-ID
- <ag72CqZP7iFR8wWj@pks.im>
- In-Reply-To
- <20260513-includeif-worktree-v4-2-f8e6212d1fba@black-desk.cn>
On Wed, May 13, 2026 at 04:08:18PM +0800, Chen Linxuan via B4 Relay wrote:
Show 18 quoted lines
> diff --git a/Documentation/config.adoc b/Documentation/config.adoc > index 62eebe7c5450..6299b1e3a019 100644 > --- a/Documentation/config.adoc > +++ b/Documentation/config.adoc > @@ -146,6 +146,46 @@ refer to linkgit:gitignore[5] for details. For convenience: > This is the same as `gitdir` except that matching is done > case-insensitively (e.g. on case-insensitive file systems) > > +`worktree`:: > + The data that follows the keyword `worktree` and a colon is used as a > + glob pattern. If the working directory of the current worktree matches > + the pattern, the include condition is met. > ++ > +The worktree location is the path where files are checked out (as returned > +by `git rev-parse --show-toplevel`). This is different from `gitdir`, which > +matches the `.git` directory path. In a linked worktree, the worktree path > +is the directory where that worktree's files are located, not the main > +repository's `.git` directory.
Nit: I feel like the first sentence already says it all, and the remainder is not adding much value. But I'm probably also quite biased given that I'm familiar with interals, so I don't insist on any change here.
[snip]
Show 7 quoted lines
> +While `extensions.worktreeConfig` (see linkgit:git-worktree[1]) also supports > +per-worktree configuration, it stores the config inside each repository's > +`.git/config.worktree` file and requires running `git config --worktree` > +inside each worktree individually. In contrast, `includeIf "worktree:..."` > +can be set once in a global or system-level configuration file (e.g. > +`~/.config/git/config`) and applies to all repositories at once based on > +their worktree location.
Nit: I tihnk saying "global or system-level" is totally sufficient, there really is no need to explain where those files live.
But again, I'm not sure whether we need a new version for this change.
Show 5 quoted lines
> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh > index 6e51f892f320..07b6fb649cd2 100755 > --- a/t/t1305-config-include.sh > +++ b/t/t1305-config-include.sh > @@ -396,4 +396,117 @@ test_expect_success 'onbranch without repository but explicit nonexistent Git di
[snip]
Show 14 quoted lines
> +test_expect_success 'conditional include, worktree, icase' ' > + git init wt-icase && > + ( > + cd wt-icase && > + test_commit initial && > + wt_path="$(pwd)" && > + wt_upper=$(echo "$wt_path" | tr a-z A-Z) && > + echo "[includeIf \"worktree/i:$wt_upper\"]path=icase-inc" >>.git/config && > + echo "[test]wticase=1" >.git/icase-inc && > + echo 1 >expect && > + git config test.wticase >actual && > + test_cmp expect actual > + ) > +'
Ah, one more thing I didn't notice for the last version: it's good that we have a check for the case-insensitive behaviour, but we're missing a test that verifies that we're in fact case-sensitive by default. That test would of course not work with a case-insensitive filesystem, but we can depend on the `!CASE_INSENSITIVE_FS` prerequisite for that.
Other than that this series looks good to me, thanks!
Patrick