Re: [PATCH] config: introduce an Operating System-specific `includeIf` condition

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Nov 21 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> It is relatively common for users to maintain identical `~/.gitconfig`
> files across all of their setups, using the `includeIf` construct
> liberally to adjust the settings to the respective setup as needed.
>
> In case of Operating System-specific adjustments, Git currently offers
> no support to the users and they typically use a work-around like this:
>
> 	[includeIf "gitdir:/home/"]
> 		path = ~/.gitconfig-linux
> 	[includeIf "gitdir:/Users/"]
> 		path = ~/.gitconfig-mac
> 	[includeIf "gitdir:C:"]
> 		path = ~/.gitconfig-windows
>
> However, this is fragile, as it would not even allow to discern between
> Operating Systems that happen to host their home directories in
> `/home/`, such as Linux and the BSDs.

This looks like a really sensible thing to do, thanks.

> +`os`::
> +	The data that follows this keyword is taken as the name of an
> +	Operating System; If it matches the output of `uname -s`, the
> +	include condition is met.

Here yu say it "matches uname -s", but later in the test we can see
that's not the case. This is because compat/mingw.c is the source of
that "Windows" string, which we use for the uname() at the C level.

I don't think we've needed to document it before, but we should do so
here I'd think. Per
https://stackoverflow.com/questions/3466166/how-to-check-if-running-in-cygwin-mac-or-linux
users would follow these docs, try MinGw, then be confused, no?

> +static int include_by_os(const char *cond, size_t cond_len)
> +{
> +	struct utsname uname_info;
> +
> +	return !uname(&uname_info) &&
> +		!strncasecmp(uname_info.sysname, cond, cond_len) &&

Our config.mak.uname doesn't to case-insensitive uname matching, and
AFAIK these don't change between platforms versions. So why do we need
to support LINUX, LiNuX etc. in addition to the canonical Linux?

I'm not opposed to it if there's a good reason, but as we have "gitdir"
and "gitdir/i" shouldn't we make that "os/i" for consistency, if it's
needed?

> +test_expect_success '[includeIf "os:..."]' '
> +	test_config x.y 0 &&
> +	echo "[x] y = z" >.git/xyz &&
> +
> +	if test_have_prereq MINGW
> +	then
> +		uname_s=Windows
> +	else
> +		uname_s="$(uname -s)"
> +	fi &&
> +	test_config "includeIf.os:not-$uname_s.path" xyz &&

Re above: If it is important to support LINUX etc. these tests should
check it, they pass if it's converted to strncmp().

> +	test 0 = "$(git  config x.y)" &&

Hides segfaults, use test_cmp or test_cmp_config?

> +	test_config "includeIf.os:$uname_s.path" xyz &&
> +	test z = "$(git config x.y)"

Ditto segfault-hiding.