Re: [ANNOUNCE] Git v2.16.0-rc0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

>> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux