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

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

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.

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