git-send-email: smtpserver in $HOME

git-send-email: smtpserver in $HOME

[Jan “Khardix” Staněk]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

Hi all,
I ran into a bit of an issue when attempting to configure sendemail.
I use a GMail integration tool [1]
which is installed into my $HOME directory (~/.local/bin/gmi),
and I would like to use it as `smtpserver`.

[1]: https://github.com/gauteh/lieer

However, I also use different machines with different login/usernames,
which means the above is not so simple.
Basically I would have to keep login-specific configuration files
with appropriate absolute path to the tool "hardcoded" in.
Since I like to keep my configuration in git
and synced across my machines,
that is a state I do not like very much.

Would it be feasible to treat the `smtpserver` as path option
and expand `~`/`~user` paths?
Would it break anything
(i.e., is `~` a valid character for beginning of a hostname)?

Any other thoughts on this? Am I missing anything obvious? :)

Thanks in advance for any considerations and replies.
--
Jan Staněk – Khardix

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: git-send-email: smtpserver in $HOME

[Junio C Hamano]

Jan “Khardix” Staněk  <khardix@gmail.com> writes:

> Would it be feasible to treat the `smtpserver` as path option
> and expand `~`/`~user` paths?
> Would it break anything
> (i.e., is `~` a valid character for beginning of a hostname)?

I haven't given too much thought, but offhand do not think of a
reason why a change like the attached would break things.



 git-send-email.perl | 2 ++
 1 file changed, 2 insertions(+)

diff --git c/git-send-email.perl w/git-send-email.perl
index 1f425c0809..ff58ac5046 100755
--- c/git-send-email.perl
+++ w/git-send-email.perl
@@ -1006,6 +1006,8 @@ sub expand_one_alias {
 		}
 	}
 	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
+} elsif ($smtp_server =~ /^~/) {
+	$smtp_server = glob($smtp_server);
 }
 
 if ($compose && $compose > 0) {

Re: git-send-email: smtpserver in $HOME

[Jan “Khardix” Staněk]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

On 2021-02-15, Junio C Hamano wrote:
> I haven't given too much thought, but offhand do not think of a
> reason why a change like the attached would break things.

Seems reasonable, but I figured I rather ask beforehand.

>  git-send-email.perl | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git c/git-send-email.perl w/git-send-email.perl
> index 1f425c0809..ff58ac5046 100755
> --- c/git-send-email.perl
> +++ w/git-send-email.perl
> @@ -1006,6 +1006,8 @@ sub expand_one_alias {
>  		}
>  	}
>  	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
> +} elsif ($smtp_server =~ /^~/) {
> +	$smtp_server = glob($smtp_server);
>  }

This introduces a special case just for handling $smtp_server…
I was thinking something in the way of the following:

diff --git a/git-send-email.perl b/git-send-email.perl
index 1f425c0809..84c07daf6d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -268,7 +268,6 @@ sub do_edit {
 );

 my %config_settings = (
-    "smtpserver" => \$smtp_server,
     "smtpserverport" => \$smtp_server_port,
     "smtpserveroption" => \@smtp_server_options,
     "smtpuser" => \$smtp_authuser,
@@ -294,6 +293,7 @@ sub do_edit {

 my %config_path_settings = (
     "aliasesfile" => \@alias_files,
+    "smtpserver" => \$smtp_server,
     "smtpsslcertpath" => \$smtp_ssl_cert_path,
 );

This turns the smtpserver option into a "path setting",
which does the user expansion.
My concern was that if there is a SMTP server actually named
i.e. `~someone.example.org`, this change would break that.
Of course, the question is if something like that
is possible or supported…

I have not yet allocated enough time to figure out how to run
and/or modify the test suite, so I do not know if this would actually
break something. I will try to do that in the near future.
--
Jan Staněk – Khardix

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: git-send-email: smtpserver in $HOME

[Jeff King]

On Mon, Feb 15, 2021 at 06:14:39PM -0800, Junio C Hamano wrote:

> Jan “Khardix” Staněk  <khardix@gmail.com> writes:
> 
> > Would it be feasible to treat the `smtpserver` as path option
> > and expand `~`/`~user` paths?
> > Would it break anything
> > (i.e., is `~` a valid character for beginning of a hostname)?
> 
> I haven't given too much thought, but offhand do not think of a
> reason why a change like the attached would break things.

I agree it's unlikely to break anything.

Still, it seems a bit unusual for an executed program to handle tilde
like this. The usual mechanism is for us to run it with the shell and
expect to find it in $PATH.

It looks like there's some weirdness here, though; $smtp_server may be a
hostname, and it looks like we use "/" to distinguish a file path. I
wonder if allowing "!my-sendmail" would be more consistent with other
parts of Git (not to mention more flexible).

-Peff

Re: git-send-email: smtpserver in $HOME

[Junio C Hamano]

Jan “Khardix” Staněk  <khardix@gmail.com> writes:

> On 2021-02-15, Junio C Hamano wrote:
>> I haven't given too much thought, but offhand do not think of a
>> reason why a change like the attached would break things.
>
> Seems reasonable, but I figured I rather ask beforehand.
>
>>  git-send-email.perl | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git c/git-send-email.perl w/git-send-email.perl
>> index 1f425c0809..ff58ac5046 100755
>> --- c/git-send-email.perl
>> +++ w/git-send-email.perl
>> @@ -1006,6 +1006,8 @@ sub expand_one_alias {
>>  		}
>>  	}
>>  	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
>> +} elsif ($smtp_server =~ /^~/) {
>> +	$smtp_server = glob($smtp_server);
>>  }
>
> This introduces a special case just for handling $smtp_server…

