* [PATCH] git-send-email: use ! to indicate relative path to command
@ 2021-05-11 18:37 Gregory Anders
2021-05-11 18:57 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Gregory Anders @ 2021-05-11 18:37 UTC (permalink / raw)
To: git; +Cc: Gregory Anders
When the smtpServer config option is prefixed with a ! character, the
value of the option should be interpreted as a command to look up on
PATH.
---
Please note that I am a total perl newbie. It's very likely that I did
something suboptimally or in a non-idiomatic way. Please let me know if
that's the case.
git-send-email.perl | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
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};
+ }
+
my $pid = open my $sm, '|-';
defined $pid or die $!;
if (!$pid) {
--
2.31.1.576.gc7e8ed1dea
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] git-send-email: use ! to indicate relative path to command
2021-05-11 18:37 [PATCH] git-send-email: use ! to indicate relative path to command Gregory Anders
@ 2021-05-11 18:57 ` Jeff King
2021-05-11 19:03 ` Gregory Anders
0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2021-05-11 18:57 UTC (permalink / raw)
To: Gregory Anders; +Cc: git
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] git-send-email: use ! to indicate relative path to command
2021-05-11 18:57 ` Jeff King
@ 2021-05-11 19:03 ` Gregory Anders
2021-05-11 19:11 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Gregory Anders @ 2021-05-11 19:03 UTC (permalink / raw)
To: Jeff King; +Cc: git
I also noticed this after some quick testing and just sent a v2 right
before seeing your reply.
Your (untested) implementation seems much cleaner than mine, and I'm
happy to give that a try. Question: is it okay that we pass just a raw
command name to exec instead of a full path? That is, is there any
reason we need to first find the command in PATH *and then* pass it to
exec (which is what my v2 implementation does)?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] git-send-email: use ! to indicate relative path to command
2021-05-11 19:03 ` Gregory Anders
@ 2021-05-11 19:11 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2021-05-11 19:11 UTC (permalink / raw)
To: Gregory Anders; +Cc: git
On Tue, May 11, 2021 at 01:03:32PM -0600, Gregory Anders wrote:
> I also noticed this after some quick testing and just sent a v2 right before
> seeing your reply.
>
> Your (untested) implementation seems much cleaner than mine, and I'm happy
> to give that a try. Question: is it okay that we pass just a raw command
> name to exec instead of a full path? That is, is there any reason we need to
> first find the command in PATH *and then* pass it to exec (which is what my
> v2 implementation does)?
I don't think so. Perl's exec() should do the PATH lookup itself. I was
surprised not to see this mentioned explicitly in the documentation, but
it clearly does work. E.g., try:
perl -e 'exec("ls")'
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-11 19:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11 18:37 [PATCH] git-send-email: use ! to indicate relative path to command Gregory Anders
2021-05-11 18:57 ` Jeff King
2021-05-11 19:03 ` Gregory Anders
2021-05-11 19:11 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).