Re: [PATCH] send-email: try to get fqdn by running hostname --fqdn on Linux and macOS

From: Junio C Hamano <gitster@pobox.com>
Date: May 9, 2025, 20:13 UTC
Message-ID: <xmqqseldzgoh.fsf@gitster.g>
In-Reply-To: <PN3PR01MB9597C419019DC28E489D2AF9B88AA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>

Aditya Garg <gargaditya08@live.com> writes:

Show 22 quoted lines

> `hostname` is a popular command available on both Linux and macOS. As
> per the man-page[1], `hostname --fqdn` command returns the fully
> qualified domain name (FQDN) of the system. The current Net::Domain
> perl module being used in the script for the same has been quite
> unrealiable in many cases. Thankfully, we now have a better check for
> valid_fqdn, which does reject the invalid FQDNs given by this module
> properly, but at the same time, it will result in a fallback to
> 'localhost.localdomain' being used. `hostname --fqdn` has been quite
> reliable (probably even more reliable than the Net::Domain module) and
> before falling back to 'localhost.localdomain', we should try to use it.
> Interestingly, the `hostname` command is actually used by perl modules
> like Net::Domain[2] and Sys::Hostname[3] to get the hostname. So, lets
> give `hostname --fqdn` a chance as well!
>
> [1]: https://man7.org/linux/man-pages/man1/hostname.1.html
> [2]: https://github.com/Perl/perl5/blob/blead/cpan/libnet/lib/Net/Domain.pm#L88
> [3]: https://github.com/Perl/perl5/blob/blead/ext/Sys-Hostname/Hostname.pm#L93
>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
>  git-send-email.perl | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

As maildomain() is called at most once in a process, thanks to send_message() conditionally calling it only to set $smtp_domain that is not yet set, I do not personally mind adding an extra fork/exec here, but ...

Show 15 quoted lines

> diff --git a/git-send-email.perl b/git-send-email.perl
> index 55b7e00d29..735d8abc12 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1393,8 +1393,20 @@ sub maildomain_mta {
>  	return $maildomain;
>  }
>  
> +sub maildomain_hostname_command {
> +	my $maildomain;
> +
> +	if ($^O eq 'linux' || $^O eq 'darwin') {
> +		my $domain = `(hostname --fqdn) 2>/dev/null`;
> +		chomp($domain);
> +		$maildomain = $domain if valid_fqdn($domain);

... we do not know everybody's implementation, especially including the non stardard ones, of 'hostname --fqdn'. Some may stay silent, or say something only to its standard error, when it cannot produce a usable output, which is the above code expects, but some others emit whatever it wants to to its standard output while signalling an error with its exit value, when it sees some error (like "I do not know about that 'fqdn' option").

In short, I do not have too much trouble against the idea to add "ask hostname(1)" to the source of maildomain information, but I'd prefer for the implementation to be a bit more careful to detect errors, more careful than "if we get anything on its standard output, it cannot be an error and we'd use that". I understand that the call to "if valid_fqdn()" tightens the condition a bit better by looking at $domain, but we shouldn't be even chomping $domain or feeding it to valid_fqdn() when we know the `hostname` failed in the first place.

> +	}
> +	return $maildomain;
> +}

Thanks.