Re: [RFC PATCH 1/1] config: learn the "hostname:" includeIf condition
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 7, 2024, 21:40 UTC
- Message-ID
- <xmqqil1xk9fv.fsf@gitster.g>
- In-Reply-To
- <20240307205006.467443-2-ignacio@iencinas.com>
Ignacio Encinas <ignacio@iencinas.com> writes:
Show 20 quoted lines
> 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.
Show 5 quoted lines
> +`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.
Show 21 quoted lines
> 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.
Show 12 quoted lines
> 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.