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

[Taylor Blau <me@ttaylorr.com>]

On Sat, Mar 16, 2024 at 12:19:44PM +0100, Ignacio Encinas Rubio wrote:
>
>
> On 16/3/24 7:57, Jeff King wrote:
> > On Sat, Mar 09, 2024 at 07:18:28PM +0100, Ignacio Encinas wrote:
> >
> >> diff --git a/Documentation/config.txt b/Documentation/config.txt
> >> index e3a74dd1c19d..9a22fd260935 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.
> >
> > Do we need to define "hostname" in more detail here? Specifically, I'm
> > wondering whether the result will be a FQDN or not (i.e., the output of
> > "hostname" vs "hostname -f"). Looking at the code I think it will just
> > be the short name returned. That's probably OK, but it may be worth
> > documenting.
>
> Thanks for pointing it out. I agree that it should be further clarified.
>
> Indeed, I was referring to the short name reported by gethostname(2),
> which should agree with "hostname". What do you think about
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 9a22fd260935..268a9fab7be0 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -189,7 +189,8 @@ 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.
> +       hostname (output of gethostname(2)) matches the

Hmm. gethostname(2)'s manual page isn't overly specific on the details
here, either.

I admittedly don't love the idea of documenting this implementation
detail (that is, that we are calling gethostname() and using its output
to compare against). I think it's fine to say instead, "the short
hostname", and leave it at that.

Alternatively, you could say "If the machine's short hostname (as
opposed to a fully-qualified hostname, as returned by `hostname -f`)
matches the pattern [...]".

I think I have a vague preference towards the latter.

Thanks,
Taylor