Re: [PATCH v3] git-send-email: honor $PATH for sendmail binary
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 28, 2017, 01:13 UTC
- Message-ID
- <xmqq609v9pdm.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20171128004904.13978-1-flokli@flokli.de>
Florian Klink <flokli@flokli.de> writes:
Show 6 quoted lines
> 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.
Show 18 quoted lines
> 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.
Show 17 quoted lines
> --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;