git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Bryan Turner <bturner@atlassian.com>,
	Brandon Williams <bmwill@google.com>,
	Ben Humphreys <behumphreys@atlassian.com>,
	Git Users <git@vger.kernel.org>
Subject: Re: [ANNOUNCE] Git v2.16.0-rc0
Date: Fri, 15 Jun 2018 01:13:21 -0400	[thread overview]
Message-ID: <20180615051321.GA5840@sigill.intra.peff.net> (raw)
In-Reply-To: <20180615042033.GB255581@aiede.svl.corp.google.com>

On Thu, Jun 14, 2018 at 09:20:33PM -0700, Jonathan Nieder wrote:

> What we've switched to is a versioned interface.  By setting
> GIT_SSH_VARIANT=simple, you are asking Git to promise to pass exactly
> <x> options.  If Git has a new option it wants to pass (like the "-o
> SendEnv" thing) but can live without it, then it can avoid breaking
> your wrapper and continue to follow this new promise.
> 
> The trouble is that GIT_SSH_VARIANT=simple is too... simple.  You
> would like a variant that passes in [-p port] [-4] [-6] as well.  We
> didn't implement that because we didn't have the attention of any
> wrapper writer who wanted it; in absence of a potential user, we
> decided to wait for a user to propose the interface they want.  Now we
> can celebrate, since that day has come.

I'm not sure I'm celebrating. It seems like work for not much benefit.
I'd just as soon use VARIANT=ssh and deal with any fallouts if my script
does not behave exactly like openssh in all regards.

> How would you like your ssh variant to work?  Some possibilities to
> get the thought process going:
> 
>  A. Would you want to set a variable 'GIT_SSH_SUPPORTS_OPTIONS=p46'
>     to inform Git about what options you support?

Not really. That just creates more work when I have to later use "op46"
to support "-o", even though my script already handles it fine (or
worse, since "-o" is open-ended).

>  B. Alternatively, what about a 'GIT_SSH_VARIANT=capabilities' variant
>     that calls "your-ssh-variant --capabailities" to get a
>     machine-readable list of capabilities it supports?

Not really. Now I have to implement this --capabilities thing. This is
literally a 10 line script.

>  C. Alternatively, would you like all parameters to come in on stdin,
>     credential helper style?

That's even worse. ;)

>  D. Other ideas?

I really am happy just saying "look, my script is basically openssh, so
just assume that". If it breaks, it breaks, and I'll fix it.

The one thing I was left puzzled by is why "-G" didn't work in the first
place, since the script really does just pass through its options.
Poking around, I think the problem actually _is_ the old ssh version
thing. I have a new one on my workstation, but our CI boxes are jessie,
and have openssh 1:6.7p1-5+deb8u4. And that's where I was digging into
the failure.

So sorry for misleading you earlier. The wrapper script looks like it's
a red herring to some degree (I guess its name not being "ssh"
contributed, but then the -G detection failed).

> If you were using an old version of OpenSSH, this would be a reason to
> revive the old patch, but I'm tempted to stall longer just to get more
> use cases like this to come out of the woodwork.

So apparently I am using an old version. Just switching to use "-V"
seems like it might be another solution, then.

-Peff

  reply	other threads:[~2018-06-15  5:13 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
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 [this message]
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

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

  git send-email \
    --in-reply-to=20180615051321.GA5840@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=behumphreys@atlassian.com \
    --cc=bmwill@google.com \
    --cc=bturner@atlassian.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /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.
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 NNTP newsgroup(s).