| Message ID | 20210512033039.4022-1-greg@gpanders.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-send-email: add sendmailCommand option | expand |
Gregory Anders <greg@gpanders.com> writes: > The sendemail.smtpServer option currently supports using a sendmail-like > program to send emails by specifying an absolute file path. However, That is not wrong per-se, but it is not limited to the configuration variable, but is a shared trait with --smtp-server command line option. It is easier on the readers to mention both. Our problem description talks about the status quo in the present tense. No noiseword "currently " necessary. I.e. something along this line: The sendemail.smtpServer configuration variable (and the "--smtp-server" command line option of "git send-email" command) allows to name a command to run to send emails by specifying an absolute path name. However, > this is not ideal for the following reasons: > > 1. It overloads the meaning of smtpServer (now a program is being used > for the server?) > 2. It doesn't allow for non-absolute paths, arguments, or arbitrary > scripting. > > Requiring an absolute path is bad for portability, as the same > program may be in different locations on different systems. If I wish > to pass arguments to my program, I have to use the smtpServerOption > option, which is cumbersome (as it must be repeated for each option) > and doesn't adhere to normal git conventions. Up to here, nice explanation of the background and description of the problem being solved. > This patch attempts to solve these problems by introducing a new > configuration option sendemail.sendmailCommand as well as a command line > option --sendmail-cmd. The value of this option is invoked with the > standard sendmail options passed as arguments. When presenting a potential solution, in the history of this project, we'd talk as if we are giving an order to the codebase to "be like so". Introduce a command line option '--sendmail-cmd' and a configuration variable sendemail.sendmailCommand that can be used to specify the command line (possibly including its command line options) to send pieces of e-mail. This is invoked while honoring $PATH, so it does not have to be named with an absolute path to the command. Give it a higher precedence over --smtp-server (and sendemail.smtpServer), as the new interface is more flexible. > sendmailCommand has precedence over smtpServer. If both options are > unspecified, the default is to search for 'sendmail' in /usr/sbin, > /usr/lib, and $PATH. If not found, smtpServer is set to localhost. This > mimics the current behavior when smtpServer is unspecified. I do not think "If both options are unspecified" and everything after it is needed. > The option is passed to Perl's `exec()` function, which automatically > determines whether or not to invoke a shell. If shell metacharacters are > detected, then a shell is used; otherwise, the command is invoked > directly. I do not think this, and the two examples below (omitted), are relevant, either. The "metacharacters make the command diverted to shell" is a mere optimization and not of interest to the end users. Even if sendemail.sendmailcommand is set to just a single word 'msmtp', which does not have any metacharacter, we _could_ spawn it via the shell and the observable end result would be the same as if the single word was directly executed without the shell. > This change deprecates the use of absolute paths in > sendemail.smtpServer, although support is kept for backward > compatibility. I am on the fence about saying this. We may eventually want to deprecate, but until we start issuing a warning when the absolute-path form is used, I'd rather not to call it "deprecated" in either the proposed log message or in the documenation. > --- Missing sign-off. > > Note that this patch is incompatible with (and supersedes) the patch > discussed here: > > https://public-inbox.org/git/YJs2RceLliGHI5TX@gpanders.com/T/#t Thanks---such a note is very valuable. > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt > index 93708aefea..d9fe8cb7c0 100644 > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -159,13 +159,23 @@ Sending > ~~~~~~~ > > --envelope-sender=<address>:: > - Specify the envelope sender used to send the emails. > - This is useful if your default address is not the address that is > - subscribed to a list. In order to use the 'From' address, set the > - value to "auto". If you use the sendmail binary, you must have > - suitable privileges for the -f parameter. Default is the value of the > - `sendemail.envelopeSender` configuration variable; if that is > - unspecified, choosing the envelope sender is left to your MTA. > + Specify the envelope sender used to send the emails. This is > + useful if your default address is not the address that is > + subscribed to a list. In order to use the 'From' address, set > + the value to "auto". If you use the sendmail binary, you must > + have suitable privileges for the -f parameter. Default is the > + value of the `sendemail.envelopeSender` configuration variable; > + if that is unspecified, choosing the envelope sender is left to > + your MTA. Is this a totally unwarranted rewrapping of an unrelated part of the same document, or was there some words or phrases in this description of the envelope-sender option that needed to be adjusted for the introduction of sendmail-cmd option? > +--sendmail-cmd=<command>:: > + Specify a command to run to send the email. The command should > + be compatible with `sendmail` as the arguments are passed > + directly. The command will be executed in the shell if > + necessary. Default is the value of `sendemail.sendmailCommand`. > + If unspecified, and if --smtp-server is also unspecified, > + git-send-email will search for `sendmail` in `/usr/sbin`, > + `/usr/lib` and $PATH if such a program is available. OK, but doesn't this also need to support '-i'? > @@ -211,13 +221,14 @@ a password is obtained using 'git-credential'. > > --smtp-server=<host>:: > If set, specifies the outgoing SMTP server to use (e.g. > - `smtp.example.com` or a raw IP address). Alternatively it can > - 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 to search for `sendmail` in > - `/usr/sbin`, `/usr/lib` and $PATH if such program is > - available, falling back to `localhost` otherwise. > + `smtp.example.com` or a raw IP address). If unspecified, and if > + `--sendmail-cmd` is also unspecified, the default is to search > + for `sendmail` in `/usr/sbin`, `/usr/lib` and $PATH if such a > + program is available, falling back to `localhost` otherwise. > + > + For backward compatibility, this option can also specify a full > + pathname of a sendmail-like program instead; the program must > + support the `-i` option. Prefer using `--sendmail-cmd` instead. Drop the last sentence, if we are not going to explain why. Or perhaps: ... an absolute path to a program that behaves like `sendmail` (among other things, it must support the `-i` option). As you only can specify the path to the program and cannot give any leading arguments to it, consider using `--sendmail-cmd` instead. > @@ -1490,14 +1497,15 @@ sub send_message { > > unshift (@sendmail_parameters, @smtp_server_options); > > + if (file_name_is_absolute($smtp_server)) { > + # Preserved for backward compatibility > + $sendmail_command ||= $smtp_server; > + } Hmph, I wonder if this makes the intent more clear. if (!defined $sendmail_command && file_name_is_absolute($smtp_server)) { $sendmail_command = $smtp_server; } That is, if the user gave us the command in newer form, we do not even have to bother checking if the server is given as an absolute pathname. > @@ -1069,7 +1069,7 @@ test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' ' > git send-email \ > --from="Example <nobody@example.com>" \ > --to=nobody@example.com \ > - --smtp-server="$(pwd)/fake.sendmail" \ > + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ > outdir/*.patch && > grep "^ " msgtxt1 | > grep "=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= <utf8@example.com>" You seem to have replaced every smtp-server="$(pwd)/ mechanically with sendmai-cmd=\"$(pwd)/, but please make sure that we have at least one test left that passes an absolute path to --smtp-server to ensure that the old mechanism keeps working. A bonus point for marking such a test that needs to be adjusted when the actual deprecation happens (i.e. we'd likely to detect the use of absolute path and throw a warning, so the test should notice the warning message). Also you would want to tweak some of the --sendmail-cmd variants to use just the command name, with and without args, to ensure that (1) discovery on $PATH works, and (2) passing initial args works. Thanks.
Gregory Anders wrote: > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -159,13 +159,23 @@ Sending > ~~~~~~~ > > --envelope-sender=<address>:: > - Specify the envelope sender used to send the emails. > - This is useful if your default address is not the address that is > - subscribed to a list. In order to use the 'From' address, set the > - value to "auto". If you use the sendmail binary, you must have > - suitable privileges for the -f parameter. Default is the value of the > - `sendemail.envelopeSender` configuration variable; if that is > - unspecified, choosing the envelope sender is left to your MTA. > + Specify the envelope sender used to send the emails. This is > + useful if your default address is not the address that is > + subscribed to a list. In order to use the 'From' address, set > + the value to "auto". If you use the sendmail binary, you must > + have suitable privileges for the -f parameter. Default is the > + value of the `sendemail.envelopeSender` configuration variable; > + if that is unspecified, choosing the envelope sender is left to > + your MTA. I'm not against these kinds of changes but it took me one minute to figure out all you did was change the format. This belongs in a separate patch. > +--sendmail-cmd=<command>:: Oh no no no. Don't do shortcuts. If you think --sendmail-command is too long, then address that problem head on, don't try to hide it. I do think it's too long, which is why I suggested --command (especially since it's obvious which command we are talking about), but I wouldn't suggest --sdm-command, or something of that sort. We have to own our decisions. 1. --command 2. --sendmail 3. --sendmail-command We have to pick one. I suggest #1. To try to make #3 shorter is just shoving the problem under the rug. > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -70,6 +70,7 @@ sub usage { > > Sending: > --envelope-sender <str> * Email envelope sender. > + --sendmail-cmd <str> * Shell command to run to send email. > --smtp-server <str:int> * Outgoing SMTP server to use. The port > is optional. Default 'localhost'. > --smtp-server-option <str> * Outgoing SMTP server option to use. > @@ -252,6 +253,7 @@ sub do_edit { > my (@suppress_cc); > my ($auto_8bit_encoding); > my ($compose_encoding); > +my ($sendmail_command); > # Variables with corresponding config settings & hardcoded defaults > my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() > my $thread = 1; > @@ -299,6 +301,7 @@ sub do_edit { > "assume8bitencoding" => \$auto_8bit_encoding, > "composeencoding" => \$compose_encoding, > "transferencoding" => \$target_xfer_encoding, > + "sendmailcommand" => \$sendmail_command, > ); > > my %config_path_settings = ( > @@ -432,6 +435,7 @@ sub read_config { > "no-bcc" => \$no_bcc, > "chain-reply-to!" => \$chain_reply_to, > "no-chain-reply-to" => sub {$chain_reply_to = 0}, > + "sendmail-cmd=s" => \$sendmail_command, Isn't it interesting that to make the code readable you picked $sendmail_command, but you don't want users to type so much, even if it's more readable? Once again: "$command=s" -> \$command, > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -57,7 +57,7 @@ test_no_confirm () { > git send-email \ > --from="Example <from@example.com>" \ > --to=nobody@example.com \ > - --smtp-server="$(pwd)/fake.sendmail" \ > + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ People are already using --smpt-server=$cmd, we need to keep testing that. Yes, eventually we would want them to move to --sendmail-cmd (or --command, or whatever), but that won't happen tomorrow. Therefore our primary tests need to be focused on --smtp-server. We need new *additional* tests for --sendmail-cmd, but those should not override the current tests. At least not right now. Cheers.
On Tue, May 11 2021, Gregory Anders wrote: > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt > index 93708aefea..d9fe8cb7c0 100644 > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -159,13 +159,23 @@ Sending > ~~~~~~~ > > --envelope-sender=<address>:: > - Specify the envelope sender used to send the emails. > - This is useful if your default address is not the address that is > - subscribed to a list. In order to use the 'From' address, set the > - value to "auto". If you use the sendmail binary, you must have > - suitable privileges for the -f parameter. Default is the value of the > - `sendemail.envelopeSender` configuration variable; if that is > - unspecified, choosing the envelope sender is left to your MTA. > + Specify the envelope sender used to send the emails. This is > + useful if your default address is not the address that is > + subscribed to a list. In order to use the 'From' address, set > + the value to "auto". If you use the sendmail binary, you must > + have suitable privileges for the -f parameter. Default is the > + value of the `sendemail.envelopeSender` configuration variable; > + if that is unspecified, choosing the envelope sender is left to > + your MTA. Please don't include word-wrapping for unrelated changes in the main patch. > - $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug* > + > + if (!defined $sendmail_command) { > + $smtp_server = 'localhost'; # could be 127.0.0.1, too... *shrug* > + } > } This "let's not accept a 0" change seems unrelated & should be split into a prep cleanup / refactoring patch. On the one hand it's sensible, on the other nobody cares about having a command named "0" in their path (or a hostname), so I think it's fine to have the ||= Perl idiom leak out here. But also, this just seems like confusing logic. Per your docs "your sendmailCommand has precedence over smtpServer.". Why not make this "if not $sendmail_command" part of the top-level check here (the if this one is nested under), which is only done if $smtp_sever is not defined, if $sendmail_command is defined we don't care about $smtp_server later on, no? > if ($compose && $compose > 0) { > @@ -1490,14 +1497,15 @@ sub send_message { > > unshift (@sendmail_parameters, @smtp_server_options); > > + if (file_name_is_absolute($smtp_server)) { > + # Preserved for backward compatibility > + $sendmail_command ||= $smtp_server; > + } > + > if ($dry_run) { > # We don't want to send the email. > - } elsif (file_name_is_absolute($smtp_server)) { > - my $pid = open my $sm, '|-'; > - defined $pid or die $!; > - if (!$pid) { > - exec($smtp_server, @sendmail_parameters) or die $!; > - } > + } elsif (defined $sendmail_command) { > + open my $sm, '|-', "$sendmail_command @sendmail_parameters"; Can we really not avoid moving from exec-as-list so Perl quotes everything, to doing our own interpolation here? It looks like the tests don't check arguments with whitespace (which should fail with this change). > print $sm "$header\n$message"; > close $sm or die $!; > } else { I've just skimmed the previous thread, so forgive me if this was brought up. I for one would be fine with just using --smtp-server and not adding an --sendmail-command, and doing this by simply doing an exec() on whatever the user specifies. If it's an absolute path and an executable command, OK. If it's a command name we find in $PATH, OK, or other valid shell whatever. You can use $? to distinguish between a failed and nonexisting command. If not exec() will return and we continue resolving it as a hostname/IP address/whatever. We'll have a conflict if someone has a command in their $PATH called gmail.com or whatever, but really. Who does that? Maybe it's way too nasty. This design is also fine, just a suggestion. > @@ -1592,14 +1600,14 @@ sub send_message { > printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject); > } else { > print($dry_run ? __("Dry-OK. Log says:\n") : __("OK. Log says:\n")); > - if (!file_name_is_absolute($smtp_server)) { > + if (defined $sendmail_command) { > + print "Sendmail: $sendmail_command ".join(' ',@sendmail_parameters)."\n"; > + } else { > print "Server: $smtp_server\n"; > print "MAIL FROM:<$raw_from>\n"; > foreach my $entry (@recipients) { > print "RCPT TO:<$entry>\n"; > } > - } else { > - print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n"; > } > print $header, "\n"; > if ($smtp) { Minor nit: Let's just continue to use "if (!" here to keep the diff minimal or split up such refactoring into another change...
On Wed, 12 May 2021 13:19 +0900, Junio C Hamano wrote: >Is this a totally unwarranted rewrapping of an unrelated part of the >same document, or was there some words or phrases in this >description of the envelope-sender option that needed to be adjusted >for the introduction of sendmail-cmd option? Yes it's just a rewrapping that I did while adding my new paragraph. The other reviewers pointed this out as well. My mistake, I will remove this from the next patch revision. >> +--sendmail-cmd=<command>:: >> + Specify a command to run to send the email. The command should >> + be compatible with `sendmail` as the arguments are passed >> + directly. The command will be executed in the shell if >> + necessary. Default is the value of `sendemail.sendmailCommand`. >> + If unspecified, and if --smtp-server is also unspecified, >> + git-send-email will search for `sendmail` in `/usr/sbin`, >> + `/usr/lib` and $PATH if such a program is available. >OK, but doesn't this also need to support '-i'? 'The command should be compatible with `sendmail`' was meant to imply this, though I can make this more explicit. >> @@ -211,13 +221,14 @@ a password is obtained using 'git-credential'. >> >> --smtp-server=<host>:: >> If set, specifies the outgoing SMTP server to use (e.g. >> - `smtp.example.com` or a raw IP address). Alternatively it can >> - 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 to search for `sendmail` in >> - `/usr/sbin`, `/usr/lib` and $PATH if such program is >> - available, falling back to `localhost` otherwise. >> + `smtp.example.com` or a raw IP address). If unspecified, and if >> + `--sendmail-cmd` is also unspecified, the default is to search >> + for `sendmail` in `/usr/sbin`, `/usr/lib` and $PATH if such a >> + program is available, falling back to `localhost` otherwise. >> + >> + For backward compatibility, this option can also specify a full >> + pathname of a sendmail-like program instead; the program must >> + support the `-i` option. Prefer using `--sendmail-cmd` instead. >Drop the last sentence, if we are not going to explain why. I do think nudging users to use the "correct" option is valuable, so I will add some why text. Though I think adhering to the "--smtp-server should specify a host and --sendmail-cmd specifies a command" dichotomy is a good reason in and of itself. >> @@ -1490,14 +1497,15 @@ sub send_message { >> >> unshift (@sendmail_parameters, @smtp_server_options); >> >> + if (file_name_is_absolute($smtp_server)) { >> + # Preserved for backward compatibility >> + $sendmail_command ||= $smtp_server; >> + } > >Hmph, I wonder if this makes the intent more clear. > > if (!defined $sendmail_command && file_name_is_absolute($smtp_server)) { > $sendmail_command = $smtp_server; > } > >That is, if the user gave us the command in newer form, we do not >even have to bother checking if the server is given as an absolute >pathname. Yes I think you're right, I'll make this change. >You seem to have replaced every smtp-server="$(pwd)/ mechanically >with sendmai-cmd=\"$(pwd)/, but please make sure that we have at >least one test left that passes an absolute path to --smtp-server to >ensure that the old mechanism keeps working. A bonus point for >marking such a test that needs to be adjusted when the actual >deprecation happens (i.e. we'd likely to detect the use of absolute >path and throw a warning, so the test should notice the warning >message). Noted, I'll add a test for this case. >Also you would want to tweak some of the --sendmail-cmd variants to >use just the command name, with and without args, to ensure that (1) >discovery on $PATH works, and (2) passing initial args works. I did add two such tests: diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 65b3035371..82a3efb987 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -2148,6 +2148,29 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' ' test_cmp expected-list actual-list ' +test_expect_success $PREREQ 'test using relative path with sendmailCommand' ' + clean_fake_sendmail && + PATH="$(pwd):$PATH" \ + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --sendmail-cmd="fake.sendmail" \ + HEAD~2 && + test_path_is_file commandline1 && + test_path_is_file commandline2 +' + +test_expect_success $PREREQ 'test using shell with sendmailCommand' ' + clean_fake_sendmail && + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --sendmail-cmd="[ 1 -eq 1 ] && \"$(pwd)/fake.sendmail\"" \ + HEAD~2 && + test_path_is_file commandline1 && + test_path_is_file commandline2 +' + test_expect_success $PREREQ 'invoke hook' ' mkdir -p .git/hooks && Granted, the second test tests for some generic shell expression, not passing arguments, but I think if the former works the latter ought to as well. Thanks for your feedback.
On Wed, 12 May 2021 02:57 -0500, Felipe Contreras wrote: >Gregory Anders wrote: >I'm not against these kinds of changes but it took me one minute to >figure out all you did was change the format. > >This belongs in a separate patch. Yes this was pointed out by the other reviewers as well, I'll omit it in subsequent revisions. > >> +--sendmail-cmd=<command>:: > >Oh no no no. Don't do shortcuts. > >If you think --sendmail-command is too long, then address that problem >head on, don't try to hide it. > >I do think it's too long, which is why I suggested --command (especially >since it's obvious which command we are talking about), but I wouldn't >suggest --sdm-command, or something of that sort. We have to own our >decisions. > > 1. --command > 2. --sendmail > 3. --sendmail-command > >We have to pick one. I suggest #1. > >To try to make #3 shorter is just shoving the problem under the rug. The intention behind `--sendmail-cmd` was consistency with `--to-cmd` and `--cc-cmd`. Though both of those options also use the condensed 'cmd' form in their configuration options as well, so I suppose I should also change this one to 'sendemail.sendmailcmd'. I'm not opposed to just '--sendmail' and 'sendemail.sendmail' either. I personally believe --sendmail-cmd is the clearest, even if it's verbose, but I'll concede to whatever consensus we arrive at. > >> --- a/git-send-email.perl >> +++ b/git-send-email.perl >> @@ -70,6 +70,7 @@ sub usage { >> >> Sending: >> --envelope-sender <str> * Email envelope sender. >> + --sendmail-cmd <str> * Shell command to run to send email. >> --smtp-server <str:int> * Outgoing SMTP server to use. The port >> is optional. Default 'localhost'. >> --smtp-server-option <str> * Outgoing SMTP server option to use. >> @@ -252,6 +253,7 @@ sub do_edit { >> my (@suppress_cc); >> my ($auto_8bit_encoding); >> my ($compose_encoding); >> +my ($sendmail_command); >> # Variables with corresponding config settings & hardcoded defaults >> my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() >> my $thread = 1; >> @@ -299,6 +301,7 @@ sub do_edit { >> "assume8bitencoding" => \$auto_8bit_encoding, >> "composeencoding" => \$compose_encoding, >> "transferencoding" => \$target_xfer_encoding, >> + "sendmailcommand" => \$sendmail_command, >> ); >> >> my %config_path_settings = ( >> @@ -432,6 +435,7 @@ sub read_config { >> "no-bcc" => \$no_bcc, >> "chain-reply-to!" => \$chain_reply_to, >> "no-chain-reply-to" => sub {$chain_reply_to = 0}, >> + "sendmail-cmd=s" => \$sendmail_command, > >Isn't it interesting that to make the code readable you picked >$sendmail_command, but you don't want users to type so much, even if >it's more readable? See my note above. > >> --- a/t/t9001-send-email.sh >> +++ b/t/t9001-send-email.sh >> @@ -57,7 +57,7 @@ test_no_confirm () { >> git send-email \ >> --from="Example <from@example.com>" \ >> --to=nobody@example.com \ >> - --smtp-server="$(pwd)/fake.sendmail" \ >> + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ > >People are already using --smpt-server=$cmd, we need to keep testing >that. > >Yes, eventually we would want them to move to --sendmail-cmd (or >--command, or whatever), but that won't happen tomorrow. Therefore our >primary tests need to be focused on --smtp-server. > >We need new *additional* tests for --sendmail-cmd, but those should not >override the current tests. At least not right now. I will add a test case for the absolute path form of --smtp-server; however, if we are introducing an option for specifying a sendmail-like command, surely that is the one to use when using "fake.sendmail", no? If we leave the test cases as-is for now, we introduce a split that someone will eventually need to come back and update anyway. Instead of kicking that can down the road, I thought it best to go ahead and do it now. Thanks for your feedback. Greg
On Wed, 12 May 2021 11:04 +0200, Ævar Arnfjörð Bjarmason wrote: > >On Tue, May 11 2021, Gregory Anders wrote: > >> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt >> index 93708aefea..d9fe8cb7c0 100644 >> --- a/Documentation/git-send-email.txt >> +++ b/Documentation/git-send-email.txt >> @@ -159,13 +159,23 @@ Sending >> ~~~~~~~ >> >> --envelope-sender=<address>:: >> - Specify the envelope sender used to send the emails. >> - This is useful if your default address is not the address that is >> - subscribed to a list. In order to use the 'From' address, set the >> - value to "auto". If you use the sendmail binary, you must have >> - suitable privileges for the -f parameter. Default is the value of the >> - `sendemail.envelopeSender` configuration variable; if that is >> - unspecified, choosing the envelope sender is left to your MTA. >> + Specify the envelope sender used to send the emails. This is >> + useful if your default address is not the address that is >> + subscribed to a list. In order to use the 'From' address, set >> + the value to "auto". If you use the sendmail binary, you must >> + have suitable privileges for the -f parameter. Default is the >> + value of the `sendemail.envelopeSender` configuration variable; >> + if that is unspecified, choosing the envelope sender is left to >> + your MTA. > >Please don't include word-wrapping for unrelated changes in the main >patch. My mistake, this has been pointed out to me multiple times now. I'll remove it in the next revision and I'll be sure to avoid this in the future. > >> - $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug* >> + >> + if (!defined $sendmail_command) { >> + $smtp_server = 'localhost'; # could be 127.0.0.1, too... *shrug* >> + } >> } > >This "let's not accept a 0" change seems unrelated & should be split >into a prep cleanup / refactoring patch. On the one hand it's sensible, >on the other nobody cares about having a command named "0" in their path >(or a hostname), so I think it's fine to have the ||= Perl idiom leak >out here. > >But also, this just seems like confusing logic. Per your docs "your >sendmailCommand has precedence over smtpServer.". > >Why not make this "if not $sendmail_command" part of the top-level check >here (the if this one is nested under), which is only done if >$smtp_sever is not defined, if $sendmail_command is defined we don't >care about $smtp_server later on, no? I mostly left this the way it is to minimize the diff, as this is the style the code was already written in. I agree that explicitly checking whether sendmail_command is undefined is probably clearer to the reader. > >> if ($compose && $compose > 0) { >> @@ -1490,14 +1497,15 @@ sub send_message { >> >> unshift (@sendmail_parameters, @smtp_server_options); >> >> + if (file_name_is_absolute($smtp_server)) { >> + # Preserved for backward compatibility >> + $sendmail_command ||= $smtp_server; >> + } >> + >> if ($dry_run) { >> # We don't want to send the email. >> - } elsif (file_name_is_absolute($smtp_server)) { >> - my $pid = open my $sm, '|-'; >> - defined $pid or die $!; >> - if (!$pid) { >> - exec($smtp_server, @sendmail_parameters) or die $!; >> - } >> + } elsif (defined $sendmail_command) { >> + open my $sm, '|-', "$sendmail_command @sendmail_parameters"; > >Can we really not avoid moving from exec-as-list so Perl quotes >everything, to doing our own interpolation here? It looks like the tests >don't check arguments with whitespace (which should fail with this >change). > Shell interpolation in this case is considered a feature, not a bug, i.e. we want to provide users the ability to use arbitrary shell expressions (as they do in e.g. aliases) or pass arguments. I also modeled this after the implementation for --to-cmd and --cc-cmd, which also run their respective commands in the shell just like this. Also, this *did* cause the tests to fail since the tests write output to a path with a space in it. You'll notice that in the diff for the tests I had to wrap '$(pwd)/fake.sendmail' in additional quotes to resolve this. >> @@ -1592,14 +1600,14 @@ sub send_message { >> printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject); >> } else { >> print($dry_run ? __("Dry-OK. Log says:\n") : __("OK. Log says:\n")); >> - if (!file_name_is_absolute($smtp_server)) { >> + if (defined $sendmail_command) { >> + print "Sendmail: $sendmail_command ".join(' ',@sendmail_parameters)."\n"; >> + } else { >> print "Server: $smtp_server\n"; >> print "MAIL FROM:<$raw_from>\n"; >> foreach my $entry (@recipients) { >> print "RCPT TO:<$entry>\n"; >> } >> - } else { >> - print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n"; >> } >> print $header, "\n"; >> if ($smtp) { > >Minor nit: Let's just continue to use "if (!" here to keep the diff >minimal or split up such refactoring into another change... Sure, I can do that. Thanks for your feedback.
Gregory Anders wrote: > >> +--sendmail-cmd=<command>:: > > > >Oh no no no. Don't do shortcuts. > > > >If you think --sendmail-command is too long, then address that problem > >head on, don't try to hide it. > > > >I do think it's too long, which is why I suggested --command (especially > >since it's obvious which command we are talking about), but I wouldn't > >suggest --sdm-command, or something of that sort. We have to own our > >decisions. > > > > 1. --command > > 2. --sendmail > > 3. --sendmail-command > > > >We have to pick one. I suggest #1. > > > >To try to make #3 shorter is just shoving the problem under the rug. > > The intention behind `--sendmail-cmd` was consistency with `--to-cmd` > and `--cc-cmd`. Though both of those options also use the condensed > 'cmd' form in their configuration options as well, so I suppose I should > also change this one to 'sendemail.sendmailcmd'. I see. In that case that might make sense. I still prefer #1. > >> --- a/t/t9001-send-email.sh > >> +++ b/t/t9001-send-email.sh > >> @@ -57,7 +57,7 @@ test_no_confirm () { > >> git send-email \ > >> --from="Example <from@example.com>" \ > >> --to=nobody@example.com \ > >> - --smtp-server="$(pwd)/fake.sendmail" \ > >> + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ > > > >People are already using --smpt-server=$cmd, we need to keep testing > >that. > > > >Yes, eventually we would want them to move to --sendmail-cmd (or > >--command, or whatever), but that won't happen tomorrow. Therefore our > >primary tests need to be focused on --smtp-server. > > > >We need new *additional* tests for --sendmail-cmd, but those should not > >override the current tests. At least not right now. > > I will add a test case for the absolute path form of --smtp-server; > however, if we are introducing an option for specifying a sendmail-like > command, surely that is the one to use when using "fake.sendmail", no? > > If we leave the test cases as-is for now, we introduce a split that > someone will eventually need to come back and update anyway. Instead of > kicking that can down the road, I thought it best to go ahead and do it > now. The sole purpose of software is that it's useful to users. Software that works today but not tomorrow is bad software. The primary purpose of the testing framework is to ensure that doesn't happen; that git keeps working in the same way today than it did yesterday. That's why it's more important that tests excercise the options people were using yesterday. In addition we also want to be testing new functionality, but that's *in addition*. Maybe at some point in the future more people will be using --sendmail-cmd, but right now that's not the case. Right now we need to be testing the option people are using *today*. Cheers.
On Wed, 12 May 2021 12:21 -0500, Felipe Contreras wrote: >The sole purpose of software is that it's useful to users. Software >that works today but not tomorrow is bad software. > >The primary purpose of the testing framework is to ensure that doesn't >happen; that git keeps working in the same way today than it did >yesterday. > >That's why it's more important that tests excercise the options people >were using yesterday. > >In addition we also want to be testing new functionality, but that's *in >addition*. > >Maybe at some point in the future more people will be using >--sendmail-cmd, but right now that's not the case. Right now we need to >be testing the option people are using *today*. I agree with this completely. Is this requirement satisfied with the addition of a test that checks the usage of an absolute path with --smtp-server (the behavior that people were using yesterday), while all of the existing tests are still converted to the new option? I have such a test written (along with a few others) in the forthcoming v2 patch: diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 65b3035371..45bc3c0c4c 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -2148,6 +2148,55 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' ' test_cmp expected-list actual-list ' +test_expect_success $PREREQ 'test using absolute path for --smtp-server' ' + clean_fake_sendmail && + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + HEAD^ && + test_path_is_file commandline1 +' + +test_expect_success $PREREQ 'arguments not supported with --smtp-server' ' + test_expect_code 127 git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail -f nobody@example.com" \ + HEAD^ +' + +test_expect_success $PREREQ 'test using command name with --sendmail-cmd' ' + clean_fake_sendmail && + PATH="$(pwd):$PATH" \ + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --sendmail-cmd="fake.sendmail" \ + HEAD^ && + test_path_is_file commandline1 +' + +test_expect_success $PREREQ 'test using arguments with --sendmail-cmd' ' + clean_fake_sendmail && + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\" -f nobody@example.com" \ + HEAD^ && + test_path_is_file commandline1 +' + +test_expect_success $PREREQ 'test shell expression with --sendmail-cmd' ' + clean_fake_sendmail && + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --sendmail-cmd="[ 1 -eq 1 ] && \"$(pwd)/fake.sendmail\"" \ + HEAD^ && + test_path_is_file commandline1 +' + test_expect_success $PREREQ 'invoke hook' ' mkdir -p .git/hooks &&
Gregory Anders wrote: > On Wed, 12 May 2021 12:21 -0500, Felipe Contreras wrote: > >The sole purpose of software is that it's useful to users. Software > >that works today but not tomorrow is bad software. > > > >The primary purpose of the testing framework is to ensure that doesn't > >happen; that git keeps working in the same way today than it did > >yesterday. > > > >That's why it's more important that tests excercise the options people > >were using yesterday. > > > >In addition we also want to be testing new functionality, but that's *in > >addition*. > > > >Maybe at some point in the future more people will be using > >--sendmail-cmd, but right now that's not the case. Right now we need to > >be testing the option people are using *today*. > > I agree with this completely. Is this requirement satisfied with the > addition of a test that checks the usage of an absolute path with > --smtp-server (the behavior that people were using yesterday), while all > of the existing tests are still converted to the new option? I have such > a test written (along with a few others) in the forthcoming v2 patch: I still insist the current tests should not be converted. But ultimately it's Junio's call. Cheers.
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 93708aefea..d9fe8cb7c0 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -159,13 +159,23 @@ Sending ~~~~~~~ --envelope-sender=<address>:: - Specify the envelope sender used to send the emails. - This is useful if your default address is not the address that is - subscribed to a list. In order to use the 'From' address, set the - value to "auto". If you use the sendmail binary, you must have - suitable privileges for the -f parameter. Default is the value of the - `sendemail.envelopeSender` configuration variable; if that is - unspecified, choosing the envelope sender is left to your MTA. + Specify the envelope sender used to send the emails. This is + useful if your default address is not the address that is + subscribed to a list. In order to use the 'From' address, set + the value to "auto". If you use the sendmail binary, you must + have suitable privileges for the -f parameter. Default is the + value of the `sendemail.envelopeSender` configuration variable; + if that is unspecified, choosing the envelope sender is left to + your MTA. + +--sendmail-cmd=<command>:: + Specify a command to run to send the email. The command should + be compatible with `sendmail` as the arguments are passed + directly. The command will be executed in the shell if + necessary. Default is the value of `sendemail.sendmailCommand`. + If unspecified, and if --smtp-server is also unspecified, + git-send-email will search for `sendmail` in `/usr/sbin`, + `/usr/lib` and $PATH if such a program is available. --smtp-encryption=<encryption>:: Specify the encryption to use, either 'ssl' or 'tls'. Any other @@ -211,13 +221,14 @@ a password is obtained using 'git-credential'. --smtp-server=<host>:: If set, specifies the outgoing SMTP server to use (e.g. - `smtp.example.com` or a raw IP address). Alternatively it can - 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 to search for `sendmail` in - `/usr/sbin`, `/usr/lib` and $PATH if such program is - available, falling back to `localhost` otherwise. + `smtp.example.com` or a raw IP address). If unspecified, and if + `--sendmail-cmd` is also unspecified, the default is to search + for `sendmail` in `/usr/sbin`, `/usr/lib` and $PATH if such a + program is available, falling back to `localhost` otherwise. + + For backward compatibility, this option can also specify a full + pathname of a sendmail-like program instead; the program must + support the `-i` option. Prefer using `--sendmail-cmd` instead. --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 175da07d94..cecca03ed1 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -70,6 +70,7 @@ sub usage { Sending: --envelope-sender <str> * Email envelope sender. + --sendmail-cmd <str> * Shell command to run to send email. --smtp-server <str:int> * Outgoing SMTP server to use. The port is optional. Default 'localhost'. --smtp-server-option <str> * Outgoing SMTP server option to use. @@ -252,6 +253,7 @@ sub do_edit { my (@suppress_cc); my ($auto_8bit_encoding); my ($compose_encoding); +my ($sendmail_command); # Variables with corresponding config settings & hardcoded defaults my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() my $thread = 1; @@ -299,6 +301,7 @@ sub do_edit { "assume8bitencoding" => \$auto_8bit_encoding, "composeencoding" => \$compose_encoding, "transferencoding" => \$target_xfer_encoding, + "sendmailcommand" => \$sendmail_command, ); my %config_path_settings = ( @@ -432,6 +435,7 @@ sub read_config { "no-bcc" => \$no_bcc, "chain-reply-to!" => \$chain_reply_to, "no-chain-reply-to" => sub {$chain_reply_to = 0}, + "sendmail-cmd=s" => \$sendmail_command, "smtp-server=s" => \$smtp_server, "smtp-server-option=s" => \@smtp_server_options, "smtp-server-port=s" => \$smtp_server_port, @@ -1008,11 +1012,14 @@ sub expand_one_alias { push @sendmail_paths, map {"$_/sendmail"} split /:/, $ENV{PATH}; foreach (@sendmail_paths) { if (-x $_) { - $smtp_server = $_; + $sendmail_command ||= $_; last; } } - $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug* + + if (!defined $sendmail_command) { + $smtp_server = 'localhost'; # could be 127.0.0.1, too... *shrug* + } } if ($compose && $compose > 0) { @@ -1490,14 +1497,15 @@ sub send_message { unshift (@sendmail_parameters, @smtp_server_options); + if (file_name_is_absolute($smtp_server)) { + # Preserved for backward compatibility + $sendmail_command ||= $smtp_server; + } + if ($dry_run) { # We don't want to send the email. - } elsif (file_name_is_absolute($smtp_server)) { - my $pid = open my $sm, '|-'; - defined $pid or die $!; - if (!$pid) { - exec($smtp_server, @sendmail_parameters) or die $!; - } + } elsif (defined $sendmail_command) { + open my $sm, '|-', "$sendmail_command @sendmail_parameters"; print $sm "$header\n$message"; close $sm or die $!; } else { @@ -1592,14 +1600,14 @@ sub send_message { printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject); } else { print($dry_run ? __("Dry-OK. Log says:\n") : __("OK. Log says:\n")); - if (!file_name_is_absolute($smtp_server)) { + if (defined $sendmail_command) { + print "Sendmail: $sendmail_command ".join(' ',@sendmail_parameters)."\n"; + } else { print "Server: $smtp_server\n"; print "MAIL FROM:<$raw_from>\n"; foreach my $entry (@recipients) { print "RCPT TO:<$entry>\n"; } - } else { - print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n"; } print $header, "\n"; if ($smtp) { diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 65b3035371..82a3efb987 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -57,7 +57,7 @@ test_no_confirm () { git send-email \ --from="Example <from@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $@ \ $patches >stdout && ! grep "Send this email" stdout && @@ -94,7 +94,7 @@ test_expect_success $PREREQ 'No confirm with sendemail.confirm=never' ' ' test_expect_success $PREREQ 'Send patches' ' - git send-email --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors + git send-email --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --sendmail-cmd="\"$(pwd)/fake.sendmail\"" $patches 2>errors ' test_expect_success $PREREQ 'setup expect' ' @@ -112,7 +112,7 @@ test_expect_success $PREREQ 'Verify commandline' ' test_expect_success $PREREQ 'Send patches with --envelope-sender' ' clean_fake_sendmail && - git send-email --envelope-sender="Patch Contributor <patch@example.com>" --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors + git send-email --envelope-sender="Patch Contributor <patch@example.com>" --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --sendmail-cmd="\"$(pwd)/fake.sendmail\"" $patches 2>errors ' test_expect_success $PREREQ 'setup expect' ' @@ -132,7 +132,7 @@ test_expect_success $PREREQ 'Verify commandline' ' test_expect_success $PREREQ 'Send patches with --envelope-sender=auto' ' clean_fake_sendmail && - git send-email --envelope-sender=auto --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors + git send-email --envelope-sender=auto --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --sendmail-cmd="\"$(pwd)/fake.sendmail\"" $patches 2>errors ' test_expect_success $PREREQ 'setup expect' ' @@ -178,7 +178,7 @@ test_expect_success $PREREQ 'cc trailer with various syntax' ' EOF clean_fake_sendmail && git send-email -1 --to=recipient@example.com \ - --smtp-server="$(pwd)/fake.sendmail" && + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" && test_cmp expected-cc commandline1 ' @@ -197,7 +197,7 @@ test_expect_success $PREREQ 'cc trailer with get_maintainer.pl output' ' clean_fake_sendmail && git send-email -1 --to=recipient@example.com \ --cc-cmd=./expected-cc-script.sh \ - --smtp-server="$(pwd)/fake.sendmail" && + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" && test_cmp expected-cc commandline1 ' @@ -252,7 +252,7 @@ test_suppress_self () { --to=nobody@example.com \ --cc-cmd=./cccmd-sed \ --suppress-cc=self \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ suppress-self-$3.patch && mv msgtxt1 msgtxt1-$3 && @@ -338,7 +338,7 @@ test_expect_success $PREREQ 'Prompting works' ' (echo "to@example.com" && echo "" ) | GIT_SEND_EMAIL_NOTTY=1 git send-email \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches \ 2>errors && grep "^From: A U Thor <author@example.com>\$" msgtxt1 && @@ -352,7 +352,7 @@ test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' ' sane_unset GIT_COMMITTER_NAME && sane_unset GIT_COMMITTER_EMAIL && GIT_SEND_EMAIL_NOTTY=1 git send-email \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --to=to@example.com \ $patches </dev/null 2>errors ) @@ -366,7 +366,7 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' sane_unset GIT_COMMITTER_EMAIL && GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY && test_must_fail git send-email \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --to=to@example.com \ $patches </dev/null 2>errors && test_i18ngrep "tell me who you are" errors @@ -389,7 +389,7 @@ test_expect_success $PREREQ 'tocmd works' ' git send-email \ --from="Example <nobody@example.com>" \ --to-cmd=./tocmd-sed \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ tocmd.patch \ && grep "^To: tocmd@example.com" msgtxt1 @@ -403,7 +403,7 @@ test_expect_success $PREREQ 'cccmd works' ' --from="Example <nobody@example.com>" \ --to=nobody@example.com \ --cc-cmd=./cccmd-sed \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ cccmd.patch \ && grep "^ cccmd@example.com" msgtxt1 @@ -423,7 +423,7 @@ test_expect_success $PREREQ 'reject long lines' ' test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --transfer-encoding=8bit \ $patches longline.patch \ 2>actual && @@ -443,7 +443,7 @@ test_expect_success $PREREQ 'Author From: in message body' ' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && sed "1,/^\$/d" <msgtxt1 >msgbody1 && grep "From: A <author@example.com>" msgbody1 @@ -454,7 +454,7 @@ test_expect_success $PREREQ 'Author From: not in message body' ' git send-email \ --from="A <author@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && sed "1,/^\$/d" <msgtxt1 >msgbody1 && ! grep "From: A <author@example.com>" msgbody1 @@ -464,7 +464,7 @@ test_expect_success $PREREQ 'allow long lines with --no-validate' ' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --no-validate \ $patches longline.patch \ 2>errors @@ -475,7 +475,7 @@ test_expect_success $PREREQ 'short lines with auto encoding are 8bit' ' git send-email \ --from="A <author@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --transfer-encoding=auto \ $patches && grep "Content-Transfer-Encoding: 8bit" msgtxt1 @@ -486,7 +486,7 @@ test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --transfer-encoding=auto \ --no-validate \ longline.patch && @@ -500,7 +500,7 @@ test_expect_success $PREREQ 'carriage returns with auto encoding are quoted-prin git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --transfer-encoding=auto \ --no-validate \ cr.patch && @@ -513,7 +513,7 @@ do git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --transfer-encoding=$enc \ --validate \ $patches longline.patch @@ -533,7 +533,7 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" ' test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --validate \ longline.patch 2>actual && test_path_is_file my-hooks.ran && @@ -552,7 +552,7 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" ' test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --validate \ longline.patch 2>actual && test_path_is_file my-hooks.ran && @@ -571,7 +571,7 @@ do git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --transfer-encoding=$enc \ $patches && grep "Content-Transfer-Encoding: $enc" msgtxt1 @@ -584,7 +584,7 @@ test_expect_success $PREREQ 'Invalid In-Reply-To' ' --from="Example <nobody@example.com>" \ --to=nobody@example.com \ --in-reply-to=" " \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches \ 2>errors && ! grep "^In-Reply-To: < *>" msgtxt1 @@ -596,7 +596,7 @@ test_expect_success $PREREQ 'Valid In-Reply-To when prompting' ' echo "To Example <to@example.com>" && echo "" ) | GIT_SEND_EMAIL_NOTTY=1 git send-email \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches 2>errors && ! grep "^In-Reply-To: < *>" msgtxt1 ' @@ -609,7 +609,7 @@ test_expect_success $PREREQ 'In-Reply-To without --chain-reply-to' ' --to=nobody@example.com \ --no-chain-reply-to \ --in-reply-to="$(cat expect)" \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches $patches $patches \ 2>errors && # The first message is a reply to --in-reply-to @@ -631,7 +631,7 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' ' --to=nobody@example.com \ --chain-reply-to \ --in-reply-to="$(cat expect)" \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches $patches $patches \ 2>errors && sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt1 >actual && @@ -658,7 +658,7 @@ test_expect_success $PREREQ '--compose works' ' --compose --subject foo \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches \ 2>errors ' @@ -992,7 +992,7 @@ test_confirm () { git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $@ $patches >stdout && grep "Send this email" stdout } @@ -1034,7 +1034,7 @@ test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' ' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/*.patch </dev/null ' @@ -1046,7 +1046,7 @@ test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' ' test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches </dev/null ' @@ -1058,7 +1058,7 @@ test_expect_success $PREREQ 'confirm does not loop forever' ' yes "bogus" | test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches ' @@ -1069,7 +1069,7 @@ test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' ' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/*.patch && grep "^ " msgtxt1 | grep "=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= <utf8@example.com>" @@ -1085,7 +1085,7 @@ test_expect_success $PREREQ '--compose adds MIME for utf8 body' ' --compose --subject foo \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && grep "^utf8 body" msgtxt1 && grep "^Content-Type: text/plain; charset=UTF-8" msgtxt1 @@ -1108,7 +1108,7 @@ test_expect_success $PREREQ '--compose respects user mime type' ' --compose --subject foo \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && grep "^utf8 body" msgtxt1 && grep "^Content-Type: text/plain; charset=iso-8859-1" msgtxt1 && @@ -1122,7 +1122,7 @@ test_expect_success $PREREQ '--compose adds MIME for utf8 subject' ' --compose --subject utf8-sübjëct \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && grep "^fake edit" msgtxt1 && grep "^Subject: =?UTF-8?q?utf8-s=C3=BCbj=C3=ABct?=" msgtxt1 @@ -1136,7 +1136,7 @@ test_expect_success $PREREQ 'utf8 author is correctly passed on' ' git format-patch --stdout -1 >funny_name.patch && git send-email --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ funny_name.patch && grep "^From: Füñný Nâmé <odd_?=mail@example.com>" msgtxt1 ' @@ -1149,7 +1149,7 @@ test_expect_success $PREREQ 'utf8 sender is not duplicated' ' git format-patch --stdout -1 >funny_name.patch && git send-email --from="Füñný Nâmé <odd_?=mail@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ funny_name.patch && grep "^From: " msgtxt1 >msgfrom && test_line_count = 1 msgfrom @@ -1166,7 +1166,7 @@ test_expect_success $PREREQ 'sendemail.composeencoding works' ' --compose --subject foo \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && grep "^utf8 body" msgtxt1 && grep "^Content-Type: text/plain; charset=iso-8859-1" msgtxt1 @@ -1183,7 +1183,7 @@ test_expect_success $PREREQ '--compose-encoding works' ' --compose --subject foo \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && grep "^utf8 body" msgtxt1 && grep "^Content-Type: text/plain; charset=iso-8859-1" msgtxt1 @@ -1201,7 +1201,7 @@ test_expect_success $PREREQ '--compose-encoding overrides sendemail.composeencod --compose --subject foo \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && grep "^utf8 body" msgtxt1 && grep "^Content-Type: text/plain; charset=iso-8859-2" msgtxt1 @@ -1215,7 +1215,7 @@ test_expect_success $PREREQ '--compose-encoding adds correct MIME for subject' ' --compose --subject utf8-sübjëct \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $patches && grep "^fake edit" msgtxt1 && grep "^Subject: =?iso-8859-2?q?utf8-s=C3=BCbj=C3=ABct?=" msgtxt1 @@ -1465,7 +1465,7 @@ test_expect_success $PREREQ 'ASCII subject is not RFC2047 quoted' ' clean_fake_sendmail && echo bogus | git send-email --from=author@example.com --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --8bit-encoding=UTF-8 \ email-using-8bit >stdout && grep "Subject" msgtxt1 >actual && @@ -1484,7 +1484,7 @@ test_expect_success $PREREQ 'asks about and fixes 8bit encodings' ' clean_fake_sendmail && echo | git send-email --from=author@example.com --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-8bit >stdout && grep "do not declare a Content-Transfer-Encoding" stdout && grep email-using-8bit stdout && @@ -1498,7 +1498,7 @@ test_expect_success $PREREQ 'sendemail.8bitEncoding works' ' git config sendemail.assume8bitEncoding UTF-8 && echo bogus | git send-email --from=author@example.com --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-8bit >stdout && egrep "Content|MIME" msgtxt1 >actual && test_cmp content-type-decl actual @@ -1509,7 +1509,7 @@ test_expect_success $PREREQ '--8bit-encoding overrides sendemail.8bitEncoding' ' git config sendemail.assume8bitEncoding "bogus too" && echo bogus | git send-email --from=author@example.com --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --8bit-encoding=UTF-8 \ email-using-8bit >stdout && egrep "Content|MIME" msgtxt1 >actual && @@ -1538,7 +1538,7 @@ test_expect_success $PREREQ '--8bit-encoding also treats subject' ' clean_fake_sendmail && echo bogus | git send-email --from=author@example.com --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ --8bit-encoding=UTF-8 \ email-using-8bit >stdout && grep "Subject" msgtxt1 >actual && @@ -1563,7 +1563,7 @@ test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEnc test_must_fail git -c sendemail.transferEncoding=8bit \ send-email \ --transfer-encoding=7bit \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-8bit \ 2>errors >out && grep "cannot send message as 7bit" errors && @@ -1574,7 +1574,7 @@ test_expect_success $PREREQ 'sendemail.transferEncoding via config' ' clean_fake_sendmail && test_must_fail git -c sendemail.transferEncoding=7bit \ send-email \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-8bit \ 2>errors >out && grep "cannot send message as 7bit" errors && @@ -1585,7 +1585,7 @@ test_expect_success $PREREQ 'sendemail.transferEncoding via cli' ' clean_fake_sendmail && test_must_fail git send-email \ --transfer-encoding=7bit \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-8bit \ 2>errors >out && grep "cannot send message as 7bit" errors && @@ -1602,7 +1602,7 @@ test_expect_success $PREREQ '8-bit and sendemail.transferencoding=quoted-printab clean_fake_sendmail && git send-email \ --transfer-encoding=quoted-printable \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-8bit \ 2>errors >out && sed "1,/^$/d" msgtxt1 >actual && @@ -1619,7 +1619,7 @@ test_expect_success $PREREQ '8-bit and sendemail.transferencoding=base64' ' clean_fake_sendmail && git send-email \ --transfer-encoding=base64 \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-8bit \ 2>errors >out && sed "1,/^$/d" msgtxt1 >actual && @@ -1645,7 +1645,7 @@ test_expect_success $PREREQ 'convert from quoted-printable to base64' ' clean_fake_sendmail && git send-email \ --transfer-encoding=base64 \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-qp \ 2>errors >out && sed "1,/^$/d" msgtxt1 >actual && @@ -1675,7 +1675,7 @@ test_expect_success $PREREQ 'CRLF and sendemail.transferencoding=quoted-printabl clean_fake_sendmail && git send-email \ --transfer-encoding=quoted-printable \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-crlf \ 2>errors >out && sed "1,/^$/d" msgtxt1 >actual && @@ -1692,7 +1692,7 @@ test_expect_success $PREREQ 'CRLF and sendemail.transferencoding=base64' ' clean_fake_sendmail && git send-email \ --transfer-encoding=base64 \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ email-using-crlf \ 2>errors >out && sed "1,/^$/d" msgtxt1 >actual && @@ -1710,7 +1710,7 @@ test_expect_success $PREREQ 'refusing to send cover letter template' ' test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/0002-*.patch \ outdir/0000-*.patch \ outdir/0001-*.patch \ @@ -1727,7 +1727,7 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' ' --force \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/0002-*.patch \ outdir/0000-*.patch \ outdir/0001-*.patch \ @@ -1750,7 +1750,7 @@ test_cover_addresses () { --from="Example <nobody@example.com>" \ --no-to --no-cc \ "$@" \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/0000-*.patch \ outdir/0001-*.patch \ outdir/0002-*.patch \ @@ -1789,7 +1789,7 @@ test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' ' git send-email \ --from="Example <nobody@example.com>" \ --to=sbd \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/0001-*.patch \ 2>errors >out && grep "^!somebody@example\.org!$" commandline1 && @@ -1804,7 +1804,7 @@ test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' ' git send-email \ --from="Example <nobody@example.com>" \ --to=sbd \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/0001-*.patch \ 2>errors >out && grep "^!somebody@example\.org!$" commandline1 @@ -1818,7 +1818,7 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' ' git send-email \ --from="Example <nobody@example.com>" \ --to=sbd \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/0001-*.patch \ 2>errors >out && grep "^!someone@example\.org!$" commandline1 @@ -1929,7 +1929,7 @@ test_sendmail_aliases () { git send-email \ --from="Example <nobody@example.com>" \ --to=alice --to=bcgrp \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ outdir/0001-*.patch \ 2>errors >out && for i in $expect @@ -1995,7 +1995,7 @@ test_expect_success $PREREQ 'alias support in To header' ' git format-patch --stdout -1 --to=sbd >aliased.patch && git send-email \ --from="Example <nobody@example.com>" \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ aliased.patch \ 2>errors >out && grep "^!someone@example\.org!$" commandline1 @@ -2009,7 +2009,7 @@ test_expect_success $PREREQ 'alias support in Cc header' ' git format-patch --stdout -1 --cc=sbd >aliased.patch && git send-email \ --from="Example <nobody@example.com>" \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ aliased.patch \ 2>errors >out && grep "^!someone@example\.org!$" commandline1 @@ -2025,7 +2025,7 @@ test_expect_success $PREREQ 'tocmd works with aliases' ' git send-email \ --from="Example <nobody@example.com>" \ --to-cmd=./tocmd-sed \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ tocmd.patch \ 2>errors >out && grep "^!someone@example\.org!$" commandline1 @@ -2041,7 +2041,7 @@ test_expect_success $PREREQ 'cccmd works with aliases' ' git send-email \ --from="Example <nobody@example.com>" \ --cc-cmd=./cccmd-sed \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ cccmd.patch \ 2>errors >out && grep "^!someone@example\.org!$" commandline1 @@ -2053,7 +2053,7 @@ do_xmailer_test () { git send-email \ --from="Example <nobody@example.com>" \ --to=someone@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ $params \ 0001-*.patch \ 2>errors >out && @@ -2148,6 +2148,29 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' ' test_cmp expected-list actual-list ' +test_expect_success $PREREQ 'test using relative path with sendmailCommand' ' + clean_fake_sendmail && + PATH="$(pwd):$PATH" \ + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --sendmail-cmd="fake.sendmail" \ + HEAD~2 && + test_path_is_file commandline1 && + test_path_is_file commandline2 +' + +test_expect_success $PREREQ 'test using shell with sendmailCommand' ' + clean_fake_sendmail && + git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --sendmail-cmd="[ 1 -eq 1 ] && \"$(pwd)/fake.sendmail\"" \ + HEAD~2 && + test_path_is_file commandline1 && + test_path_is_file commandline2 +' + test_expect_success $PREREQ 'invoke hook' ' mkdir -p .git/hooks && @@ -2174,7 +2197,7 @@ test_expect_success $PREREQ 'invoke hook' ' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/../fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/../fake.sendmail\"" \ ../0001-add-main.patch && # Verify error message when a patch is rejected by the hook @@ -2182,7 +2205,7 @@ test_expect_success $PREREQ 'invoke hook' ' test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/../fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/../fake.sendmail\"" \ ../another.patch 2>err && test_i18ngrep "rejected by sendemail-validate hook" err ) @@ -2192,7 +2215,7 @@ test_expect_success $PREREQ 'test that send-email works outside a repo' ' nongit git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ "$(pwd)/0001-add-main.patch" ' @@ -2201,7 +2224,7 @@ test_expect_success $PREREQ 'test that sendmail config is rejected' ' test_must_fail git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ HEAD^ 2>err && test_i18ngrep "found configuration options for '"'"sendmail"'"'" err ' @@ -2211,7 +2234,7 @@ test_expect_success $PREREQ 'test that sendmail config rejection is specific' ' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ HEAD^ ' @@ -2221,7 +2244,7 @@ test_expect_success $PREREQ 'test forbidSendmailVariables behavior override' ' git send-email \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ + --sendmail-cmd="\"$(pwd)/fake.sendmail\"" \ HEAD^ '
The sendemail.smtpServer option currently supports using a sendmail-like program to send emails by specifying an absolute file path. However, this is not ideal for the following reasons: 1. It overloads the meaning of smtpServer (now a program is being used for the server?) 2. It doesn't allow for non-absolute paths, arguments, or arbitrary scripting. Requiring an absolute path is bad for portability, as the same program may be in different locations on different systems. If I wish to pass arguments to my program, I have to use the smtpServerOption option, which is cumbersome (as it must be repeated for each option) and doesn't adhere to normal git conventions. This patch attempts to solve these problems by introducing a new configuration option sendemail.sendmailCommand as well as a command line option --sendmail-cmd. The value of this option is invoked with the standard sendmail options passed as arguments. sendmailCommand has precedence over smtpServer. If both options are unspecified, the default is to search for 'sendmail' in /usr/sbin, /usr/lib, and $PATH. If not found, smtpServer is set to localhost. This mimics the current behavior when smtpServer is unspecified. The option is passed to Perl's `exec()` function, which automatically determines whether or not to invoke a shell. If shell metacharacters are detected, then a shell is used; otherwise, the command is invoked directly. This means that e.g. [sendemail] sendmailCommand = f() { if [ "$(uname -s) = Darwin ]; then sendmail "$@"; else postfix "$@"; fi; };f will be executed in a shell (as expected), while [sendemail] sendmailCommand = msmtp will be executed directly. This change deprecates the use of absolute paths in sendemail.smtpServer, although support is kept for backward compatibility. --- Note that this patch is incompatible with (and supersedes) the patch discussed here: https://public-inbox.org/git/YJs2RceLliGHI5TX@gpanders.com/T/#t Documentation/git-send-email.txt | 39 ++++--- git-send-email.perl | 30 ++++-- t/t9001-send-email.sh | 169 ++++++++++++++++++------------- 3 files changed, 140 insertions(+), 98 deletions(-)