Git development
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: me@black-desk.cn
Cc: git@vger.kernel.org,
	Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v4 2/2] config: add "worktree" and "worktree/i" includeIf conditions
Date: Thu, 21 May 2026 14:09:46 +0200	[thread overview]
Message-ID: <ag72CqZP7iFR8wWj@pks.im> (raw)
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:
> 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]
> +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.

> 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]
> +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

      parent reply	other threads:[~2026-05-21 12:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13  8:08 [PATCH v4 0/2] includeIf: add "worktree" condition for matching working tree path Chen Linxuan via B4 Relay
2026-05-13  8:08 ` [PATCH v4 1/2] config: refactor include_by_gitdir() into include_by_path() Chen Linxuan via B4 Relay
2026-05-13  8:08 ` [PATCH v4 2/2] config: add "worktree" and "worktree/i" includeIf conditions Chen Linxuan via B4 Relay
2026-05-13 13:59   ` Phillip Wood
2026-05-21 12:09   ` Patrick Steinhardt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ag72CqZP7iFR8wWj@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=me@black-desk.cn \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox