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

2014-11-10 Thread Thomas Quinot
* 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.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-11-10 Thread Junio C Hamano
Thomas Quinot tho...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-11-09 Thread Thomas Quinot
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 tho...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-11-09 Thread Jeff King
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 tho...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html