[PATCH] connect: read $GIT_SSH_COMMAND from config file

[PATCH] connect: read $GIT_SSH_COMMAND from config file

[Nguyễn Thái Ngọc Duy]

Similar to $GIT_ASKPASS or $GIT_PROXY_COMMAND, we also read from
config file first then fall back to $GIT_SSH_COMMAND.

This is useful for selecting different private keys targetting the
same host (e.g. github)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 core.gitProxy can also be used for my purpose (I need pushing, not
 fetching though; the document says for fetching but I guess
 core.gitProxy always works for pushing), but then the key file
 is hidden behind the script. And writing a script for this seems a
 bit overkill.

 Documentation/config.txt |  7 +++++++
 connect.c                | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 58673cf..3bf070c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -443,6 +443,13 @@ specify that no proxy be used for a given domain pattern.
 This is useful for excluding servers inside a firewall from
 proxy use, while defaulting to a common proxy for external domains.
 
+core.sshCommand::
+	If this variable 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 is in the same form as
+	'GIT_SSH_COMMAND' environment variable and is overriden when
+	the environment variable is set.
+
 core.ignoreStat::
 	If true, Git will avoid using lstat() calls to detect if files have
 	changed by setting the "assume-unchanged" bit for those tracked files
diff --git a/connect.c b/connect.c
index c53f3f1..722dc3f 100644
--- a/connect.c
+++ b/connect.c
@@ -658,6 +658,19 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
 
 static struct child_process no_fork = CHILD_PROCESS_INIT;
 
