Re: [PATCH v3] git-send-email: honor $PATH for sendmail binary

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

Florian Klink <flokli@flokli.de> writes:

> This extends git-send-email to also consider sendmail binaries in $PATH
> after checking the (fixed) list of /usr/sbin and /usr/lib, and before
> falling back to localhost.
>
> Signed-off-by: Florian Klink <flokli@flokli.de>
> ---

Thanks for an update.

In an ideal world where we were introducing git-send-email for the
first time without any existing users, we would certainly prefer
things on directories listed in $PATH, and use the two traditional
hardcoded places merely as fallback, but because we do have existing
users who have been relying on the code finding /usr/lib/sendmail
(even when they have something called 'sendmail' that they do not
want to use on their $PATH) and doing that ideal implementation
would break things for them.  Those who have /usr/lib/sendmail
installed that they do not want to use can continue to use
sendemail.smtpserver---if $PATH were searched first, they could
instead list the path that has their faviourite sendmail on it
without setting the configuration, but it does not change the fact
that they need to do _something_ anyway, so it is not too huge a
deal.

>  Documentation/git-send-email.txt | 6 +++---
>  git-send-email.perl              | 4 +++-
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index bac9014ac..44db25567 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -203,9 +203,9 @@ a password is obtained using 'git-credential'.
>  	specify a full pathname of a sendmail-like program instead;
>  	the program must support the `-i` option.  Default value can
>  	be specified by the `sendemail.smtpServer` configuration
> -	option; the built-in default is `/usr/sbin/sendmail` or
> -	`/usr/lib/sendmail` if such program is available, or
> -	`localhost` otherwise.
> +	option; the built-in default is to search for `sendmail` in
> +	`/usr/sbin`, `/usr/lib/sendmail` and $PATH if such program is
> +	available, falling back to `localhost` otherwise.

"search for `sendmail` in `/usr/sbin`, `/usr/lib/sendmail`" would
mean we would not be happy with /usr/lib/sendmail but would be with
either /usr/sbin/sendmail or /usr/lib/sendmail/sendmail, which is
not what you wanted to say.  I'd do 's|/usr/lib/sendmail|/usr/lib|'
while queueing.

Thanks again.

>  --smtp-server-port=<port>::
>  	Specifies a port different from the default port (SMTP
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 2208dcc21..edcc6d346 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -885,7 +885,9 @@ if (defined $initial_reply_to) {
>  }
>  
>  if (!defined $smtp_server) {
> -	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
> +	my @sendmail_paths = qw( /usr/sbin/sendmail /usr/lib/sendmail );
> +	push @sendmail_paths, map {"$_/sendmail"} split /:/, $ENV{PATH};
> +	foreach (@sendmail_paths) {
>  		if (-x $_) {
>  			$smtp_server = $_;
>  			last;