git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Bryan Turner <bturner@atlassian.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Brandon Williams <bmwill@google.com>,
	Ben Humphreys <behumphreys@atlassian.com>,
	Git Users <git@vger.kernel.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	git-packagers@googlegroups.com
Subject: Re: [ANNOUNCE] Git v2.16.0-rc0
Date: Tue, 2 Jan 2018 21:35:16 -0800	[thread overview]
Message-ID: <20180103053516.GB87855@aiede.mtv.corp.google.com> (raw)
In-Reply-To: <CAGyf7-FQp4q2vvH1ponQvmVDTu0hiMSK1JKytQZ4O1i0MCnz7g@mail.gmail.com>

Hi,

A few more notes.

Bryan Turner wrote:

> bturner@ubuntu:~$ ssh -V
> OpenSSH_6.6.1p1 Ubuntu-2ubuntu2.8, OpenSSL 1.0.1f 6 Jan 2014
>
> bturner@ubuntu:~$ ssh -G -p 7999 localhost
> unknown option -- G
> usage: ssh [-1246AaCfgKkMNnqsTtVvXxYy] [-b bind_address] [-c cipher_spec]
[...]
> Is it possible to adjust the check, somehow, so it doesn't impact
> older OpenSSH versions like this? As it stands, it seems likely a fair
> number of users who have an SSH command that does support -4, -6 and
> -p are going to end up getting "penalized" because it doesn't also
> support -G, and have to manually set their SSH variant to "ssh" (or
> something other than "auto") to avoid the automatic detection.
>
> I'd love to say I have a brilliant idea for how to work around this,
> oh and here's a patch, but I don't. One option might be trying to
> actually review the output, and another might be to run "ssh -V", but
> both of those have their own flaws (and the extra process forks aren't
> "free").

I have tomorrow off, so I've filed https://crbug.com/git/7 to make
sure I remember to follow up the day after.  Of course I'll be happy
if someone updates that bug saying they've fixed it in the meantime.

One possibility would be to use -V as a fallback when -G fails, or
even as a replacement for this usage of -G.  To avoid misdetecting
PuTTY and other ssh variants that also implement -V as OpenSSH, we
would have to parse the output.  This would also misdetect a script
that does

	host=$1; shift
	ssh "$host" -- "$@"

as supporting OpenSSH options, when the use of -- ensures it doesn't.

Another possibility is to parse the output when -G fails.  That's
hacky, but I think it would work well!  We would not have to be too
clever, since we can look for the exact output produced by the
versions of OpenSSH that we care about.  This still has issues with
scripts that forward arguments to OpenSSH, but at least those issues
would go away once the user updates their copy of ssh. ;-)

Another possibility is to pass options *before* -V:

	ssh -p 7999 -V

Since OpenSSH parses its arguments left-to-right, this gives similar
information to what we did with -G, and scripts like

	host=$1; shift
	ssh "$host" -- "$@"

would even be correctly detected as not supporting OpenSSH options.
We still would need to parse the output to distinguish OpenSSH from
other ssh implementations like putty (unlike OpenSSH, putty saves up
argument errors in an 'error' variable and forgets about them once it
sees -V).

Trying -G and falling back to -V seems like the simplest detection
mechanism to me at the moment.  I'm hoping I'm missing something
simple (another ssh option?) that allows avoiding this mess.

Regardless, I think we should do something like [1] first to get rid
of the regression.  Thanks again for reporting it.

Sincerely,
Jonathan

[1] https://public-inbox.org/git/20180103050730.GA87855@aiede.mtv.corp.google.com/

  parent reply	other threads:[~2018-01-03  5:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-29  4:30 [ANNOUNCE] Git v2.16.0-rc0 Junio C Hamano
2017-12-29  7:17 ` Kaartic Sivaraam
2017-12-29 17:13 ` Paul Smith
2018-01-04 20:18   ` Thomas Gummerer
2018-01-04 21:35     ` Paul Smith
2018-01-03  3:34 ` Bryan Turner
2018-01-03  5:07   ` Jonathan Nieder
2018-01-03  5:41     ` Bryan Turner
2018-01-03  5:50       ` Jonathan Nieder
2018-01-03 21:15     ` Junio C Hamano
2018-01-03  5:35   ` Jonathan Nieder [this message]
2018-06-08  4:50     ` Jeff King
2018-06-12 16:29       ` Junio C Hamano
2018-06-14 18:30         ` Jeff King
2018-06-14 18:55           ` Jonathan Nieder
2018-06-14 19:39             ` Jeff King
2018-06-15  4:20               ` Jonathan Nieder
2018-06-15  5:13                 ` Jeff King
2018-06-15  7:26                   ` Jeff King
2018-01-04 20:33 ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180103053516.GB87855@aiede.mtv.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=behumphreys@atlassian.com \
    --cc=bmwill@google.com \
    --cc=bturner@atlassian.com \
    --cc=git-packagers@googlegroups.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).