{"thread":{"id":"37904","subject":"[PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS","startedAt":"2014-11-08T10:44:39Z","lastAt":"2014-11-10T16:54:58Z","messageCount":15,"participants":["Thomas Quinot","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"251549","messageId":"20141108104439.GA89717@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":null,"subject":"[PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-08T10:44:39Z","receivedAt":"2014-11-08T10:44:39Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"It may be impractical to install a wrapper script for ssh\nwhen additional parameters need to be passed. Provide an\nalternative way of specifying these by means of the GIT_SSH_ARGS\nenvironment variable. Arguments are whitespace delimited following\nusual shell conventions; embedded whitespace can be enclosed\nin quotes, or escaped with a backslash.\n\nSigned-off-by: Thomas Quinot <thomas@quinot.org>\n---\n\nDear fellow GIT developers,\n\nI hope I won't stray too far away from established procedures\nwith my first contribution to git. This patch adds support\nfor a GIT_SSH_ARGS environment variable, providing a way\nof specifying ssh arguments without having to create a\nwrapper script.\n\nThomas.\n\n Documentation/git.txt | 11 +++++++++--\n connect.c             | 22 ++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 9202010..3ac7b5b 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -887,13 +887,20 @@ other\n \tthan the default SSH port.\n +\n To pass options to the program that you want to list in GIT_SSH\n-you will need to wrap the program and options into a shell script,\n-then set GIT_SSH to refer to the shell script.\n+you can either wrap the program and options into a shell script,\n+then set GIT_SSH to refer to the shell script, or use the\n+GIT_SSH_ARGS environment variable (see below).\n +\n Usually it is easier to configure any desired options through your\n personal `.ssh/config` file.  Please consult your ssh documentation\n for further details.\n \n+'GIT_SSH_ARGS'::\n+\tThis environment variables provides additional arguments to be\n+\tpassed to GIT_SSH. Arguments are split by spaces, the usual shell\n+\tquoting and escaping is supported. Quote pairs or a backslash can\n+\tbe used to quote them.\n+\n 'GIT_ASKPASS'::\n \tIf this environment variable is set, then Git commands which need to\n \tacquire passwords or passphrases (e.g. for HTTP or IMAP authentication)\ndiff --git a/connect.c b/connect.c\nindex d47d0ec..3d4b182 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -701,9 +701,15 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\tconn->in = conn->out = -1;\n \t\tif (protocol == PROTO_SSH) {\n \t\t\tconst char *ssh = getenv(\"GIT_SSH\");\n+\t\t\tconst char *ssh_args_env = getenv(\"GIT_SSH_ARGS\");\n+\t\t\tchar *ssh_args = ssh_args_env ?\n+\t\t\t\txstrdup(ssh_args_env) : NULL;\n \t\t\tint putty = ssh && strcasestr(ssh, \"plink\");\n \t\t\tchar *ssh_host = hostandport;\n \t\t\tconst char *port = NULL;\n+\t\t\tconst char **argv;\n+\t\t\tint argc;\n+\n \t\t\tget_host_and_port(&ssh_host, &port);\n \t\t\tport = get_port_numeric(port);\n \n@@ -717,6 +723,22 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\t\t\targv_array_push(&conn->args, putty ? \"-P\" : \"-p\");\n \t\t\t\targv_array_push(&conn->args, port);\n \t\t\t}\n+\n+\t\t\tif (ssh_args) {\n+\t\t\t\targc = split_cmdline(ssh_args, &argv);\n+\t\t\t\tif (argc < 0) {\n+\t\t\t\t\tfree(ssh_args);\n+\t\t\t\t\tdie(\"invalid GIT_SSH_ARGS '%s': %s\",\n+\t\t\t\t\t    ssh_args_env,\n+\t\t\t\t\t    split_cmdline_strerror(argc));\n+\t\t\t\t}\n+\n+\t\t\t\twhile (argc--)\n+\t\t\t\t\targv_array_push(&conn->args, *argv++);\n+\t\t\t\tfree(ssh_args);\n+\t\t\t\tfree(argv);\n+\t\t\t}\n+\n \t\t\targv_array_push(&conn->args, ssh_host);\n \t\t} else {\n \t\t\t/* remove repo-local variables from the environment */\n-- \n1.9.2\n"},{"id":"251555","messageId":"20141108110958.GB20750@peff.net","threadId":"37904","inReplyTo":"20141108104439.GA89717@melamine.cuivre.fr.eu.org","subject":"Re: [PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-08T11:09:58Z","receivedAt":"2014-11-08T11:09:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 08, 2014 at 11:44:39AM +0100, Thomas Quinot wrote:\n\n> It may be impractical to install a wrapper script for ssh\n> when additional parameters need to be passed. Provide an\n> alternative way of specifying these by means of the GIT_SSH_ARGS\n> environment variable. Arguments are whitespace delimited following\n> usual shell conventions; embedded whitespace can be enclosed\n> in quotes, or escaped with a backslash.\n\nThis has bugged me before, too. Almost every sub-program invoked by git\nis done so using a shell, so you can put in arbitrary arguments or even\nsnippets of shell code. But not GIT_SSH.\n\nI think the original reason was to match other programs with similar\nvariables. At this point, we can't just change it on a whim; the quoting\nrequirements are different and it would break people's setups. So your\napproach of adding a new variable is good (the other option is figuring\nout a long-term transition plan).\n\nHowever, I'm not sure adding a separate variable for options is the best\nwe can do. Besides being a bit clunky, it has two big shortcomings:\n\n  1. It's not consistent with other git variables that use the shell\n     (e.g., GIT_PAGER).\n\n  2. It's not as flexible as a shell; you can't specify \"$HOME/bin/ssh\"\n     or other even more esoteric things, like dynamically tweaking\n     the options based on the environment.\n\nWhat do you think of adding an alternate variable that is not ssh\n_arguments_, but rather just a full shell command for running ssh?\nI'm not sure what it could be called (GIT_SSH_SH is probably too\nconfusing).\n\n> I hope I won't stray too far away from established procedures\n> with my first contribution to git. This patch adds support\n> for a GIT_SSH_ARGS environment variable, providing a way\n> of specifying ssh arguments without having to create a\n> wrapper script.\n\nThe formatting and whatnot of your submission looks fine to me. The\npatch itself looks reasonable, though I didn't look too closely after\nmaking my comments above. :)\n\n-Peff\n"},{"id":"251557","messageId":"20141108123554.GA90057@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"20141108110958.GB20750@peff.net","subject":"Re: [PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-08T12:35:54Z","receivedAt":"2014-11-08T12:35:54Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"Jeff,\n\nThanks for your feedback!\n\n* Jeff King, 2014-11-08 :\n\n> What do you think of adding an alternate variable that is not ssh\n> _arguments_, but rather just a full shell command for running ssh?\n> I'm not sure what it could be called (GIT_SSH_SH is probably too\n> confusing).\n\nInteresting idea, I had not thought of this, but I agree that it looks\nnicer and more flexible from the user's point of view. In addition, it\nturns out that it makes the implementation even simpler! I suggest\n'GIT_SSH_CMD' for the variable name; patch v2 coming...\n\nThomas.\n"},{"id":"251559","messageId":"20141108142753.GA28652@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"20141108123554.GA90057@melamine.cuivre.fr.eu.org","subject":"[PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-08T14:27:53Z","receivedAt":"2014-11-08T14:27:53Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"It may be impractical to install a wrapper script for GIT_SSH\nwhen additional parameters need to be passed. Provide an alternative\nway of specifying a shell command to be run, including command line\narguments, by means of the GIT_SSH_CMD environment variable, which\nbehaves like GIT_SSH but is passed to the shell.\n\nSigned-off-by: Thomas Quinot <thomas@quinot.org>\n---\n\nAs suggested by Jeff, here is a second version introducing a GIT_SSH_CMD\nvariable that overrides GIT_SSH, and is processed by the shell.\n\nNote that with this first patch only, the special processing for PuTTY/plink\nlooks at the whole command in that case. I deliberately left it that way,\neven though one might imagine a case where this would cause a false positive,\ne.g. if the user's login name includes the string 'plink':\n   GIT_SSH_CMD=\"ssh -l fooplink\"\n\nThe work-around in this case would be to write:\n   GIT_SSH_CMD=\"ssh -l foop''link\"\n\nA second patch, coming in a followup message, pre-splits $GIT_SSH_CMD\nand ensures that we look for \"plink\"/\"tortoiseplink\" only in the argv[0]\nelement.\n\nThomas.\n\n Documentation/git.txt | 26 ++++++++++++++------------\n connect.c             | 13 ++++++++++---\n 2 files changed, 24 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 9202010..519711d 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -876,19 +876,21 @@ other\n \tand the `core.editor` option in linkgit:git-config[1].\n \n 'GIT_SSH'::\n-\tIf this environment variable is set then 'git fetch'\n-\tand 'git push' will use this command instead\n-\tof 'ssh' when they need to connect to a remote system.\n-\tThe '$GIT_SSH' command will be given exactly two or\n-\tfour arguments: the 'username@host' (or just 'host')\n-\tfrom the URL and the shell command to execute on that\n-\tremote system, optionally preceded by '-p' (literally) and\n-\tthe 'port' from the URL when it specifies something other\n-\tthan the default SSH port.\n+'GIT_SSH_CMD'::\n+\tIf either of these environment variables is set then 'git fetch'\n+\tand 'git push' will use the specified command instead of 'ssh'\n+\twhen they need to connect to a remote system.\n+\tThe command will be given exactly two or four arguments: the\n+\t'username@host' (or just 'host') from the URL and the shell\n+\tcommand to execute on that remote system, optionally preceded by\n+\t'-p' (literally) and the 'port' from the URL when it specifies\n+\tsomething other than the default SSH port.\n +\n-To pass options to the program that you want to list in GIT_SSH\n-you will need to wrap the program and options into a shell script,\n-then set GIT_SSH to refer to the shell script.\n+`$GIT_SSH_CMD` takes precedence over `$GIT_SSH`, and is interpreted\n+by the shell, which allows additional arguments to be included.\n+`$GIT_SSH` on the other hand must be just the path to a program\n+(which can be a wrapper shell script, if additional arguments are\n+needed).\n +\n Usually it is easier to configure any desired options through your\n personal `.ssh/config` file.  Please consult your ssh documentation\ndiff --git a/connect.c b/connect.c\nindex d47d0ec..ecb1821 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -700,16 +700,23 @@ struct child_process *git_connect(int fd[2], const char *url,\n \n \t\tconn->in = conn->out = -1;\n \t\tif (protocol == PROTO_SSH) {\n-\t\t\tconst char *ssh = getenv(\"GIT_SSH\");\n-\t\t\tint putty = ssh && strcasestr(ssh, \"plink\");\n+\t\t\tconst char *ssh;\n+\t\t\tint putty;\n \t\t\tchar *ssh_host = hostandport;\n \t\t\tconst char *port = NULL;\n \t\t\tget_host_and_port(&ssh_host, &port);\n \t\t\tport = get_port_numeric(port);\n \n-\t\t\tif (!ssh) ssh = \"ssh\";\n+\t\t\tssh = getenv(\"GIT_SSH_CMD\");\n+\t\t\tif (ssh)\n+\t\t\t\tconn->use_shell = 1;\n+\t\t\telse {\n+\t\t\t\tssh = getenv(\"GIT_SSH\");\n+\t\t\t\tif (!ssh) ssh = \"ssh\";\n+\t\t\t}\n \n \t\t\targv_array_push(&conn->args, ssh);\n+\t\t\tputty = strcasestr(ssh, \"plink\");\n \t\t\tif (putty && !strcasestr(ssh, \"tortoiseplink\"))\n \t\t\t\targv_array_push(&conn->args, \"-batch\");\n \t\t\tif (port) {\n-- \n2.1.2\n"},{"id":"251560","messageId":"20141108142940.GB28652@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"20141108123554.GA90057@melamine.cuivre.fr.eu.org","subject":"[PATCH v2 2/2] git_connect: when using GIT_SSH_CMD, check for plink in argv[0] only","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-08T14:29:40Z","receivedAt":"2014-11-08T14:29:40Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"git_connect includes circuitry to use specific command line arguments\nwhen using PuTTY's plink. This circuitry is enabled based on the\npresence of the \"plink\" and \"tortoiseplink\" substrings in the name\nof the SSH program to use. When using GIT_SSH_CMD to specify a complete\nshell command to run for ssh connections, look for these substrings only\nin the program name (not in subsequent arguments).\n\nSigned-off-by: Thomas Quinot <thomas@quinot.org>\n---\n connect.c | 33 +++++++++++++++++++++++++++------\n 1 file changed, 27 insertions(+), 6 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex ecb1821..fcb928a 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -700,22 +700,42 @@ struct child_process *git_connect(int fd[2], const char *url,\n \n \t\tconn->in = conn->out = -1;\n \t\tif (protocol == PROTO_SSH) {\n-\t\t\tconst char *ssh;\n-\t\t\tint putty;\n+\t\t\tconst char *ssh_cmd, *ssh;\n+\t\t\tchar *ssh_cmd_var = NULL;\n+\t\t\tchar *putty;\n \t\t\tchar *ssh_host = hostandport;\n \t\t\tconst char *port = NULL;\n \t\t\tget_host_and_port(&ssh_host, &port);\n \t\t\tport = get_port_numeric(port);\n \n-\t\t\tssh = getenv(\"GIT_SSH_CMD\");\n-\t\t\tif (ssh)\n+\t\t\tssh_cmd = getenv(\"GIT_SSH_CMD\");\n+\t\t\tif (ssh_cmd) {\n+\t\t\t\t/*\n+\t\t\t\t * Split command line to check for plink in\n+\t\t\t\t * ssh_argv[0].\n+\t\t\t\t */\n+\t\t\t\tconst char **ssh_argv;\n+\t\t\t\tssh_cmd_var = xstrdup(ssh_cmd);\n+\t\t\t\tint ssh_argc =\n+\t\t\t\t    split_cmdline(ssh_cmd_var, &ssh_argv);\n+\t\t\t\tif (ssh_argv < 0) {\n+\t\t\t\t\tfree(ssh_argv);\n+\t\t\t\t\tfree(ssh_cmd_var);\n+\t\t\t\t\tdie(\"invalid GIT_SSH_CMD '%s': %s\",\n+\t\t\t\t\t    ssh,\n+\t\t\t\t\t    split_cmdline_strerror(ssh_argc));\n+\t\t\t\t}\n+\t\t\t\tssh = ssh_argv[0];\n+\t\t\t\tfree(ssh_argv);\n+\n \t\t\t\tconn->use_shell = 1;\n-\t\t\telse {\n+\t\t\t\targv_array_push(&conn->args, ssh_cmd);\n+\t\t\t} else {\n \t\t\t\tssh = getenv(\"GIT_SSH\");\n \t\t\t\tif (!ssh) ssh = \"ssh\";\n+\t\t\t\targv_array_push(&conn->args, ssh);\n \t\t\t}\n \n-\t\t\targv_array_push(&conn->args, ssh);\n \t\t\tputty = strcasestr(ssh, \"plink\");\n \t\t\tif (putty && !strcasestr(ssh, \"tortoiseplink\"))\n \t\t\t\targv_array_push(&conn->args, \"-batch\");\n@@ -725,6 +745,7 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\t\t\targv_array_push(&conn->args, port);\n \t\t\t}\n \t\t\targv_array_push(&conn->args, ssh_host);\n+\t\t\tif (ssh_cmd_var) free(ssh_cmd_var);\n \t\t} else {\n \t\t\t/* remove repo-local variables from the environment */\n \t\t\tconn->env = local_repo_env;\n-- \n2.1.2\n"},{"id":"251591","messageId":"20141109095155.GB17369@peff.net","threadId":"37904","inReplyTo":"20141108142753.GA28652@melamine.cuivre.fr.eu.org","subject":"Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-09T09:51:56Z","receivedAt":"2014-11-09T09:51:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 08, 2014 at 03:27:53PM +0100, Thomas Quinot wrote:\n\n> It may be impractical to install a wrapper script for GIT_SSH\n> when additional parameters need to be passed. Provide an alternative\n> way of specifying a shell command to be run, including command line\n> arguments, by means of the GIT_SSH_CMD environment variable, which\n> behaves like GIT_SSH but is passed to the shell.\n\nThanks, I like this much better. The name GIT_SSH_CMD is not too bad.\nPersonally, of the two (GIT_SSH and GIT_SSH_CMD) I would expect the\n\"_CMD\" form to be the one that does not use the shell. But I do not\nreally have a better suggestion for the name, so perhaps it's OK.\n\n> Note that with this first patch only, the special processing for PuTTY/plink\n> looks at the whole command in that case. I deliberately left it that way,\n> even though one might imagine a case where this would cause a false positive,\n> e.g. if the user's login name includes the string 'plink':\n>    GIT_SSH_CMD=\"ssh -l fooplink\"\n> \n> The work-around in this case would be to write:\n>    GIT_SSH_CMD=\"ssh -l foop''link\"\n\nI'd be tempted to say that the \"plink\" magic does not need to kick in at\nall for GIT_SSH_CMD. There are simply too many corner cases in trying to\nguess what the shell is going to run. For example:\n\n> A second patch, coming in a followup message, pre-splits $GIT_SSH_CMD\n> and ensures that we look for \"plink\"/\"tortoiseplink\" only in the argv[0]\n> element.\n\nI don't think you can pre-split accurately. Since we promise to pass the\nresult to the shell, the user gets to write whatever they want. Even:\n\n  GIT_SSH_CMD='f() {\n    if test \"$(hostname)\" = \"some-machine\"; then\n      some-special-ssh \"$@\"\n    else\n      ssh \"$@\"\n  }\n  f'\n\nParsing complications aside, you cannot even know in git which ssh is\ngoing to be run until the shell code is executed. I think either we have\nto leave the responsibility for munging \"-p\" into \"-P\" on the side of\nthe user's shell snippet (and remember that they can still use GIT_SSH\nas usual, so we are not making anything _worse_ for them here), or we\nhave to provide some unambiguous way for them to signal which calling\nconvention we want ($GIT_SSH_CMD_IS_PLINK or something).\n\n-Peff\n"},{"id":"251606","messageId":"20141109123934.GA50956@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"20141109095155.GB17369@peff.net","subject":"Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-09T12:39:34Z","receivedAt":"2014-11-09T12:39:34Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"* Jeff King, 2014-11-09 :\n\n> Thanks, I like this much better. The name GIT_SSH_CMD is not too bad.\n> Personally, of the two (GIT_SSH and GIT_SSH_CMD) I would expect the\n> \"_CMD\" form to be the one that does not use the shell.\n\nRight, except of course we're stuck with the compatibility issue in any\ncase.\n\n> But I do not really have a better suggestion for the name, so perhaps\n> it's OK.\n\nCould also be GIT_SSH_SHELLCMD, to denote that this is a *shell*\ncommand...\n\n> Parsing complications aside, you cannot even know in git which ssh is\n> going to be run until the shell code is executed. I think either we have\n> to leave the responsibility for munging \"-p\" into \"-P\" on the side of\n> the user's shell snippet\n\nThat sounds like a reasonable approach, and leaves us with the simpler\nchange (that splitting circuitry was admittely a bit awkward...)\n\nThomas.\n"},{"id":"251609","messageId":"20141109132413.GA53369@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"20141109123934.GA50956@melamine.cuivre.fr.eu.org","subject":"[PATCH v3] git_connect: set ssh shell command in GIT_SSH_CMD","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-09T13:24:13Z","receivedAt":"2014-11-09T13:24:13Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"It may be impractical to install a wrapper script for GIT_SSH\nwhen additional parameters need to be passed. Provide an alternative\nway of specifying a shell command to be run, including command line\narguments, by means of the GIT_SSH_CMD environment variable, which\nbehaves like GIT_SSH but is passed to the shell.\n\nThe special circuitry to modify parameters in the case of using\nPuTTY's plink/tortoiseplink is activated only when using GIT_SSH;\nin the case of using GIT_SSH_CMD, it is deliberately left up to the\nuser to make any required adaptation before calling the underlying\nssh implementation.\n\nSigned-off-by: Thomas Quinot <thomas@quinot.org>\n---\nThird version: disabling the complicated plink circuitry altogether\nwhen using GIT_SSH_CMD, as discussed. This keeps the change\nvery simple, which is nice.\n\nKeepin GIT_SSH_CMD for the variable name for the time being, as\nI'm not very enthusiastic with GIT_SSH_SHELLCMD (more explicit,\nbut too long for my taste). No strong opinion on that point\nthough.\n\nThomas.\n\n Documentation/git.txt | 26 ++++++++++++++------------\n connect.c             | 14 +++++++++++---\n 2 files changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 9202010..519711d 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -876,19 +876,21 @@ other\n \tand the `core.editor` option in linkgit:git-config[1].\n \n 'GIT_SSH'::\n-\tIf this environment variable is set then 'git fetch'\n-\tand 'git push' will use this command instead\n-\tof 'ssh' when they need to connect to a remote system.\n-\tThe '$GIT_SSH' command will be given exactly two or\n-\tfour arguments: the 'username@host' (or just 'host')\n-\tfrom the URL and the shell command to execute on that\n-\tremote system, optionally preceded by '-p' (literally) and\n-\tthe 'port' from the URL when it specifies something other\n-\tthan the default SSH port.\n+'GIT_SSH_CMD'::\n+\tIf either of these environment variables is set then 'git fetch'\n+\tand 'git push' will use the specified command instead of 'ssh'\n+\twhen they need to connect to a remote system.\n+\tThe command will be given exactly two or four arguments: the\n+\t'username@host' (or just 'host') from the URL and the shell\n+\tcommand to execute on that remote system, optionally preceded by\n+\t'-p' (literally) and the 'port' from the URL when it specifies\n+\tsomething other than the default SSH port.\n +\n-To pass options to the program that you want to list in GIT_SSH\n-you will need to wrap the program and options into a shell script,\n-then set GIT_SSH to refer to the shell script.\n+`$GIT_SSH_CMD` takes precedence over `$GIT_SSH`, and is interpreted\n+by the shell, which allows additional arguments to be included.\n+`$GIT_SSH` on the other hand must be just the path to a program\n+(which can be a wrapper shell script, if additional arguments are\n+needed).\n +\n Usually it is easier to configure any desired options through your\n personal `.ssh/config` file.  Please consult your ssh documentation\ndiff --git a/connect.c b/connect.c\nindex d47d0ec..7fe5268 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -700,14 +700,22 @@ struct child_process *git_connect(int fd[2], const char *url,\n \n \t\tconn->in = conn->out = -1;\n \t\tif (protocol == PROTO_SSH) {\n-\t\t\tconst char *ssh = getenv(\"GIT_SSH\");\n-\t\t\tint putty = ssh && strcasestr(ssh, \"plink\");\n+\t\t\tconst char *ssh;\n+\t\t\tint putty;\n \t\t\tchar *ssh_host = hostandport;\n \t\t\tconst char *port = NULL;\n \t\t\tget_host_and_port(&ssh_host, &port);\n \t\t\tport = get_port_numeric(port);\n \n-\t\t\tif (!ssh) ssh = \"ssh\";\n+\t\t\tssh = getenv(\"GIT_SSH_CMD\");\n+\t\t\tif (ssh) {\n+\t\t\t\tconn->use_shell = 1;\n+\t\t\t\tputty = 0;\n+\t\t\t} else {\n+\t\t\t\tssh = getenv(\"GIT_SSH\");\n+\t\t\t\tif (!ssh) ssh = \"ssh\";\n+\t\t\t\tputty = strcasestr(ssh, \"plink\") != NULL;\n+\t\t\t}\n \n \t\t\targv_array_push(&conn->args, ssh);\n \t\t\tif (putty && !strcasestr(ssh, \"tortoiseplink\"))\n-- \n2.1.2\n"},{"id":"251622","messageId":"xmqq389se0up.fsf@gitster.dls.corp.google.com","threadId":"37904","inReplyTo":"20141109123934.GA50956@melamine.cuivre.fr.eu.org","subject":"Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-09T17:32:30Z","receivedAt":"2014-11-09T17:32:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Quinot <thomas@quinot.org> writes:\n\n> Could also be GIT_SSH_SHELLCMD, to denote that this is a *shell*\n> command...\n\nWhatever.  I loathe the CMD abbreviation, though.  Why spell out\nSHELL but not COMMAND?  I.e. GIT_SSH_[SHELL_]COMMAND\n\n>> Parsing complications aside, you cannot even know in git which ssh is\n>> going to be run until the shell code is executed. I think either we have\n>> to leave the responsibility for munging \"-p\" into \"-P\" on the side of\n>> the user's shell snippet\n>\n> That sounds like a reasonable approach, and leaves us with the simpler\n> change (that splitting circuitry was admittely a bit awkward...)\n\nIt does not make sense to special case \"plink\" at the level of this\nmechanism where the spawned command line does not even have to\ninvoke any \"ssh\" and its derivatives, so I fully agree that choosing\nbetween -P and -p should be the responsibility of whoever is using\nthis mechanism.\n\nThanks.\n"},{"id":"251628","messageId":"20141109174738.GA58680@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"xmqq389se0up.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-09T17:47:38Z","receivedAt":"2014-11-09T17:47:38Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"* Junio C Hamano, 2014-11-09 :\n\n> Whatever.  I loathe the CMD abbreviation, though.  Why spell out\n> SHELL but not COMMAND?  I.e. GIT_SSH_[SHELL_]COMMAND\n\nNo strong opinion :-) GIT_SSH_COMMAND looks just fine to me.\n(GIT_SSH_SHELL_COMMAND starts to feel a bit long...)\n \nThomas.\n"},{"id":"251630","messageId":"xmqqppcwb6ip.fsf@gitster.dls.corp.google.com","threadId":"37904","inReplyTo":"20141109174738.GA58680@melamine.cuivre.fr.eu.org","subject":"Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-09T17:58:22Z","receivedAt":"2014-11-09T17:58:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Quinot <thomas@quinot.org> writes:\n\n> * Junio C Hamano, 2014-11-09 :\n>\n>> Whatever.  I loathe the CMD abbreviation, though.  Why spell out\n>> SHELL but not COMMAND?  I.e. GIT_SSH_[SHELL_]COMMAND\n>\n> No strong opinion :-) GIT_SSH_COMMAND looks just fine to me.\n> (GIT_SSH_SHELL_COMMAND starts to feel a bit long...)\n\nYeah I agree.\n"},{"id":"251643","messageId":"20141109224231.GA59064@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"xmqqppcwb6ip.fsf@gitster.dls.corp.google.com","subject":"[PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-09T22:42:32Z","receivedAt":"2014-11-09T22:42:32Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"It may be impractical to install a wrapper script for GIT_SSH\nwhen additional parameters need to be passed. Provide an alternative\nway of specifying a shell command to be run, including command line\narguments, by means of the GIT_SSH_COMMAND environment variable,\nwhich behaves like GIT_SSH but is passed to the shell.\n\nThe special circuitry to modify parameters in the case of using\nPuTTY's plink/tortoiseplink is activated only when using GIT_SSH;\nin the case of using GIT_SSH_COMMAND, it is deliberately left up to\nthe user to make any required parameters adaptation before calling\nthe underlying ssh implementation.\n\nSigned-off-by: Thomas Quinot <thomas@quinot.org>\n---\n\nAmended patch as per discussion with Junio: change variable name\nto GIT_SSH_COMMAND, keep plink special circuitry disabled for\nthis case (leaving it enabled only when using GIT_SSH, thus\npreserving compatibility with legacy usage).\n\nThomas.\n\n Documentation/git.txt | 26 ++++++++++++++------------\n connect.c             | 14 +++++++++++---\n 2 files changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 9202010..6a25ba3 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -876,19 +876,21 @@ other\n \tand the `core.editor` option in linkgit:git-config[1].\n \n 'GIT_SSH'::\n-\tIf this environment variable is set then 'git fetch'\n-\tand 'git push' will use this command instead\n-\tof 'ssh' when they need to connect to a remote system.\n-\tThe '$GIT_SSH' command will be given exactly two or\n-\tfour arguments: the 'username@host' (or just 'host')\n-\tfrom the URL and the shell command to execute on that\n-\tremote system, optionally preceded by '-p' (literally) and\n-\tthe 'port' from the URL when it specifies something other\n-\tthan the default SSH port.\n+'GIT_SSH_COMMAND'::\n+\tIf either of these environment variables is set then 'git fetch'\n+\tand 'git push' will use the specified command instead of 'ssh'\n+\twhen they need to connect to a remote system.\n+\tThe command will be given exactly two or four arguments: the\n+\t'username@host' (or just 'host') from the URL and the shell\n+\tcommand to execute on that remote system, optionally preceded by\n+\t'-p' (literally) and the 'port' from the URL when it specifies\n+\tsomething other than the default SSH port.\n +\n-To pass options to the program that you want to list in GIT_SSH\n-you will need to wrap the program and options into a shell script,\n-then set GIT_SSH to refer to the shell script.\n+`$GIT_SSH_COMMAND` takes precedence over `$GIT_SSH`, and is interpreted\n+by the shell, which allows additional arguments to be included.\n+`$GIT_SSH` on the other hand must be just the path to a program\n+(which can be a wrapper shell script, if additional arguments are\n+needed).\n +\n Usually it is easier to configure any desired options through your\n personal `.ssh/config` file.  Please consult your ssh documentation\ndiff --git a/connect.c b/connect.c\nindex d47d0ec..b8bc2ee 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -700,14 +700,22 @@ struct child_process *git_connect(int fd[2], const char *url,\n \n \t\tconn->in = conn->out = -1;\n \t\tif (protocol == PROTO_SSH) {\n-\t\t\tconst char *ssh = getenv(\"GIT_SSH\");\n-\t\t\tint putty = ssh && strcasestr(ssh, \"plink\");\n+\t\t\tconst char *ssh;\n+\t\t\tint putty;\n \t\t\tchar *ssh_host = hostandport;\n \t\t\tconst char *port = NULL;\n \t\t\tget_host_and_port(&ssh_host, &port);\n \t\t\tport = get_port_numeric(port);\n \n-\t\t\tif (!ssh) ssh = \"ssh\";\n+\t\t\tssh = getenv(\"GIT_SSH_COMMAND\");\n+\t\t\tif (ssh) {\n+\t\t\t\tconn->use_shell = 1;\n+\t\t\t\tputty = 0;\n+\t\t\t} else {\n+\t\t\t\tssh = getenv(\"GIT_SSH\");\n+\t\t\t\tif (!ssh) ssh = \"ssh\";\n+\t\t\t\tputty = strcasestr(ssh, \"plink\") != NULL;\n+\t\t\t}\n \n \t\t\targv_array_push(&conn->args, ssh);\n \t\t\tif (putty && !strcasestr(ssh, \"tortoiseplink\"))\n-- \n2.1.2\n"},{"id":"251656","messageId":"20141110071154.GD7677@peff.net","threadId":"37904","inReplyTo":"20141109224231.GA59064@melamine.cuivre.fr.eu.org","subject":"Re: [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-10T07:11:55Z","receivedAt":"2014-11-10T07:11:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 09, 2014 at 11:42:32PM +0100, Thomas Quinot wrote:\n\n> It may be impractical to install a wrapper script for GIT_SSH\n> when additional parameters need to be passed. Provide an alternative\n> way of specifying a shell command to be run, including command line\n> arguments, by means of the GIT_SSH_COMMAND environment variable,\n> which behaves like GIT_SSH but is passed to the shell.\n> \n> The special circuitry to modify parameters in the case of using\n> PuTTY's plink/tortoiseplink is activated only when using GIT_SSH;\n> in the case of using GIT_SSH_COMMAND, it is deliberately left up to\n> the user to make any required parameters adaptation before calling\n> the underlying ssh implementation.\n> \n> Signed-off-by: Thomas Quinot <thomas@quinot.org>\n> ---\n> \n> Amended patch as per discussion with Junio: change variable name\n> to GIT_SSH_COMMAND, keep plink special circuitry disabled for\n> this case (leaving it enabled only when using GIT_SSH, thus\n> preserving compatibility with legacy usage).\n\nI think this version looks good. Thanks for working on it.\n\nTwo style micro-nits (that I do not think even merit a re-roll by\nthemselves, but Junio may want to mark up as he applies):\n\n> +\t\t\t} else {\n> +\t\t\t\tssh = getenv(\"GIT_SSH\");\n> +\t\t\t\tif (!ssh) ssh = \"ssh\";\n\nYou did not even introduce this line, but only reindented it. However,\nour code style usually would write this as:\n\n\tif (!ssh)\n\t\tssh = \"ssh\";\n\n> +\t\t\t\tputty = strcasestr(ssh, \"plink\") != NULL;\n\nWe would usually write this as:\n\n\tputty = !!strcasestr(ssh, \"plink\");\n\n-Peff\n"},{"id":"251660","messageId":"20141110082858.GA71642@melamine.cuivre.fr.eu.org","threadId":"37904","inReplyTo":"20141110071154.GD7677@peff.net","subject":"Re: [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND","fromName":"Thomas Quinot","fromEmail":"thomas@quinot.org","sentAt":"2014-11-10T08:28:58Z","receivedAt":"2014-11-10T08:28:58Z","isPatch":true,"sender":{"key":"thomas@quinot.org","avatar":"https://avatars.githubusercontent.com/u/1678806?v=4"},"body":"* Jeff King, 2014-11-10 :\n\n> I think this version looks good. Thanks for working on it.\n\nThanks!\n\n> Two style micro-nits (that I do not think even merit a re-roll by\n> themselves, but Junio may want to mark up as he applies):\n\nOK, committed locally, I can resend a PATCH v5 if that's more\nconvenient -- do let me know.\n\nThomas.\n"},{"id":"251681","messageId":"xmqqr3xb9esd.fsf@gitster.dls.corp.google.com","threadId":"37904","inReplyTo":"20141110082858.GA71642@melamine.cuivre.fr.eu.org","subject":"Re: [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-10T16:54:58Z","receivedAt":"2014-11-10T16:54:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Quinot <thomas@quinot.org> writes:\n\n> * Jeff King, 2014-11-10 :\n>\n>> I think this version looks good. Thanks for working on it.\n>\n> Thanks!\n>\n>> Two style micro-nits (that I do not think even merit a re-roll by\n>> themselves, but Junio may want to mark up as he applies):\n>\n> OK, committed locally, I can resend a PATCH v5 if that's more\n> convenient -- do let me know.\n\nLocally tweaked while queuing.\n\nThanks both.\n"}]}