Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD

From: Jeff King <peff@peff.net>
Date: Nov 9, 2014, 09:51 UTC
Message-ID: <20141109095155.GB17369@peff.net>
In-Reply-To: <20141108142753.GA28652@melamine.cuivre.fr.eu.org>

On Sat, Nov 08, 2014 at 03:27:53PM +0100, Thomas Quinot wrote:

Show 5 quoted lines

> It may be impractical to install a wrapper script for GIT_SSH
> when additional parameters need to be passed. Provide an alternative
> way of specifying a shell command to be run, including command line
> arguments, by means of the GIT_SSH_CMD environment variable, which
> behaves like GIT_SSH but is passed to the shell.

Thanks, I like this much better. The name GIT_SSH_CMD is not too bad. Personally, of the two (GIT_SSH and GIT_SSH_CMD) I would expect the "_CMD" form to be the one that does not use the shell. But I do not really have a better suggestion for the name, so perhaps it's OK.

Show 8 quoted lines

> Note that with this first patch only, the special processing for PuTTY/plink
> looks at the whole command in that case. I deliberately left it that way,
> even though one might imagine a case where this would cause a false positive,
> e.g. if the user's login name includes the string 'plink':
>    GIT_SSH_CMD="ssh -l fooplink"
> 
> The work-around in this case would be to write:
>    GIT_SSH_CMD="ssh -l foop''link"

I'd be tempted to say that the "plink" magic does not need to kick in at all for GIT_SSH_CMD. There are simply too many corner cases in trying to guess what the shell is going to run. For example:

> A second patch, coming in a followup message, pre-splits $GIT_SSH_CMD
> and ensures that we look for "plink"/"tortoiseplink" only in the argv[0]
> element.

I don't think you can pre-split accurately. Since we promise to pass the result to the shell, the user gets to write whatever they want. Even:

  GIT_SSH_CMD='f() {
    if test "$(hostname)" = "some-machine"; then
      some-special-ssh "$@"
    else
      ssh "$@"
  }
  f'

Parsing complications aside, you cannot even know in git which ssh is going to be run until the shell code is executed. I think either we have to leave the responsibility for munging "-p" into "-P" on the side of the user's shell snippet (and remember that they can still use GIT_SSH as usual, so we are not making anything _worse_ for them here), or we have to provide some unambiguous way for them to signal which calling convention we want ($GIT_SSH_CMD_IS_PLINK or something).

-Peff