Yes, and that was very much deliberate, as I think

	git send-email --smtp-server=~/bin/my-phoney-sendmail

won't be affected by %config_path_settings.  $smtp_ssl_cert_path has
the same problem already, and I didn't want to make things worse (I
think %config_path_settings is a mistake---it is fine to have a list
of variables that can use ~tilde expansion, but I do not see why it
makes sense to allow the ~tilde expansion when the value came from
configureation files, and not from the command line).

> My concern was that if there is a SMTP server actually named
> i.e. `~someone.example.org`, this change would break that.

Can tilde appear in a valid DNS name?  I doubt it.

Re: git-send-email: smtpserver in $HOME

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Still, it seems a bit unusual for an executed program to handle tilde
> like this. The usual mechanism is for us to run it with the shell and
> expect to find it in $PATH.

Yes, the user should be able to deal with the $PATH.

> It looks like there's some weirdness here, though; $smtp_server may be a
> hostname, and it looks like we use "/" to distinguish a file path. I
> wonder if allowing "!my-sendmail" would be more consistent with other
> parts of Git (not to mention more flexible).

I am not sure '!' prefix fits well here.

When cloning from something (that is not yet known as an entity that
we must go over the network), we do an equivalent of "test -f" (for
bundles) and "test -d" (for local repositories), and I think the use
of "does it refer to a local file" here matches it as a precedent.

I do find it sloppy that the check uses file_name_is_absolute() that
only checks the shape of the string, without seeing if it actually
exists and is an executable file, though.

Thanks.

Re: git-send-email: smtpserver in $HOME

[Jeff King]

On Tue, Feb 16, 2021 at 10:57:39AM -0800, Junio C Hamano wrote:

> > It looks like there's some weirdness here, though; $smtp_server may be a
> > hostname, and it looks like we use "/" to distinguish a file path. I
> > wonder if allowing "!my-sendmail" would be more consistent with other
> > parts of Git (not to mention more flexible).
> 
> I am not sure '!' prefix fits well here.

Why not? It matches alias config, credential config, and perhaps others
as "treat this as a shell command".

And like a leading "/", it would not be likely to be confused with an
actual hostname.

> When cloning from something (that is not yet known as an entity that
> we must go over the network), we do an equivalent of "test -f" (for
> bundles) and "test -d" (for local repositories), and I think the use
> of "does it refer to a local file" here matches it as a precedent.

Sure, but you cannot say "does it refer to a local file" for a
non-absolute path. And that is the source of the problem, IMHO: there is
no way to signal "this is a command I expect to be executed" except by
using an absolute path.

Or do you mean that we should see if $smtp_server exists in the PATH,
and if so prefer it over a network hostname?

> I do find it sloppy that the check uses file_name_is_absolute() that
> only checks the shape of the string, without seeing if it actually
> exists and is an executable file, though.

Yes, though that comes with its own problems: you mean to say "foo", but
due to a typo or missing program it is not found, and so you hit "foo"
on the network. Probably not the end of the world, but unexpected, I
would say. That's why I'd prefer having a syntactic marker that
indicates what you mean.

