Re: [ANNOUNCE] Git v2.16.0-rc0

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jun 15, 2018, 04:20 UTC
Message-ID: <20180615042033.GB255581@aiede.svl.corp.google.com>
In-Reply-To: <20180614193943.GA2226@sigill.intra.peff.net>

Hi,

Jeff King wrote:

> On Thu, Jun 14, 2018 at 11:55:22AM -0700, Jonathan Nieder wrote:

>> Do you mean that it doesn't pass "-G" through, or that when using old
>> versions of openssh that doesn't support "-G" the probing fails?
>
> It just doesn't pass "-G" through.

Thanks.

Show 10 quoted lines

>> If the former, then detecting the wrapper as something other than
>> "ssh" is intended behavior (though we might want to change what that
>> something is, as discussed in the previous thread).  If the latter,
>> then this is https://crbug.com/git/7 which I consider to be a bug.
>
> I certainly see the argument that "well, if it doesn't do '-G' then it's
> not _really_ openssh". My counter to that is that we don't actually
> _care_ about -G (and never did before recently). It's just a proxy for
> "do we understand -p", which my script does understand. My wrapper might
> eventually break if we depend on new options (like "-o SendEnv")

Exactly: what we want to detect is not whether the script happens to
support the OpenSSH options we use today, but whether it is a thin
wrapper around OpenSSH that supports *all* options.

This wrapper definitely does not qualify. As we start to use more OpenSSH options, we are likely to break it. As you say,

> the worst case there is generally no different before or after your
> patch: the command barfs.

but as a caller, Git has no way to know that: it is easily possible that the wrapper would ignore the new option we want to pass, for example.

In other words, I think your response is based on the interface that we previously, foolishly advertised: "We will pass exactly these options to your wrapper". Except those options changed over time, multiple times. It was a terrible interface.

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.

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?

 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?

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

 D. Other ideas?

[...]

> But again, I'm just describing what makes sense to me. If you feel
> strongly about requiring the variant to be explicitly specified, I can
> certainly live with that.

I think I do feel strongly.

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.

Of course that's a dangerous thought process. Probably by tomorrow I'll think better of it and revive the patch. In the meantime I would be happy to see your answers to the questions above.

Sincerely, Jonathan