{"thread":{"id":"47232","subject":"[PATCH] git-send-email: honor $PATH","startedAt":"2017-11-18T12:52:14Z","lastAt":"2017-11-28T01:13:51Z","messageCount":9,"participants":["Florian Klink","brian m. carlson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"332800","messageId":"20171118124249.26479-1-flokli@flokli.de","threadId":"47232","inReplyTo":null,"subject":"[PATCH] git-send-email: honor $PATH","fromName":"Florian Klink","fromEmail":"flokli@flokli.de","sentAt":"2017-11-18T12:42:49Z","receivedAt":"2017-11-18T12:52:14Z","isPatch":true,"sender":{"key":"flokli@flokli.de","avatar":"https://avatars.githubusercontent.com/u/183879?v=4"},"body":"This will search $PATH for a sendmail binary, instead of the (previously\nfixed) list of paths.\n\nSigned-off-by: Florian Klink \n---\n Documentation/git-send-email.txt | 5 ++---\n git-send-email.perl | 3 ++-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex bac9014ac..b9b1f2c41 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -203,9 +203,8 @@ a password is obtained using 'git-credential'.\n \tspecify a full pathname of a sendmail-like program instead;\n \tthe program must support the `-i` option. Default value can\n \tbe specified by the `sendemail.smtpServer` configuration\n-\toption; the built-in default is `/usr/sbin/sendmail` or\n-\t`/usr/lib/sendmail` if such program is available, or\n-\t`localhost` otherwise.\n+\toption; the built-in default is to search in $PATH if such program is\n+\tavailable, or `localhost` otherwise.\n \n --smtp-server-port=::\n \tSpecifies a port different from the default port (SMTP\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2208dcc21..8e357aeab 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -885,7 +885,8 @@ if (defined $initial_reply_to) {\n }\n \n if (!defined $smtp_server) {\n-\tforeach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {\n+\tmy @sendmail_paths = map {\"$_/sendmail\"} split /:/, $ENV{PATH};\n+\tforeach (@sendmail_paths) {\n \t\tif (-x $_) {\n \t\t\t$smtp_server = $_;\n \t\t\tlast;\n-- \n2.15.0\n\n"},{"id":"332823","messageId":"20171118210109.ttp7bnihi3xb4xvs@genre.crustytoothpaste.net","threadId":"47232","inReplyTo":"20171118124249.26479-1-flokli@flokli.de","subject":"Re: [PATCH] git-send-email: honor $PATH","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-11-18T21:01:09Z","receivedAt":"2017-11-18T21:09:58Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sat, Nov 18, 2017 at 01:42:49PM +0100, Florian Klink wrote:\n> This will search $PATH for a sendmail binary, instead of the (previously\n> fixed) list of paths.\n> \n> Signed-off-by: Florian Klink \n> ---\n> Documentation/git-send-email.txt | 5 ++---\n> git-send-email.perl | 3 ++-\n> 2 files changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index bac9014ac..b9b1f2c41 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -203,9 +203,8 @@ a password is obtained using 'git-credential'.\n> \tspecify a full pathname of a sendmail-like program instead;\n> \tthe program must support the `-i` option. Default value can\n> \tbe specified by the `sendemail.smtpServer` configuration\n> -\toption; the built-in default is `/usr/sbin/sendmail` or\n> -\t`/usr/lib/sendmail` if such program is available, or\n> -\t`localhost` otherwise.\n> +\toption; the built-in default is to search in $PATH if such program is\n> +\tavailable, or `localhost` otherwise.\n\nThis patch adds support for PATH, but it also removes the fixed paths.\nOn many systems, unprivileged users don't have /usr/sbin in their PATH,\nand I know of no systems which provide /usr/lib as a PATH value.\nTherefore, it's possible that this change will break automatic detection\nof sendmail for many users.\n\nI think what you probably want to do is use entries in PATH first, and\nleave the two old values as backups at the end.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"332824","messageId":"20171118212833.qwcvt2divpi4smtl@tp.flokli.de","threadId":"47232","inReplyTo":"20171118210109.ttp7bnihi3xb4xvs@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-send-email: honor $PATH","fromName":"Florian Klink","fromEmail":"flokli@flokli.de","sentAt":"2017-11-18T21:28:33Z","receivedAt":"2017-11-18T21:29:23Z","isPatch":true,"sender":{"key":"flokli@flokli.de","avatar":"https://avatars.githubusercontent.com/u/183879?v=4"},"body":">This patch adds support for PATH, but it also removes the fixed paths.\n>On many systems, unprivileged users don't have /usr/sbin in their PATH,\n>and I know of no systems which provide /usr/lib as a PATH value.\n>Therefore, it's possible that this change will break automatic detection\n>of sendmail for many users.\n>\n>I think what you probably want to do is use entries in PATH first, and\n>leave the two old values as backups at the end.\n\nYou're perfectly right - forgot about /usr/sbin not in PATH for unprivileged\nusers. Will send a new patch which appends to the original list.\n"},{"id":"332826","messageId":"20171118222012.23137-1-flokli@flokli.de","threadId":"47232","inReplyTo":"20171118212833.qwcvt2divpi4smtl@tp.flokli.de","subject":"[PATCH v2] git-send-email: honor $PATH for sendmail binary","fromName":"Florian Klink","fromEmail":"flokli@flokli.de","sentAt":"2017-11-18T22:20:12Z","receivedAt":"2017-11-18T22:20:25Z","isPatch":true,"sender":{"key":"flokli@flokli.de","avatar":"https://avatars.githubusercontent.com/u/183879?v=4"},"body":"This extends git-send-email to also consider sendmail binaries in $PATH,\nin addition to the (fixed) list of /usr/sbin and /usr/lib.fixed) list of\npaths.\n\nSigned-off-by: Florian Klink \n---\n Documentation/git-send-email.txt | 6 +++---\n git-send-email.perl | 4 +++-\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex bac9014ac..7af48f8eb 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -203,9 +203,9 @@ a password is obtained using 'git-credential'.\n \tspecify a full pathname of a sendmail-like program instead;\n \tthe program must support the `-i` option. Default value can\n \tbe specified by the `sendemail.smtpServer` configuration\n-\toption; the built-in default is `/usr/sbin/sendmail` or\n-\t`/usr/lib/sendmail` if such program is available, or\n-\t`localhost` otherwise.\n+\toption; the built-in default is to search in $PATH,\n+\tthen /usr/sbin and /usr/lib/sendmail afterwards if such program\n+\tis available, falling back to `localhost` otherwise.\n \n --smtp-server-port=::\n \tSpecifies a port different from the default port (SMTP\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2208dcc21..570f04079 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -885,7 +885,9 @@ if (defined $initial_reply_to) {\n }\n \n if (!defined $smtp_server) {\n-\tforeach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {\n+\tmy @sendmail_paths = map {\"$_/sendmail\"} split /:/, $ENV{PATH};\n+\tpush @sendmail_paths, qw( /usr/sbin/sendmail /usr/lib/sendmail );\n+\tforeach (@sendmail_paths) {\n \t\tif (-x $_) {\n \t\t\t$smtp_server = $_;\n \t\t\tlast;\n-- \n2.15.0\n\n"},{"id":"332835","messageId":"xmqqd14foz7p.fsf@gitster.mtv.corp.google.com","threadId":"47232","inReplyTo":"20171118210109.ttp7bnihi3xb4xvs@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-send-email: honor $PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-19T01:04:58Z","receivedAt":"2017-11-19T01:05:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> This patch adds support for PATH, but it also removes the fixed paths.\n> On many systems, unprivileged users don't have /usr/sbin in their PATH,\n> and I know of no systems which provide /usr/lib as a PATH value.\n> Therefore, it's possible that this change will break automatic detection\n> of sendmail for many users.\n\nIt is more than possible ;-) that this change alone is a regression.\n\n> I think what you probably want to do is use entries in PATH first, and\n> leave the two old values as backups at the end.\n\nI do not think it would make things worse if the change were to do\nthe two standard places first and then try elements on the $PATH;\nsplit of $PATH needs to be done carefully (Windows?), though. \n\nI would feel a lot more worried about trying elements on the $PATH\nfirst and then using the two standard places as fallback. If the\norder of addition matters at all, that would mean that trying\nelements on $PATH first and then falling back to the two standard\nplaces *will* change the behaviour---for the affected users, we used\nto pick one of these two, but now we would pick something different.\nsendmail is usually installed out of the way of $PATH for regular\nusers for a reason, so picking anything whose name happens to be\nsendmail that is on $PATH does not sound right.\n\nOf course, for users who do not have sendmail at one of the two\nstandard places _and_ has one on one of the directories on $PATH,\nthe order in which we check would not make a difference, so my\nsuggestion would be to do the other way around.\n"},{"id":"332846","messageId":"20171119123511.pck5aqak4yyc6367@tp.flokli.de","threadId":"47232","inReplyTo":"xmqqd14foz7p.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-send-email: honor $PATH","fromName":"Florian Klink","fromEmail":"flokli@flokli.de","sentAt":"2017-11-19T12:35:11Z","receivedAt":"2017-11-19T12:35:20Z","isPatch":true,"sender":{"key":"flokli@flokli.de","avatar":"https://avatars.githubusercontent.com/u/183879?v=4"},"body":"On 17-11-19 10:04:58, Junio C Hamano wrote:\n>\"brian m. carlson\" writes:\n>\n>> This patch adds support for PATH, but it also removes the fixed paths.\n>> On many systems, unprivileged users don't have /usr/sbin in their PATH,\n>> and I know of no systems which provide /usr/lib as a PATH value.\n>> Therefore, it's possible that this change will break automatic detection\n>> of sendmail for many users.\n>\n>It is more than possible ;-) that this change alone is a regression.\n>\n>> I think what you probably want to do is use entries in PATH first, and\n>> leave the two old values as backups at the end.\n>\n>I do not think it would make things worse if the change were to do\n>the two standard places first and then try elements on the $PATH;\n>split of $PATH needs to be done carefully (Windows?), though.\n\nSupport to detect sendmail binaries in windows' PATH seems a bit more complex.\nThe separator is different, and PATHEXT would need to be considered too. I'm\nnot even sure if having a sendmail binary in PATH on windows is something usual\nor if defaulting to smtp to localhost (what we currently do) is good enough (tm).\nIf we want to start parsing PATH under windows too, I'd suggest to use\nFile::Which instead of implementing it on our own.\n\n>I would feel a lot more worried about trying elements on the $PATH\n>first and then using the two standard places as fallback. If the\n>order of addition matters at all, that would mean that trying\n>elements on $PATH first and then falling back to the two standard\n>places *will* change the behaviour---for the affected users, we used\n>to pick one of these two, but now we would pick something different.\n>sendmail is usually installed out of the way of $PATH for regular\n>users for a reason, so picking anything whose name happens to be\n>sendmail that is on $PATH does not sound right.\n>\n>Of course, for users who do not have sendmail at one of the two\n>standard places _and_ has one on one of the directories on $PATH,\n>the order in which we check would not make a difference, so my\n>suggestion would be to do the other way around.\n\nI could happily provide a patch that does it the other way round, too. But let's\nfirst decide on what to do with windows ;-)\n\nCheers,\nFlorian\n"},{"id":"333659","messageId":"20171128003235.vbu3afu5junfebg7@tp.flokli.de","threadId":"47232","inReplyTo":"20171119123511.pck5aqak4yyc6367@tp.flokli.de","subject":"Re: [PATCH] git-send-email: honor $PATH","fromName":"Florian Klink","fromEmail":"flokli@flokli.de","sentAt":"2017-11-28T00:32:35Z","receivedAt":"2017-11-28T00:32:53Z","isPatch":true,"sender":{"key":"flokli@flokli.de","avatar":"https://avatars.githubusercontent.com/u/183879?v=4"},"body":">Support to detect sendmail binaries in windows' PATH seems a bit more complex.\n>The separator is different, and PATHEXT would need to be considered too. I'm\n>not even sure if having a sendmail binary in PATH on windows is something usual\n>or if defaulting to smtp to localhost (what we currently do) is good enough (tm).\n>If we want to start parsing PATH under windows too, I'd suggest to use\n>File::Which instead of implementing it on our own.\n>\n>>I would feel a lot more worried about trying elements on the $PATH\n>>first and then using the two standard places as fallback. If the\n>>order of addition matters at all, that would mean that trying\n>>elements on $PATH first and then falling back to the two standard\n>>places *will* change the behaviour---for the affected users, we used\n>>to pick one of these two, but now we would pick something different.\n>>sendmail is usually installed out of the way of $PATH for regular\n>>users for a reason, so picking anything whose name happens to be\n>>sendmail that is on $PATH does not sound right.\n>>\n>>Of course, for users who do not have sendmail at one of the two\n>>standard places _and_ has one on one of the directories on $PATH,\n>>the order in which we check would not make a difference, so my\n>>suggestion would be to do the other way around.\n>\n>I could happily provide a patch that does it the other way round, too. But let's\n>first decide on what to do with windows ;-)\n\nSeems like there is not really much of motivation to try better in detecting\nsendmail binaries in PATH on windows ;-)\n\nWill send patch v3, which reverses the order as suggested by Junio shortly.\n\n-- \nFlorian\n"},{"id":"333661","messageId":"20171128004904.13978-1-flokli@flokli.de","threadId":"47232","inReplyTo":"20171119123511.pck5aqak4yyc6367@tp.flokli.de","subject":"[PATCH v3] git-send-email: honor $PATH for sendmail binary","fromName":"Florian Klink","fromEmail":"flokli@flokli.de","sentAt":"2017-11-28T00:49:04Z","receivedAt":"2017-11-28T00:49:32Z","isPatch":true,"sender":{"key":"flokli@flokli.de","avatar":"https://avatars.githubusercontent.com/u/183879?v=4"},"body":"This extends git-send-email to also consider sendmail binaries in $PATH\nafter checking the (fixed) list of /usr/sbin and /usr/lib, and before\nfalling back to localhost.\n\nSigned-off-by: Florian Klink \n---\n Documentation/git-send-email.txt | 6 +++---\n git-send-email.perl | 4 +++-\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex bac9014ac..44db25567 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -203,9 +203,9 @@ a password is obtained using 'git-credential'.\n \tspecify a full pathname of a sendmail-like program instead;\n \tthe program must support the `-i` option. Default value can\n \tbe specified by the `sendemail.smtpServer` configuration\n-\toption; the built-in default is `/usr/sbin/sendmail` or\n-\t`/usr/lib/sendmail` if such program is available, or\n-\t`localhost` otherwise.\n+\toption; the built-in default is to search for `sendmail` in\n+\t`/usr/sbin`, `/usr/lib/sendmail` and $PATH if such program is\n+\tavailable, falling back to `localhost` otherwise.\n \n --smtp-server-port=::\n \tSpecifies a port different from the default port (SMTP\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2208dcc21..edcc6d346 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -885,7 +885,9 @@ if (defined $initial_reply_to) {\n }\n \n if (!defined $smtp_server) {\n-\tforeach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {\n+\tmy @sendmail_paths = qw( /usr/sbin/sendmail /usr/lib/sendmail );\n+\tpush @sendmail_paths, map {\"$_/sendmail\"} split /:/, $ENV{PATH};\n+\tforeach (@sendmail_paths) {\n \t\tif (-x $_) {\n \t\t\t$smtp_server = $_;\n \t\t\tlast;\n-- \n2.15.0\n\n"},{"id":"333663","messageId":"xmqq609v9pdm.fsf@gitster.mtv.corp.google.com","threadId":"47232","inReplyTo":"20171128004904.13978-1-flokli@flokli.de","subject":"Re: [PATCH v3] git-send-email: honor $PATH for sendmail binary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-28T01:13:41Z","receivedAt":"2017-11-28T01:13:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Klink writes:\n\n> This extends git-send-email to also consider sendmail binaries in $PATH\n> after checking the (fixed) list of /usr/sbin and /usr/lib, and before\n> falling back to localhost.\n>\n> Signed-off-by: Florian Klink \n> ---\n\nThanks for an update.\n\nIn an ideal world where we were introducing git-send-email for the\nfirst time without any existing users, we would certainly prefer\nthings on directories listed in $PATH, and use the two traditional\nhardcoded places merely as fallback, but because we do have existing\nusers who have been relying on the code finding /usr/lib/sendmail\n(even when they have something called 'sendmail' that they do not\nwant to use on their $PATH) and doing that ideal implementation\nwould break things for them. Those who have /usr/lib/sendmail\ninstalled that they do not want to use can continue to use\nsendemail.smtpserver---if $PATH were searched first, they could\ninstead list the path that has their faviourite sendmail on it\nwithout setting the configuration, but it does not change the fact\nthat they need to do _something_ anyway, so it is not too huge a\ndeal.\n\n> Documentation/git-send-email.txt | 6 +++---\n> git-send-email.perl | 4 +++-\n> 2 files changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index bac9014ac..44db25567 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -203,9 +203,9 @@ a password is obtained using 'git-credential'.\n> \tspecify a full pathname of a sendmail-like program instead;\n> \tthe program must support the `-i` option. Default value can\n> \tbe specified by the `sendemail.smtpServer` configuration\n> -\toption; the built-in default is `/usr/sbin/sendmail` or\n> -\t`/usr/lib/sendmail` if such program is available, or\n> -\t`localhost` otherwise.\n> +\toption; the built-in default is to search for `sendmail` in\n> +\t`/usr/sbin`, `/usr/lib/sendmail` and $PATH if such program is\n> +\tavailable, falling back to `localhost` otherwise.\n\n\"search for `sendmail` in `/usr/sbin`, `/usr/lib/sendmail`\" would\nmean we would not be happy with /usr/lib/sendmail but would be with\neither /usr/sbin/sendmail or /usr/lib/sendmail/sendmail, which is\nnot what you wanted to say. I'd do 's|/usr/lib/sendmail|/usr/lib|'\nwhile queueing.\n\nThanks again.\n\n> --smtp-server-port=::\n> \tSpecifies a port different from the default port (SMTP\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 2208dcc21..edcc6d346 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -885,7 +885,9 @@ if (defined $initial_reply_to) {\n> }\n> \n> if (!defined $smtp_server) {\n> -\tforeach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {\n> +\tmy @sendmail_paths = qw( /usr/sbin/sendmail /usr/lib/sendmail );\n> +\tpush @sendmail_paths, map {\"$_/sendmail\"} split /:/, $ENV{PATH};\n> +\tforeach (@sendmail_paths) {\n> \t\tif (-x $_) {\n> \t\t\t$smtp_server = $_;\n> \t\t\tlast;\n"}]}