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")?

> +/*
> + * 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").

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

Reply via email to