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

[Junio C Hamano <gitster@pobox.com>]

Aditya Garg <gargaditya08@live.com> writes:

> `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 ...

> 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.