Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 25, 2017, 22:35 UTC
- Message-ID
- <xmqqinp2939j.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <alpine.DEB.2.20.1701251327090.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 5 quoted lines
> Now, with the patch in question (without the follow-up, which I would like > to ask you to ignore, just like you did so far), Git would not figure out > that your script calls PuTTY eventually. The work-around? Easy: > > DUMMY=/plink.exe /path/to/junio-is-a-superstar.sh
Think about how you would explain that to an end-user in our document? You'll need to explain how exactly the auto-detection works, so that the user can "exploit" the loophole to do that. And what maintenance burden does it add when auto-detection is updated?
I think I know you well enough that you know well enough that it is too ugly to live, and I suspect that the above is a tongue-in-cheek "arguing for the sake of argument" and would not need a serious response, but just in case...
> ... > Of course, given our discussion I think all of this should be documented > in the commit message as well as in the man page.
Yes. Here is what comes on an obvious clean-up patch (which will be sent as a follow-up to this message).
-- >8 -- Subject: [PATCH] connect: core.sshvariant to correct misidentification
While the logic we have been using to guess which variant of SSH implementation is in use by looking at the name of the program has been robust enough for GIT_SSH (which does not go through the shell, hence it can only spell the name the SSH program and nothing else), extending it to GIT_SSH_COMMAND and core.sshcommand opens it for larger chance of misidentification, because these can specify arbitrary shell command, and a simple "the first word on the line must be the command name" heuristic may not even look at the command name at all.
As any effort to improve the heuristic will give us only diminishing returns, and especially given that most of the users are expected to have a command line for which the heuristic would work correctly, give an explicit escape hatch to override a misidentification, just in case one happens.
It is arguably bad to add this new variable to the core.* set, in the sense that you'll never see this variable if you do not interact with other repositories over ssh, but core.sshcommand is already there for some reason, so let's match it.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/config.txt | 13 +++++++++++++ connect.c | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt index af2ae4cc02..8ea1db49c6 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -455,6 +455,19 @@ core.sshCommand:: the `GIT_SSH_COMMAND` environment variable and is overridden when the environment variable is set. +core.sshVariant:: + SSH implementations used by Git when running `git fetch`, + `git clone`, and `git push` use slightly different command + line options (e.g. PuTTY and TortoisePlink use `-P <port>` + while OpenSSH uses `-p <port>` to specify the port number. + TortoisePlink in addition requires `-batch` option to be + passed). Git guesses which variant is in use correctly from + the name of the ssh command used (see `core.sshCommand` + configuration variable and also `GIT_SSH_COMMAND` + environment variable) most of the time. You can set this + variable to 'putty', 'tortoiseplink', or 'ssh' to correct it + when Git makes an incorrect guess. + 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 aa51b33bfc..358e9c06f0 100644 --- a/connect.c +++ b/connect.c @@ -691,6 +691,29 @@ static const char *get_ssh_command(void) return NULL; } +static void override_ssh_variant(int *port_option, int *needs_batch) +{ + const char *variant; + + if (git_config_get_string_const("core.sshvariant", &variant)) + return; + if (!strcmp(variant, "tortoiseplink")) { + *port_option = 'P'; + *needs_batch = 1; + } else if (!strcmp(variant, "putty")) { + *port_option = 'P'; + *needs_batch = 0; + } else { + /* default */ + if (strcmp(variant, "ssh")) { + warning(_("core.sshvariant: unknown value '%s'"), variant); + warning(_("using OpenSSH compatible behaviour")); + } + *port_option = 'p'; + *needs_batch = 0; + } +} + /* * 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, @@ -836,6 +859,9 @@ struct child_process *git_connect(int fd[2], const char *url, argv_array_push(&conn->args, "-4"); else if (flags & CONNECT_IPV6) argv_array_push(&conn->args, "-6"); + + override_ssh_variant(&port_option, &needs_batch); + if (needs_batch) argv_array_push(&conn->args, "-batch"); if (port) {
-- 2.11.0-699-ga1f1475476