Re: [PATCH v2 1/1] config: learn the "hostname:" includeIf condition
- From
- Ignacio Encinas Rubio <ignacio@iencinas.com>
- Date
- Mar 16, 2024, 16:46 UTC
- Message-ID
- <f20a342c-d6e2-47da-95cf-b61d9dd282c8@iencinas.com>
- In-Reply-To
- <ZfXCJAsACzKLndNm@nand.local>
On 16/3/24 17:00, Taylor Blau wrote:
Show 48 quoted lines
> 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.
I agree it isn't too descriptive, but the reason I chose to do it was because this doesn't seem thoroughly documented anywhere:
hostname(1):
hostname will print the name of the system as returned by the gethostname(2) function.
-s, --short
Display the short host name. This is the host name cut at the first dot.I have only superficial knowledge about the terminology, but from what I have read, it seems like we're actually reading the "nodename" (see uname(2)), which shouldn't but can contain dots ".", which "hostname -s" will trim, but "hostname" won't.
After seeing all this and the huge potential for confusing everybody, I chose the easy way out.
I'm ok with saying "short hostname" but I'm not terribly happy with it as it won't match "hostname -s" if "nodename" has dots (it will always match "hostname" from what I have seen in the hostname source code from the debian package which I assume everyone uses).
Do you think this is worth worrying about? Or people with "nodename"s making "hostname" and "hostname --short" disagree should know that by short hostname we mean "hostname" and not "hostname --short".
I might be missing something, but I somehow find all of this pretty confusing.
Show 7 quoted lines
> 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
Thank you for the review!