Re: [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND

From: Jeff King <peff@peff.net>
Date: Nov 10, 2014, 07:11 UTC
Message-ID: <20141110071154.GD7677@peff.net>
In-Reply-To: <20141109224231.GA59064@melamine.cuivre.fr.eu.org>

On Sun, Nov 09, 2014 at 11:42:32PM +0100, Thomas Quinot wrote:

Show 19 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_COMMAND environment variable,
> which behaves like GIT_SSH but is passed to the shell.
> 
> The special circuitry to modify parameters in the case of using
> PuTTY's plink/tortoiseplink is activated only when using GIT_SSH;
> in the case of using GIT_SSH_COMMAND, it is deliberately left up to
> the user to make any required parameters adaptation before calling
> the underlying ssh implementation.
> 
> Signed-off-by: Thomas Quinot <thomas@quinot.org>
> ---
> 
> Amended patch as per discussion with Junio: change variable name
> to GIT_SSH_COMMAND, keep plink special circuitry disabled for
> this case (leaving it enabled only when using GIT_SSH, thus
> preserving compatibility with legacy usage).

I think this version looks good. Thanks for working on it.

Two style micro-nits (that I do not think even merit a re-roll by themselves, but Junio may want to mark up as he applies):

> +			} else {
> +				ssh = getenv("GIT_SSH");
> +				if (!ssh) ssh = "ssh";

You did not even introduce this line, but only reindented it. However, our code style usually would write this as:

	if (!ssh)
		ssh = "ssh";

> +				putty = strcasestr(ssh, "plink") != NULL;

We would usually write this as:

	putty = !!strcasestr(ssh, "plink");

-Peff