On 09/27, Junio C Hamano wrote:
> Brandon Williams <bmw...@google.com> writes:
>
> > +`GIT_PROTOCOL`::
> > + For internal use only. Used in handshaking the wire protocol.
> > + Contains a colon ':' separated list of keys with optional values
> > + 'key[=value]'. Presence of unknown keys must be tolerated.
>
> Is this meant to be used only on the "server" end? Am I correct to
> interpret "handshaking" to mean the initial connection acceptor
> (e.g. "git daemon") uses it to pass what it decided to the programs
> that implement the service (e.g. "git receive-pack")?
Yes, the idea is that the client will request a protocol version by
setting GIT_PROTOCOL (or indirectly set when using git:// or http://).
upload-pack and receive-pack will use the keys and values set in
GIT_PROTOCOL to determine which version of the protocol to use. At some
point in the future they may even use other keys and values as a means
of sending more information in an initial request from the client.
>
> > +/*
> > + * Environment variable used in handshaking the wire protocol.
> > + * Contains a colon ':' separated list of keys with optional values
> > + * 'key[=value]'. Presence of unknown keys must be tolerated.
> > + */
> > +#define GIT_PROTOCOL_ENVIRONMENT "GIT_PROTOCOL"
>
> "Must be tolerated" feels a bit strange. When somebody asks you to
> use "version 3" or "version 1 variant 2", when you only know
> "version 0" or "version 1" and you are not yet even aware of the
> concept of "variant", we simply ignore "variant=2" as if it wasn't
> there, even though "version=3" will be rejected (because we know of
> "version"; it's just that we don't know "version=3").
By "Must be tolerated" I was trying to get across that if the server
seeing something it doesn't understand, it shouldn't choke. Maybe a
better wording would be to use the word "ignored"?
>
> > +enum protocol_version determine_protocol_version_server(void)
> > +{
> > + const char *git_protocol = getenv(GIT_PROTOCOL_ENVIRONMENT);
> > + enum protocol_version version = protocol_v0;
> > +
> > + if (git_protocol) {
> > + struct string_list list = STRING_LIST_INIT_DUP;
> > + const struct string_list_item *item;
> > + string_list_split(&list, git_protocol, ':', -1);
> > +
> > + for_each_string_list_item(item, &list) {
> > + const char *value;
> > + enum protocol_version v;
> > +
> > + if (skip_prefix(item->string, "version=", &value)) {
> > + v = parse_protocol_version(value);
> > + if (v > version)
> > + version = v;
> > + }
> > + }
> > +
> > + string_list_clear(&list, 0);
> > + }
> > +
> > + return version;
> > +}
>
> This implements "the largest one wins", not "the last one wins". Is
> there a particular reason why the former is chosen?
>
I envision this logic changing for newer servers once more protocol
versions are added because at some point a server may want to disallow a
particular version (because of a security issue or what have you). So I
figured the easiest thing to do for now was to implement "Newest version
wins".
--
Brandon Williams
