* [PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS
@ 2014-11-08 10:44 Thomas Quinot
2014-11-08 11:09 ` Jeff King
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Quinot @ 2014-11-08 10:44 UTC (permalink / raw
To: git
It may be impractical to install a wrapper script for ssh
when additional parameters need to be passed. Provide an
alternative way of specifying these by means of the GIT_SSH_ARGS
environment variable. Arguments are whitespace delimited following
usual shell conventions; embedded whitespace can be enclosed
in quotes, or escaped with a backslash.
Signed-off-by: Thomas Quinot <thomas@quinot.org>
---
Dear fellow GIT developers,
I hope I won't stray too far away from established procedures
with my first contribution to git. This patch adds support
for a GIT_SSH_ARGS environment variable, providing a way
of specifying ssh arguments without having to create a
wrapper script.
Thomas.
Documentation/git.txt | 11 +++++++++--
connect.c | 22 ++++++++++++++++++++++
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 9202010..3ac7b5b 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -887,13 +887,20 @@ other
than the default SSH port.
+
To pass options to the program that you want to list in GIT_SSH
-you will need to wrap the program and options into a shell script,
-then set GIT_SSH to refer to the shell script.
+you can either wrap the program and options into a shell script,
+then set GIT_SSH to refer to the shell script, or use the
+GIT_SSH_ARGS environment variable (see below).
+
Usually it is easier to configure any desired options through your
personal `.ssh/config` file. Please consult your ssh documentation
for further details.
+'GIT_SSH_ARGS'::
+ This environment variables provides additional arguments to be
+ passed to GIT_SSH. Arguments are split by spaces, the usual shell
+ quoting and escaping is supported. Quote pairs or a backslash can
+ be used to quote them.
+
'GIT_ASKPASS'::
If this environment variable is set, then Git commands which need to
acquire passwords or passphrases (e.g. for HTTP or IMAP authentication)
diff --git a/connect.c b/connect.c
index d47d0ec..3d4b182 100644
--- a/connect.c
+++ b/connect.c
@@ -701,9 +701,15 @@ struct child_process *git_connect(int fd[2], const char *url,
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
const char *ssh = getenv("GIT_SSH");
+ const char *ssh_args_env = getenv("GIT_SSH_ARGS");
+ char *ssh_args = ssh_args_env ?
+ xstrdup(ssh_args_env) : NULL;
int putty = ssh && strcasestr(ssh, "plink");
char *ssh_host = hostandport;
const char *port = NULL;
+ const char **argv;
+ int argc;
+
get_host_and_port(&ssh_host, &port);
port = get_port_numeric(port);
@@ -717,6 +723,22 @@ struct child_process *git_connect(int fd[2], const char *url,
argv_array_push(&conn->args, putty ? "-P" : "-p");
argv_array_push(&conn->args, port);
}
+
+ if (ssh_args) {
+ argc = split_cmdline(ssh_args, &argv);
+ if (argc < 0) {
+ free(ssh_args);
+ die("invalid GIT_SSH_ARGS '%s': %s",
+ ssh_args_env,
+ split_cmdline_strerror(argc));
+ }
+
+ while (argc--)
+ argv_array_push(&conn->args, *argv++);
+ free(ssh_args);
+ free(argv);
+ }
+
argv_array_push(&conn->args, ssh_host);
} else {
/* remove repo-local variables from the environment */
--
1.9.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS
2014-11-08 10:44 [PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS Thomas Quinot
@ 2014-11-08 11:09 ` Jeff King
2014-11-08 12:35 ` Thomas Quinot
0 siblings, 1 reply; 15+ messages in thread
From: Jeff King @ 2014-11-08 11:09 UTC (permalink / raw
To: Thomas Quinot; +Cc: git
On Sat, Nov 08, 2014 at 11:44:39AM +0100, Thomas Quinot wrote:
> It may be impractical to install a wrapper script for ssh
> when additional parameters need to be passed. Provide an
> alternative way of specifying these by means of the GIT_SSH_ARGS
> environment variable. Arguments are whitespace delimited following
> usual shell conventions; embedded whitespace can be enclosed
> in quotes, or escaped with a backslash.
This has bugged me before, too. Almost every sub-program invoked by git
is done so using a shell, so you can put in arbitrary arguments or even
snippets of shell code. But not GIT_SSH.
I think the original reason was to match other programs with similar
variables. At this point, we can't just change it on a whim; the quoting
requirements are different and it would break people's setups. So your
approach of adding a new variable is good (the other option is figuring
out a long-term transition plan).
However, I'm not sure adding a separate variable for options is the best
we can do. Besides being a bit clunky, it has two big shortcomings:
1. It's not consistent with other git variables that use the shell
(e.g., GIT_PAGER).
2. It's not as flexible as a shell; you can't specify "$HOME/bin/ssh"
or other even more esoteric things, like dynamically tweaking
the options based on the environment.
What do you think of adding an alternate variable that is not ssh
_arguments_, but rather just a full shell command for running ssh?
I'm not sure what it could be called (GIT_SSH_SH is probably too
confusing).
> I hope I won't stray too far away from established procedures
> with my first contribution to git. This patch adds support
> for a GIT_SSH_ARGS environment variable, providing a way
> of specifying ssh arguments without having to create a
> wrapper script.
The formatting and whatnot of your submission looks fine to me. The
patch itself looks reasonable, though I didn't look too closely after
making my comments above. :)
-Peff
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS
2014-11-08 11:09 ` Jeff King
@ 2014-11-08 12:35 ` Thomas Quinot
2014-11-08 14:27 ` [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD Thomas Quinot
2014-11-08 14:29 ` [PATCH v2 2/2] git_connect: when using GIT_SSH_CMD, check for plink in argv[0] only Thomas Quinot
0 siblings, 2 replies; 15+ messages in thread
From: Thomas Quinot @ 2014-11-08 12:35 UTC (permalink / raw
To: Jeff King; +Cc: git
Jeff,
Thanks for your feedback!
* Jeff King, 2014-11-08 :
> What do you think of adding an alternate variable that is not ssh
> _arguments_, but rather just a full shell command for running ssh?
> I'm not sure what it could be called (GIT_SSH_SH is probably too
> confusing).
Interesting idea, I had not thought of this, but I agree that it looks
nicer and more flexible from the user's point of view. In addition, it
turns out that it makes the implementation even simpler! I suggest
'GIT_SSH_CMD' for the variable name; patch v2 coming...
Thomas.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD
2014-11-08 12:35 ` Thomas Quinot
@ 2014-11-08 14:27 ` Thomas Quinot
2014-11-09 9:51 ` Jeff King
2014-11-08 14:29 ` [PATCH v2 2/2] git_connect: when using GIT_SSH_CMD, check for plink in argv[0] only Thomas Quinot
1 sibling, 1 reply; 15+ messages in thread
From: Thomas Quinot @ 2014-11-08 14:27 UTC (permalink / raw
To: Jeff King; +Cc: git
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.
Signed-off-by: Thomas Quinot <thomas@quinot.org>
---
As suggested by Jeff, here is a second version introducing a GIT_SSH_CMD
variable that overrides GIT_SSH, and is processed by the shell.
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"
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.
Thomas.
Documentation/git.txt | 26 ++++++++++++++------------
connect.c | 13 ++++++++++---
2 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 9202010..519711d 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -876,19 +876,21 @@ other
and the `core.editor` option in linkgit:git-config[1].
'GIT_SSH'::
- If this environment variable is set then 'git fetch'
- and 'git push' will use this command instead
- of 'ssh' when they need to connect to a remote system.
- The '$GIT_SSH' command will be given exactly two or
- four arguments: the 'username@host' (or just 'host')
- from the URL and the shell command to execute on that
- remote system, optionally preceded by '-p' (literally) and
- the 'port' from the URL when it specifies something other
- than the default SSH port.
+'GIT_SSH_CMD'::
+ If either of these environment variables is set then 'git fetch'
+ and 'git push' will use the specified command instead of 'ssh'
+ when they need to connect to a remote system.
+ The command will be given exactly two or four arguments: the
+ 'username@host' (or just 'host') from the URL and the shell
+ command to execute on that remote system, optionally preceded by
+ '-p' (literally) and the 'port' from the URL when it specifies
+ something other than the default SSH port.
+
-To pass options to the program that you want to list in GIT_SSH
-you will need to wrap the program and options into a shell script,
-then set GIT_SSH to refer to the shell script.
+`$GIT_SSH_CMD` takes precedence over `$GIT_SSH`, and is interpreted
+by the shell, which allows additional arguments to be included.
+`$GIT_SSH` on the other hand must be just the path to a program
+(which can be a wrapper shell script, if additional arguments are
+needed).
+
Usually it is easier to configure any desired options through your
personal `.ssh/config` file. Please consult your ssh documentation
diff --git a/connect.c b/connect.c
index d47d0ec..ecb1821 100644
--- a/connect.c
+++ b/connect.c
@@ -700,16 +700,23 @@ struct child_process *git_connect(int fd[2], const char *url,
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
- const char *ssh = getenv("GIT_SSH");
- int putty = ssh && strcasestr(ssh, "plink");
+ const char *ssh;
+ int putty;
char *ssh_host = hostandport;
const char *port = NULL;
get_host_and_port(&ssh_host, &port);
port = get_port_numeric(port);
- if (!ssh) ssh = "ssh";
+ ssh = getenv("GIT_SSH_CMD");
+ if (ssh)
+ conn->use_shell = 1;
+ else {
+ ssh = getenv("GIT_SSH");
+ if (!ssh) ssh = "ssh";
+ }
argv_array_push(&conn->args, ssh);
+ putty = strcasestr(ssh, "plink");
if (putty && !strcasestr(ssh, "tortoiseplink"))
argv_array_push(&conn->args, "-batch");
if (port) {
--
2.1.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] git_connect: when using GIT_SSH_CMD, check for plink in argv[0] only
2014-11-08 12:35 ` Thomas Quinot
2014-11-08 14:27 ` [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD Thomas Quinot
@ 2014-11-08 14:29 ` Thomas Quinot
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Quinot @ 2014-11-08 14:29 UTC (permalink / raw
To: Jeff King; +Cc: git
git_connect includes circuitry to use specific command line arguments
when using PuTTY's plink. This circuitry is enabled based on the
presence of the "plink" and "tortoiseplink" substrings in the name
of the SSH program to use. When using GIT_SSH_CMD to specify a complete
shell command to run for ssh connections, look for these substrings only
in the program name (not in subsequent arguments).
Signed-off-by: Thomas Quinot <thomas@quinot.org>
---
connect.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/connect.c b/connect.c
index ecb1821..fcb928a 100644
--- a/connect.c
+++ b/connect.c
@@ -700,22 +700,42 @@ struct child_process *git_connect(int fd[2], const char *url,
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
- const char *ssh;
- int putty;
+ const char *ssh_cmd, *ssh;
+ char *ssh_cmd_var = NULL;
+ char *putty;
char *ssh_host = hostandport;
const char *port = NULL;
get_host_and_port(&ssh_host, &port);
port = get_port_numeric(port);
- ssh = getenv("GIT_SSH_CMD");
- if (ssh)
+ ssh_cmd = getenv("GIT_SSH_CMD");
+ if (ssh_cmd) {
+ /*
+ * Split command line to check for plink in
+ * ssh_argv[0].
+ */
+ const char **ssh_argv;
+ ssh_cmd_var = xstrdup(ssh_cmd);
+ int ssh_argc =
+ split_cmdline(ssh_cmd_var, &ssh_argv);
+ if (ssh_argv < 0) {
+ free(ssh_argv);
+ free(ssh_cmd_var);
+ die("invalid GIT_SSH_CMD '%s': %s",
+ ssh,
+ split_cmdline_strerror(ssh_argc));
+ }
+ ssh = ssh_argv[0];
+ free(ssh_argv);
+
conn->use_shell = 1;
- else {
+ argv_array_push(&conn->args, ssh_cmd);
+ } else {
ssh = getenv("GIT_SSH");
if (!ssh) ssh = "ssh";
+ argv_array_push(&conn->args, ssh);
}
- argv_array_push(&conn->args, ssh);
putty = strcasestr(ssh, "plink");
if (putty && !strcasestr(ssh, "tortoiseplink"))
argv_array_push(&conn->args, "-batch");
@@ -725,6 +745,7 @@ struct child_process *git_connect(int fd[2], const char *url,
argv_array_push(&conn->args, port);
}
argv_array_push(&conn->args, ssh_host);
+ if (ssh_cmd_var) free(ssh_cmd_var);
} else {
/* remove repo-local variables from the environment */
conn->env = local_repo_env;
--
2.1.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD
2014-11-08 14:27 ` [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD Thomas Quinot
@ 2014-11-09 9:51 ` Jeff King
2014-11-09 12:39 ` Thomas Quinot
0 siblings, 1 reply; 15+ messages in thread
From: Jeff King @ 2014-11-09 9:51 UTC (permalink / raw
To: Thomas Quinot; +Cc: git
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD
2014-11-09 9:51 ` Jeff King
@ 2014-11-09 12:39 ` Thomas Quinot
2014-11-09 13:24 ` [PATCH v3] " Thomas Quinot
2014-11-09 17:32 ` [PATCH v2 1/2] " Junio C Hamano
0 siblings, 2 replies; 15+ messages in thread
From: Thomas Quinot @ 2014-11-09 12:39 UTC (permalink / raw
To: Jeff King; +Cc: git
* Jeff King, 2014-11-09 :
> 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.
Right, except of course we're stuck with the compatibility issue in any
case.
> But I do not really have a better suggestion for the name, so perhaps
> it's OK.
Could also be GIT_SSH_SHELLCMD, to denote that this is a *shell*
command...
> 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
That sounds like a reasonable approach, and leaves us with the simpler
change (that splitting circuitry was admittely a bit awkward...)
Thomas.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3] git_connect: set ssh shell command in GIT_SSH_CMD
2014-11-09 12:39 ` Thomas Quinot
@ 2014-11-09 13:24 ` Thomas Quinot
2014-11-09 17:32 ` [PATCH v2 1/2] " Junio C Hamano
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Quinot @ 2014-11-09 13:24 UTC (permalink / raw
To: Jeff King; +Cc: git
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.
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_CMD, it is deliberately left up to the
user to make any required adaptation before calling the underlying
ssh implementation.
Signed-off-by: Thomas Quinot <thomas@quinot.org>
---
Third version: disabling the complicated plink circuitry altogether
when using GIT_SSH_CMD, as discussed. This keeps the change
very simple, which is nice.
Keepin GIT_SSH_CMD for the variable name for the time being, as
I'm not very enthusiastic with GIT_SSH_SHELLCMD (more explicit,
but too long for my taste). No strong opinion on that point
though.
Thomas.
Documentation/git.txt | 26 ++++++++++++++------------
connect.c | 14 +++++++++++---
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 9202010..519711d 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -876,19 +876,21 @@ other
and the `core.editor` option in linkgit:git-config[1].
'GIT_SSH'::
- If this environment variable is set then 'git fetch'
- and 'git push' will use this command instead
- of 'ssh' when they need to connect to a remote system.
- The '$GIT_SSH' command will be given exactly two or
- four arguments: the 'username@host' (or just 'host')
- from the URL and the shell command to execute on that
- remote system, optionally preceded by '-p' (literally) and
- the 'port' from the URL when it specifies something other
- than the default SSH port.
+'GIT_SSH_CMD'::
+ If either of these environment variables is set then 'git fetch'
+ and 'git push' will use the specified command instead of 'ssh'
+ when they need to connect to a remote system.
+ The command will be given exactly two or four arguments: the
+ 'username@host' (or just 'host') from the URL and the shell
+ command to execute on that remote system, optionally preceded by
+ '-p' (literally) and the 'port' from the URL when it specifies
+ something other than the default SSH port.
+
-To pass options to the program that you want to list in GIT_SSH
-you will need to wrap the program and options into a shell script,
-then set GIT_SSH to refer to the shell script.
+`$GIT_SSH_CMD` takes precedence over `$GIT_SSH`, and is interpreted
+by the shell, which allows additional arguments to be included.
+`$GIT_SSH` on the other hand must be just the path to a program
+(which can be a wrapper shell script, if additional arguments are
+needed).
+
Usually it is easier to configure any desired options through your
personal `.ssh/config` file. Please consult your ssh documentation
diff --git a/connect.c b/connect.c
index d47d0ec..7fe5268 100644
--- a/connect.c
+++ b/connect.c
@@ -700,14 +700,22 @@ struct child_process *git_connect(int fd[2], const char *url,
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
- const char *ssh = getenv("GIT_SSH");
- int putty = ssh && strcasestr(ssh, "plink");
+ const char *ssh;
+ int putty;
char *ssh_host = hostandport;
const char *port = NULL;
get_host_and_port(&ssh_host, &port);
port = get_port_numeric(port);
- if (!ssh) ssh = "ssh";
+ ssh = getenv("GIT_SSH_CMD");
+ if (ssh) {
+ conn->use_shell = 1;
+ putty = 0;
+ } else {
+ ssh = getenv("GIT_SSH");
+ if (!ssh) ssh = "ssh";
+ putty = strcasestr(ssh, "plink") != NULL;
+ }
argv_array_push(&conn->args, ssh);
if (putty && !strcasestr(ssh, "tortoiseplink"))
--
2.1.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD
2014-11-09 12:39 ` Thomas Quinot
2014-11-09 13:24 ` [PATCH v3] " Thomas Quinot
@ 2014-11-09 17:32 ` Junio C Hamano
2014-11-09 17:47 ` Thomas Quinot
1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2014-11-09 17:32 UTC (permalink / raw
To: Thomas Quinot; +Cc: Jeff King, git
Thomas Quinot <thomas@quinot.org> writes:
> Could also be GIT_SSH_SHELLCMD, to denote that this is a *shell*
> command...
Whatever. I loathe the CMD abbreviation, though. Why spell out
SHELL but not COMMAND? I.e. GIT_SSH_[SHELL_]COMMAND
>> 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
>
> That sounds like a reasonable approach, and leaves us with the simpler
> change (that splitting circuitry was admittely a bit awkward...)
It does not make sense to special case "plink" at the level of this
mechanism where the spawned command line does not even have to
invoke any "ssh" and its derivatives, so I fully agree that choosing
between -P and -p should be the responsibility of whoever is using
this mechanism.
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD
2014-11-09 17:32 ` [PATCH v2 1/2] " Junio C Hamano
@ 2014-11-09 17:47 ` Thomas Quinot
2014-11-09 17:58 ` Junio C Hamano
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Quinot @ 2014-11-09 17:47 UTC (permalink / raw
To: Junio C Hamano; +Cc: Jeff King, git
* Junio C Hamano, 2014-11-09 :
> Whatever. I loathe the CMD abbreviation, though. Why spell out
> SHELL but not COMMAND? I.e. GIT_SSH_[SHELL_]COMMAND
No strong opinion :-) GIT_SSH_COMMAND looks just fine to me.
(GIT_SSH_SHELL_COMMAND starts to feel a bit long...)
Thomas.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD
2014-11-09 17:47 ` Thomas Quinot
@ 2014-11-09 17:58 ` Junio C Hamano
2014-11-09 22:42 ` [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND Thomas Quinot
0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2014-11-09 17:58 UTC (permalink / raw
To: Thomas Quinot; +Cc: Jeff King, git
Thomas Quinot <thomas@quinot.org> writes:
> * Junio C Hamano, 2014-11-09 :
>
>> Whatever. I loathe the CMD abbreviation, though. Why spell out
>> SHELL but not COMMAND? I.e. GIT_SSH_[SHELL_]COMMAND
>
> No strong opinion :-) GIT_SSH_COMMAND looks just fine to me.
> (GIT_SSH_SHELL_COMMAND starts to feel a bit long...)
Yeah I agree.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND
2014-11-09 17:58 ` Junio C Hamano
@ 2014-11-09 22:42 ` Thomas Quinot
2014-11-10 7:11 ` Jeff King
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Quinot @ 2014-11-09 22:42 UTC (permalink / raw
To: Junio C Hamano; +Cc: Jeff King, git
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).
Thomas.
Documentation/git.txt | 26 ++++++++++++++------------
connect.c | 14 +++++++++++---
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 9202010..6a25ba3 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -876,19 +876,21 @@ other
and the `core.editor` option in linkgit:git-config[1].
'GIT_SSH'::
- If this environment variable is set then 'git fetch'
- and 'git push' will use this command instead
- of 'ssh' when they need to connect to a remote system.
- The '$GIT_SSH' command will be given exactly two or
- four arguments: the 'username@host' (or just 'host')
- from the URL and the shell command to execute on that
- remote system, optionally preceded by '-p' (literally) and
- the 'port' from the URL when it specifies something other
- than the default SSH port.
+'GIT_SSH_COMMAND'::
+ If either of these environment variables is set then 'git fetch'
+ and 'git push' will use the specified command instead of 'ssh'
+ when they need to connect to a remote system.
+ The command will be given exactly two or four arguments: the
+ 'username@host' (or just 'host') from the URL and the shell
+ command to execute on that remote system, optionally preceded by
+ '-p' (literally) and the 'port' from the URL when it specifies
+ something other than the default SSH port.
+
-To pass options to the program that you want to list in GIT_SSH
-you will need to wrap the program and options into a shell script,
-then set GIT_SSH to refer to the shell script.
+`$GIT_SSH_COMMAND` takes precedence over `$GIT_SSH`, and is interpreted
+by the shell, which allows additional arguments to be included.
+`$GIT_SSH` on the other hand must be just the path to a program
+(which can be a wrapper shell script, if additional arguments are
+needed).
+
Usually it is easier to configure any desired options through your
personal `.ssh/config` file. Please consult your ssh documentation
diff --git a/connect.c b/connect.c
index d47d0ec..b8bc2ee 100644
--- a/connect.c
+++ b/connect.c
@@ -700,14 +700,22 @@ struct child_process *git_connect(int fd[2], const char *url,
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
- const char *ssh = getenv("GIT_SSH");
- int putty = ssh && strcasestr(ssh, "plink");
+ const char *ssh;
+ int putty;
char *ssh_host = hostandport;
const char *port = NULL;
get_host_and_port(&ssh_host, &port);
port = get_port_numeric(port);
- if (!ssh) ssh = "ssh";
+ ssh = getenv("GIT_SSH_COMMAND");
+ if (ssh) {
+ conn->use_shell = 1;
+ putty = 0;
+ } else {
+ ssh = getenv("GIT_SSH");
+ if (!ssh) ssh = "ssh";
+ putty = strcasestr(ssh, "plink") != NULL;
+ }
argv_array_push(&conn->args, ssh);
if (putty && !strcasestr(ssh, "tortoiseplink"))
--
2.1.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND
2014-11-09 22:42 ` [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND Thomas Quinot
@ 2014-11-10 7:11 ` Jeff King
2014-11-10 8:28 ` Thomas Quinot
0 siblings, 1 reply; 15+ messages in thread
From: Jeff King @ 2014-11-10 7:11 UTC (permalink / raw
To: Thomas Quinot; +Cc: Junio C Hamano, git
On Sun, Nov 09, 2014 at 11:42:32PM +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_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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND
2014-11-10 7:11 ` Jeff King
@ 2014-11-10 8:28 ` Thomas Quinot
2014-11-10 16:54 ` Junio C Hamano
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Quinot @ 2014-11-10 8:28 UTC (permalink / raw
To: Jeff King; +Cc: Junio C Hamano, git
* Jeff King, 2014-11-10 :
> I think this version looks good. Thanks for working on it.
Thanks!
> 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):
OK, committed locally, I can resend a PATCH v5 if that's more
convenient -- do let me know.
Thomas.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND
2014-11-10 8:28 ` Thomas Quinot
@ 2014-11-10 16:54 ` Junio C Hamano
0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2014-11-10 16:54 UTC (permalink / raw
To: Thomas Quinot; +Cc: Jeff King, git
Thomas Quinot <thomas@quinot.org> writes:
> * Jeff King, 2014-11-10 :
>
>> I think this version looks good. Thanks for working on it.
>
> Thanks!
>
>> 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):
>
> OK, committed locally, I can resend a PATCH v5 if that's more
> convenient -- do let me know.
Locally tweaked while queuing.
Thanks both.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-11-10 16:55 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-08 10:44 [PATCH] git_connect: allow passing arguments to ssh in GIT_SSH_ARGS Thomas Quinot
2014-11-08 11:09 ` Jeff King
2014-11-08 12:35 ` Thomas Quinot
2014-11-08 14:27 ` [PATCH v2 1/2] git_connect: set ssh shell command in GIT_SSH_CMD Thomas Quinot
2014-11-09 9:51 ` Jeff King
2014-11-09 12:39 ` Thomas Quinot
2014-11-09 13:24 ` [PATCH v3] " Thomas Quinot
2014-11-09 17:32 ` [PATCH v2 1/2] " Junio C Hamano
2014-11-09 17:47 ` Thomas Quinot
2014-11-09 17:58 ` Junio C Hamano
2014-11-09 22:42 ` [PATCH v4] git_connect: set ssh shell command in GIT_SSH_COMMAND Thomas Quinot
2014-11-10 7:11 ` Jeff King
2014-11-10 8:28 ` Thomas Quinot
2014-11-10 16:54 ` Junio C Hamano
2014-11-08 14:29 ` [PATCH v2 2/2] git_connect: when using GIT_SSH_CMD, check for plink in argv[0] only Thomas Quinot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.