+static const char *get_ssh_command(void)
+{
+	const char *ssh;
+
+	if ((ssh = getenv("GIT_SSH_COMMAND")))
+		return ssh;
+
+	if (!git_config_get_string_const("core.sshcommand", &ssh))
+		return ssh;
+
+	return NULL;
+}
+
 /*
  * This returns a dummy child_process if the transport protocol does not
  * need fork(2), or a struct child_process object if it does.  Once done,
@@ -758,7 +771,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 				return NULL;
 			}
 
-			ssh = getenv("GIT_SSH_COMMAND");
+			ssh = get_ssh_command();
 			if (!ssh) {
 				const char *base;
 				char *ssh_dup;
-- 
2.8.2.530.g66e63ce

Re: [PATCH] connect: read $GIT_SSH_COMMAND from config file

[Jeff King]

On Sun, Jun 26, 2016 at 01:16:35PM +0200, Nguyễn Thái Ngọc Duy wrote:

> Similar to $GIT_ASKPASS or $GIT_PROXY_COMMAND, we also read from
> config file first then fall back to $GIT_SSH_COMMAND.
> 
> This is useful for selecting different private keys targetting the
> same host (e.g. github)
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  core.gitProxy can also be used for my purpose (I need pushing, not
>  fetching though; the document says for fetching but I guess
>  core.gitProxy always works for pushing), but then the key file
>  is hidden behind the script. And writing a script for this seems a
>  bit overkill.

This patch makes sense to me. The way I have done this in the past is to
use ssh's config. So I set my remote in one repo to "foo:repo1.git" and
another to "bar:repo2.git", and then:

  Host foo
    Hostname actual-host.example.com
    SomeOption ...

  Host bar
    Hostname actual-host.example.com
    SomeOption ...

(or more likely, most repos are fine with the defaults, and you only
need one fake host block for the oddball repo).

But I think your solution is a more direct and less confusing way of
accomplishing the same thing.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 58673cf..3bf070c 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -443,6 +443,13 @@ specify that no proxy be used for a given domain pattern.
>  This is useful for excluding servers inside a firewall from
>  proxy use, while defaulting to a common proxy for external domains.
>  
> +core.sshCommand::
> +	If this variable is set then 'git fetch' and 'git push' will

Probably s/set/set,/.

> +	use the specified command instead of 'ssh' when they need to
> +	connect to a remote system. The command is in the same form as
> +	'GIT_SSH_COMMAND' environment variable and is overriden when
> +	the environment variable is set.

Probably s/'GIT_SSH_COMMAND'/the &/.

Are we using backticks for typesetting environment variables now? That
has always been my preference, but I haven't kept up with the typography
patches that have been flying lately. +cc Matthieu.

(Similar question for commands like 'git fetch').

> diff --git a/connect.c b/connect.c
> index c53f3f1..722dc3f 100644
> --- a/connect.c
> +++ b/connect.c

The code itself looks obviously correct. :)

-Peff

Re: [PATCH] connect: read $GIT_SSH_COMMAND from config file

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> On Sun, Jun 26, 2016 at 01:16:35PM +0200, Nguyễn Thái Ngọc Duy wrote:
>
>> +	use the specified command instead of 'ssh' when they need to
>> +	connect to a remote system. The command is in the same form as
>> +	'GIT_SSH_COMMAND' environment variable and is overriden when
>> +	the environment variable is set.
>
> Probably s/'GIT_SSH_COMMAND'/the &/.

I think so. I'd write either "same form as `GIT_SSH_COMMAND`" or "same
form as the `GIT_SSH_COMMAND` environment variable`.

> Are we using backticks for typesetting environment variables now? That
> has always been my preference, but I haven't kept up with the typography
> patches that have been flying lately. +cc Matthieu.

Yes, we've tried to make the docs a bit more consistant, and to document
that better in Documentation/CodingGuidelines:

 An environment variable must be prefixed with "$" only when referring to its
 value and not when referring to the variable itself, in this case there is
 nothing to add except the backticks:
   `GIT_DIR` is specified
   `$GIT_DIR/hooks/pre-receive`

> (Similar question for commands like 'git fetch').

Backticks too:

   Literal examples (e.g. use of command-line options, command names,
 branch names, configuration and environment variables) must be
 typeset in monospace (i.e. wrapped with backticks):
   `--pretty=oneline`
   `git rev-list`

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] connect: read $GIT_SSH_COMMAND from config file

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Jeff King <peff@peff.net> writes:
>
>> On Sun, Jun 26, 2016 at 01:16:35PM +0200, Nguyễn Thái Ngọc Duy wrote:
>>
>>> +	use the specified command instead of 'ssh' when they need to
>>> +	connect to a remote system. The command is in the same form as
>>> +	'GIT_SSH_COMMAND' environment variable and is overriden when
>>> +	the environment variable is set.
>>
>> Probably s/'GIT_SSH_COMMAND'/the &/.
>
> I think so. I'd write either "same form as `GIT_SSH_COMMAND`" or "same
> form as the `GIT_SSH_COMMAND` environment variable`.

Let's then squash this in.

 Documentation/config.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bd32069..fe7c9c4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -444,11 +444,11 @@ This is useful for excluding servers inside a firewall from
 proxy use, while defaulting to a common proxy for external domains.
 
 core.sshCommand::
-	If this variable is set then 'git fetch' and 'git push' will
-	use the specified command instead of 'ssh' when they need to
+	If this variable is set, `git fetch` and `git push` will
+	use the specified command instead of `ssh` when they need to
 	connect to a remote system. The command is in the same form as
-	'GIT_SSH_COMMAND' environment variable and is overriden when
-	the environment variable is set.
+	the `GIT_SSH_COMMAND` environment variable and is overriden
+	when the environment variable is set.
 
 core.ignoreStat::
 	If true, Git will avoid using lstat() calls to detect if files have

Re: [PATCH] connect: read $GIT_SSH_COMMAND from config file

[Eric Sunshine]

On Tue, Jun 28, 2016 at 1:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Let's then squash this in.
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -444,11 +444,11 @@ This is useful for excluding servers inside a firewall from
>  core.sshCommand::
> -       If this variable is set then 'git fetch' and 'git push' will
> -       use the specified command instead of 'ssh' when they need to
> +       If this variable is set, `git fetch` and `git push` will
> +       use the specified command instead of `ssh` when they need to
>         connect to a remote system. The command is in the same form as
> -       'GIT_SSH_COMMAND' environment variable and is overriden when
> -       the environment variable is set.
> +       the `GIT_SSH_COMMAND` environment variable and is overriden

s/overriden/overridden/

> +       when the environment variable is set.