diff mbox series

[v3,2/2] config: learn the "hostname:" includeIf condition

Message ID 20240319183722.211300-3-ignacio@iencinas.com (mailing list archive)
State New
Headers show
Series Add hostname condition to includeIf | expand

Commit Message

Ignacio Encinas Rubio March 19, 2024, 6:37 p.m. UTC
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  | 10 ++++++++++
 config.c                  | 17 ++++++++++++++++
 t/t1305-config-include.sh | 42 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+)

Comments

Junio C Hamano March 19, 2024, 8:53 p.m. UTC | #1
Ignacio Encinas <ignacio@iencinas.com> writes:

> Currently, customizing the configuration depending on the machine running
> git has to be done manually.

Drop "currently" (cf. https://lore.kernel.org/git/xmqqle6xbep5.fsf@gitster.g/)

It does not actually have to be done manually.  I and many others
have ~/src/home/dot/ directory where ~/src/home/dot/Makefile uses
information available in the environment (like output from the
`hostname` command), produces the .gitconfig file out of a template,
and the build procedure can even install with "make install" the
resulting file to "~/.gitconfig".  Together with other configuration
files that are kept track of in the ~/src/home/ repository, it is
managed wihtout much manual labor.

Another reason why "[includeif hostname:<name>]" may be useful is
when the same home directory is shared across multiple machines.  
As ~/.gitconfig is shared, if you need to have different settings
depending on the host, you would need to have something that a
single file ~/.gitconfig is read in different ways on these hosts.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index e3a74dd1c19d..268a9fab7be0 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -186,6 +186,12 @@ 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 (output of gethostname(2)) matches the
> +	pattern, the include condition is met.
> +

I do not think of a better way to phrase to explain what `hostname`
means in this context than the above, either.  This should be good
enough, hopefully ;-).

The entry above this one is really an oddball (it only depends on
what other repositories the current repository interacts with, and
does not care about host, directory, or anything of the sort); we
may want to move it either before the `onbranch` entry.

> diff --git a/config.c b/config.c
> index 3cfeb3d8bd99..50b3f6d24c50 100644
> --- a/config.c
> +++ b/config.c
> @@ -317,6 +317,21 @@ 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;
> +}

OK.  Just as other conditions, it is a bit annoying that we need to
make a copy of cond string only to NUL terminate it, because
wildmatch() does not take a counted string as its input, but the
above code looks good.

> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> index 5cde79ef8c4f..ef9272fd8e53 100755
> --- a/t/t1305-config-include.sh
> +++ b/t/t1305-config-include.sh
> @@ -357,4 +357,46 @@ test_expect_success 'include cycles are detected' '
>  	grep "exceeded maximum include depth" stderr
>  '
>  
> +test_expect_success 'conditional include, hostname' '
> +	cat >>.git/config <<-EOF &&
> +	[includeIf "hostname:$(test-tool xgethostname)a"]
> +		path = bar12
> +	EOF

Exactly the same comment about lost exit status from test-tool
applies here, too.

> +	cat >>.git/bar12 <<-EOF &&
> +	[test]
> +		twelve=12
> +	EOF
> +
> +	test_must_fail git config test.twelve &&
> +
> +	cat >>.git/config <<-EOF &&
> +	[includeIf "hostname:$(test-tool xgethostname)"]
> +		path = bar12
> +	EOF
> +	echo 12 >expect &&
> +	git config test.twelve >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'conditional include, hostname, wildcard' '
> +	cat >>.git/config <<-EOF &&
> +	[includeIf "hostname:$(test-tool xgethostname)a*"]

Hmph, a* is not even "one-or-more a" but "a followed by anything",
so this will not match, OK.

> +		path = bar13
> +	EOF
> +	cat >>.git/bar13 <<-EOF &&
> +	[test]
> +		thirteen = 13
> +	EOF
> +
> +	test_must_fail git config test.thirteen &&
> +
> +	cat >>.git/config <<-EOF &&
> +	[includeIf "hostname:$(test-tool xgethostname)*"]

And this is "exactly what gethostname gives, followed by anything
(including nothing)", so gethostname output should match.  OK.

> +		path = bar13
> +	EOF
> +	echo 13 >expect &&
> +	git config test.thirteen >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
Jeff King March 19, 2024, 9:04 p.m. UTC | #2
On Tue, Mar 19, 2024 at 07:37:22PM +0100, Ignacio Encinas wrote:

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

I was going to comment further here, but I see Eric already replied with
everything I was going to say. ;)

One small comment on the patch...

> +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;
> +}

This is absolutely a nit, but I think using xmemdupz() like:

  char *pattern;
  ...

  pattern = xmemdupz(cond, cond_len);
  ...
  free(pattern);

expresses the intent more directly (it's also a little more efficient,
but that's probably not measurable).

-Peff
Ignacio Encinas Rubio March 19, 2024, 9:32 p.m. UTC | #3
On 19/3/24 22:04, Jeff King wrote:
> On Tue, Mar 19, 2024 at 07:37:22PM +0100, Ignacio Encinas wrote:
> 
>> +`hostname`::
>> +	The data that follows the keyword `hostname:` is taken to be a
>> +	pattern with standard globbing wildcards. If the current
>> +	hostname (output of gethostname(2)) matches the
>> +	pattern, the include condition is met.
> 
> I was going to comment further here, but I see Eric already replied with
> everything I was going to say. ;)

Please see my reply there. Thanks for the suggestion and sorry again if 
I sounded rude!

> One small comment on the patch...
> 
>> +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;
>> +}
> 
> This is absolutely a nit, but I think using xmemdupz() like:
> 
>   char *pattern;
>   ...
> 
>   pattern = xmemdupz(cond, cond_len);
>   ...
>   free(pattern);
> 
> expresses the intent more directly (it's also a little more efficient,
> but that's probably not measurable).

Noted, thanks!
 
> -Peff
diff mbox series

Patch

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e3a74dd1c19d..268a9fab7be0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -186,6 +186,12 @@  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 (output of gethostname(2)) matches the
+	pattern, the include condition is met.
+
 A few more notes on matching via `gitdir` and `gitdir/i`:
 
  * Symlinks in `$GIT_DIR` are not resolved before matching.
@@ -261,6 +267,10 @@  Example
 	path = foo.inc
 [remote "origin"]
 	url = https://example.com/git
+
+; include only if the hostname of the machine matches some-hostname
+[includeIf "hostname:some-hostname"]
+	path = foo.inc
 ----
 
 Values
diff --git a/config.c b/config.c
index 3cfeb3d8bd99..50b3f6d24c50 100644
--- a/config.c
+++ b/config.c
@@ -317,6 +317,21 @@  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;
+}
+
 static int add_remote_url(const char *var, const char *value,
 			  const struct config_context *ctx UNUSED, void *data)
 {
@@ -406,6 +421,8 @@  static int include_condition_is_true(const struct key_value_info *kvi,
 	else if (skip_prefix_mem(cond, cond_len, "hasconfig:remote.*.url:", &cond,
 				   &cond_len))
 		return include_by_remote_url(inc, cond, cond_len);
+	else if (skip_prefix_mem(cond, cond_len, "hostname:", &cond, &cond_len))
+		return include_by_hostname(cond, cond_len);
 
 	/* unknown conditionals are always false */
 	return 0;
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 5cde79ef8c4f..ef9272fd8e53 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -357,4 +357,46 @@  test_expect_success 'include cycles are detected' '
 	grep "exceeded maximum include depth" stderr
 '
 
+test_expect_success 'conditional include, hostname' '
+	cat >>.git/config <<-EOF &&
+	[includeIf "hostname:$(test-tool xgethostname)a"]
+		path = bar12
+	EOF
+	cat >>.git/bar12 <<-EOF &&
+	[test]
+		twelve=12
+	EOF
+
+	test_must_fail git config test.twelve &&
+
+	cat >>.git/config <<-EOF &&
+	[includeIf "hostname:$(test-tool xgethostname)"]
+		path = bar12
+	EOF
+	echo 12 >expect &&
+	git config test.twelve >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'conditional include, hostname, wildcard' '
+	cat >>.git/config <<-EOF &&
+	[includeIf "hostname:$(test-tool xgethostname)a*"]
+		path = bar13
+	EOF
+	cat >>.git/bar13 <<-EOF &&
+	[test]
+		thirteen = 13
+	EOF
+
+	test_must_fail git config test.thirteen &&
+
+	cat >>.git/config <<-EOF &&
+	[includeIf "hostname:$(test-tool xgethostname)*"]
+		path = bar13
+	EOF
+	echo 13 >expect &&
+	git config test.thirteen >actual &&
+	test_cmp expect actual
+'
+
 test_done