Re: [PATCH v3] git-send-email: use ! to indicate relative path to command

From: Jeff King <peff@peff.net>
Date: May 11, 2021, 19:24 UTC
Message-ID: <YJrZ7Hho6jrVyM7f@coredump.intra.peff.net>
In-Reply-To: <20210511191510.25888-1-greg@gpanders.com>

On Tue, May 11, 2021 at 01:15:11PM -0600, Gregory Anders wrote:

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

This tells us "what", but the commit message is a good place to describe "why". That helps reviewers now understand what you're trying to accomplish, and why this is a good way to do it rather than some other patch.

IMHO the most important "why" here is that there currently is no way to specify a local smtp server program without using a full path.

I think this is a good direction to fix it, though for anybody just seeing this patch, I'd call attention to the nearby thread (and the one it links to):

  https://lore.kernel.org/git/YJrH8uqzapnpNEsb@gpanders.com/

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

We'd probably want a test here, as well; see t/t9001-send-email.sh.

We implicitly test the absolute-path behavior in that script because pass "--smtp-server=$(pwd)/fake.sendmail" in lots of places. But we'd probably want a new test block that checks that:

  PATH=$(pwd):$PATH \
  git send-email --smtp-server="!fake.sendmail"

does what you expect.

-Peff