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

From: Jeff King <peff@peff.net>
Date: Feb 16, 2021, 19:05 UTC
Message-ID: <YCwXf1CA0Xyw/B88@coredump.intra.peff.net>
In-Reply-To: <xmqq35xvrgbg.fsf@gitster.c.googlers.com>

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

Show 6 quoted lines

> > 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