Re: [RFC PATCH 1/1] config: learn the "hostname:" includeIf condition

[Junio C Hamano <gitster@pobox.com>]

Ignacio Encinas <ignacio@iencinas.com> writes:

> Currently, customizing the configuration depending on the machine running
> git has to be done manually.
>
> Add support for a new includeIf keyword "hostname:" to conditionally
> include configuration files depending on the hostname.
>
> Signed-off-by: Ignacio Encinas <ignacio@iencinas.com>
> ---
>  Documentation/config.txt  |  9 +++++++++
>  config.c                  | 16 ++++++++++++++++
>  t/t1305-config-include.sh | 22 ++++++++++++++++++++++
>  3 files changed, 47 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index e3a74dd1c1..9a22fd2609 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -186,6 +186,11 @@ As for the naming of this keyword, it is for forwards compatibility with
>  a naming scheme that supports more variable-based include conditions,
>  but currently Git only supports the exact keyword described above.

> +`hostname`::
> +	The data that follows the keyword `hostname:` is taken to be a
> +	pattern with standard globbing wildcards. If the current
> +	hostname matches the pattern, the include condition is met.
> +

OK.  This seems to copy its phrasing from the existing text for
"gitdir" and "onbranch", which greatly helps the description for
these features consistent.

> diff --git a/config.c b/config.c
> index 3cfeb3d8bd..e0611fc342 100644
> --- a/config.c
> +++ b/config.c
> @@ -317,6 +317,20 @@ static int include_by_branch(const char *cond, size_t cond_len)
>  	return ret;
>  }
>  
> +static int include_by_hostname(const char *cond, size_t cond_len)
> +{
> +	int ret;
> +	char my_host[HOST_NAME_MAX + 1];
> +	struct strbuf pattern = STRBUF_INIT;
> +	if (xgethostname(my_host, sizeof(my_host)))
> +		return 0;
> +
> +	strbuf_add(&pattern, cond, cond_len);
> +	ret = !wildmatch(pattern.buf, my_host, 0);
> +	strbuf_release(&pattern);
> +	return ret;
> +}

Have a blank line between the end of the decl block (our codebase
frowns upon decl-after-statement) and the first statement,
i.e. before "if (xgethostname...".

Otherwise this looks reasonable to me.

> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> index 5cde79ef8c..ee78d9cade 100755
> --- a/t/t1305-config-include.sh
> +++ b/t/t1305-config-include.sh
> @@ -357,4 +357,26 @@ test_expect_success 'include cycles are detected' '
>  	grep "exceeded maximum include depth" stderr
>  '
>  
> +test_expect_success 'conditional include, hostname' '
> +	echo "[includeIf \"hostname:$(hostname)a\"]path=bar12" >>.git/config &&
> +	echo "[test]twelve=12" >.git/bar12 &&
> +	test_must_fail git config test.twelve &&

Emulating other tests in this file that uses here document may make
it a bit easier to read?  E.g.,

	cat >>.gitconfig <<-EOF &&
	[includeIf "hostname:$(hostname)a"]
		path = bar12
	EOF

> +	echo "[includeIf \"hostname:$(hostname)\"]path=bar12" >>.git/config &&

Ditto for the remainder of the patch.

Thanks.