Re: [PATCH 2/8] protocol: introduce protocol extention mechanisms

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

 



On Wed, Sep 13, 2017 at 2:54 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> Create protocol.{c,h} and provide functions which future servers and
> clients can use to determine which protocol to use or is being used.
>
> Also introduce the 'GIT_PROTOCOL' environment variable which will be
> used to communicate a colon separated list of keys with optional values
> to a server.  Unknown keys and values must be tolerated.  This mechanism
> is used to communicate which version of the wire protocol a client would
> like to use with a server.
>
> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> ---
>  Documentation/config.txt | 16 +++++++++++
>  Documentation/git.txt    |  5 ++++
>  Makefile                 |  1 +
>  cache.h                  |  7 +++++
>  protocol.c               | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>  protocol.h               | 15 ++++++++++
>  6 files changed, 116 insertions(+)
>  create mode 100644 protocol.c
>  create mode 100644 protocol.h
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index dc4e3f58a..d5b28a32c 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2517,6 +2517,22 @@ The protocol names currently used by git are:
>      `hg` to allow the `git-remote-hg` helper)
>  --
>
> +protocol.version::

It would be cool to set a set of versions that are good. (I am not sure
if that can be deferred to a later patch.)

  Consider we'd have versions 0,1,2,3,4 in the future:
  In an ideal world the client and server would talk using v4
  as it is the most advanced protocol, right?
  Maybe a security/performance issue is found on the server side
  with say protocol v3. Still no big deal as we are speaking v4.
  But then consider an issue is found on the client side with v4.
  Then the client would happily talk 0..3 while the server would
  love to talk using 0,1,2,4.

The way I think about protocol version negotiation is that
both parties involved have a set of versions that they tolerate
to talk (they might understand more than the tolerated set, but the
user forbade some), and the goal of the negotiation is to find
the highest version number that is part of both the server set
and client set. So quite naturally with this line of thinking the
configuration is to configure a set of versions, which is what
I propose here. Maybe even in the wire format, separated
with colons?

> +       If set, clients will attempt to communicate with a server using
> +       the specified protocol version.  If unset, no attempt will be
> +       made by the client to communicate using a particular protocol
> +       version, this results in protocol version 0 being used.

This sounds as if we're going to be really shy at first and only
users that care will try out new versions at their own risk.
>From a users POV this may be frustrating as I would imagine that
people want to run

  git config --global protocol.version 2

to try it out and then realize that some of their hosts do not speak
2, so they have to actually configure it per repo/remote.

> +       Supported versions:

> +* `0` - the original wire protocol.

In the future this may be misleading as it doesn't specify the date of
when it was original. e.g. are capabilities already supported in "original"?

Maybe phrase it as "wire protocol as of v2.14" ? (Though this sounds
as if new capabilities added in the future are not allowed)


> +
> +extern enum protocol_version parse_protocol_version(const char *value);
> +extern enum protocol_version get_protocol_version_config(void);
> +extern enum protocol_version determine_protocol_version_server(void);
> +extern enum protocol_version determine_protocol_version_client(const char *server_response);

Here is a good place to have some comments.



[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