On Tue, May 11, 2021 at 12:37:03PM -0600, Gregory Anders wrote:
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 175da07d94..dbc5a2f51c 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1492,7 +1492,11 @@ sub send_message {
>
> if ($dry_run) {
> # We don't want to send the email.
> - } elsif (file_name_is_absolute($smtp_server)) {
> + } elsif (file_name_is_absolute($smtp_server) || $smtp_server =~ /^!/) {
> + if ($smtp_server =~ s/^!//) {
> + my $smtp_server = map {"$_/$smtp_server"} split /:/, $ENV{PATH};
> + }
> +
I don't think the new "if" block is doing what you expect:
- the result of "map" is a list, but you are assigning it to a scalar
(so you'll end up with the size of the list, which is really just
counting the number of elements in your $PATH). If you want to
search for a match in the PATH, you'd need to do something like:
for my $candidate (map { "$_/$smtp_server" } split /:/, $ENV{PATH}) {
if (-x $candidate) {
$smtp_server = $candidate;
last;
}
}
But see below.
- the bogus code in the conditional ends up doing nothing, since you
declare a new lexical version of $smtp_server (with "my"), shadowing
the outer variable.
So why does it work at all? Because the "s/^!//" in the "if" statement
actually mutates $smtp_server to remove the "!". And then feeding that
name into exec() below will do a lookup in PATH itself.
So a shorter version of the same thing is just:
...
} elsif (file_name_is_absolute($smtp_server) || $smtp_server =~ s/^!//) {
...
which detects and mutates $smtp_server in the first place.
However, it's probably not a good idea to change that variable, as it
loses information. If we call into send_message() a second time, we
won't realize we're supposed to respect "!".
So perhaps something like (totally untested):
diff --git a/git-send-email.perl b/git-send-email.perl
index 175da07d94..022dcf0999 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1492,11 +1492,14 @@ sub send_message {
if ($dry_run) {
# We don't want to send the email.
- } elsif (file_name_is_absolute($smtp_server)) {
+ } elsif (file_name_is_absolute($smtp_server) || $smtp_server =~ /^!/) {
+ my $prog = $smtp_server;
+ $prog =~ s/^!//;
+
my $pid = open my $sm, '|-';
defined $pid or die $!;
if (!$pid) {
- exec($smtp_server, @sendmail_parameters) or die $!;
+ exec($prog, @sendmail_parameters) or die $!;
}
print $sm "$header\n$message";
close $sm or die $!;
-Peff