(I'd prefer to just have $smtp_program or similar as that signal
instead, but it doesn't seem worth trying to retrofit it now).

-Peff

Re: git-send-email: smtpserver in $HOME

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Sure, but you cannot say "does it refer to a local file" for a
> non-absolute path.

Hmph, why not?  I would expect that this would work as a valid way

	$ git send-email --smtp-server=./my-phoney-smtp

to test a server substitute (perhaps for testing).  The only reason
why it does not is because file_name_is_absolute() check would not
like it.

> And that is the source of the problem, IMHO: there is
> no way to signal "this is a command I expect to be executed" except by
> using an absolute path.

Yes.

> Or do you mean that we should see if $smtp_server exists in the PATH,
> and if so prefer it over a network hostname?

We can certainly go in that direction, too.  A new --smtp-program option
would be a cleaner way to solve it, though, as you said elsewhere.

Re: git-send-email: smtpserver in $HOME

[Jeff King]

On Tue, Feb 16, 2021 at 11:16:18AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Sure, but you cannot say "does it refer to a local file" for a
> > non-absolute path.
> 
> Hmph, why not?  I would expect that this would work as a valid way
> 
> 	$ git send-email --smtp-server=./my-phoney-smtp
> 
> to test a server substitute (perhaps for testing).  The only reason
> why it does not is because file_name_is_absolute() check would not
> like it.

Ah, sure. But you cannot say "my-phoney-smtp" and find it in the PATH,
because it is syntactically indistinguishable from a host with the same
name.

-Peff

Re: git-send-email: smtpserver in $HOME

[Jan “Khardix” Staněk]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

On 2021-02-16, Junio C Hamano wrote:
> Jan “Khardix” Staněk  <khardix@gmail.com> writes:
> > This introduces a special case just for handling $smtp_server…
>
> Yes, and that was very much deliberate, as I think
>
> 	git send-email --smtp-server=~/bin/my-phoney-sendmail
>
> won't be affected by %config_path_settings.  $smtp_ssl_cert_path has
> the same problem already, and I didn't want to make things worse (I
> think %config_path_settings is a mistake---it is fine to have a list
> of variables that can use ~tilde expansion, but I do not see why it
> makes sense to allow the ~tilde expansion when the value came from
> configureation files, and not from the command line).

Well, unless I'm missing something, shouldn't the tilde above be
expanded by the shell before actually being passed as argument?

$ echo simulate --smtp-server=~/bin/my-phoney-sendmail
simulate --smtp-server=/home/khardix/bin/my-phoney-sendmail

I assumed that only the config values are handled specially
because only they need that – the CLI is handled by shell in advance.
Never played with $smtp_ssl_cert_path though,
so if there are known problems, just ignore me
– or better, point me to the relevant issue/discussion,
if you can be bothered :) Thanks.

> > My concern was that if there is a SMTP server actually named
> > i.e. `~someone.example.org`, this change would break that.
>
> Can tilde appear in a valid DNS name?  I doubt it.

So I actually did my homework and skimmed through the relevant RFCs
(RFC952 and RFC1123); as it turns out, no, it cannot
– only ASCII alphanumerics, '-' and '.' are valid characters.
--
Jan Staněk – Khardix

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: git-send-email: smtpserver in $HOME

[Jan “Khardix” Staněk]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

On 2021-02-16, Jeff King wrote:
> … It matches alias config, credential config, and perhaps others
> as "treat this as a shell command".

I will have to take closer look at the '!' prefix in other git parts,
but I like that approach.

> (I'd prefer to just have $smtp_program or similar as that signal
> instead, but it doesn't seem worth trying to retrofit it now).

That would also be my preference, but it would be a larger change
and I'm not currently confident enough to go this route.

From the points and ideas raised (and btw, thanks a lot for those),
the easiest one seems to be treating $smtp_server as path options,
the cleanest one a new $smtp_program options,
and the middle ground being adding support for '!' prefix.
Unless someone beats me to it, I will try to throw together a patch,
aiming for now for the '!' prefix and falling back to path option
– as this is potentially my first contribution here,
I'll see how it goes.
--
Jan Staněk – Khardix

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: git-send-email: smtpserver in $HOME

[Chris Torek]

Just a small point here:

On Tue, Feb 16, 2021 at 2:17 PM Jan “Khardix” Staněk <khardix@gmail.com> wrote:
> Well, unless I'm missing something, shouldn't the tilde above be
> expanded by the shell before actually being passed as argument?

Maybe.  Some shells do and some don't:

$ echo foo=~/foo
foo=~/foo

bash$ echo foo=~/foo
foo=/home/torek/foo

for instance.

> So I actually did my homework and skimmed through the relevant RFCs
> (RFC952 and RFC1123); as it turns out, no, it cannot
> – only ASCII alphanumerics, '-' and '.' are valid characters.

Right: tilde and slash are both verboten. (Underscore is too, but
that one does get used and some programs allow it.)

Chris

Re: git-send-email: smtpserver in $HOME

[Andreas Schwab]

On Feb 18 2021, Chris Torek wrote:

> Just a small point here:
>
> On Tue, Feb 16, 2021 at 2:17 PM Jan “Khardix” Staněk <khardix@gmail.com> wrote:
>> Well, unless I'm missing something, shouldn't the tilde above be
>> expanded by the shell before actually being passed as argument?
>
> Maybe.  Some shells do and some don't:
>
> $ echo foo=~/foo
> foo=~/foo
>
> bash$ echo foo=~/foo
> foo=/home/torek/foo

But:

$ echo --foo=~/foo
--foo=~/foo

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."