Re: [PATCH v2 3/9] protocol: introduce protocol extention mechanisms
From: Junio C Hamano <hidden>
Date: 2017-09-27 05:17:46
Brandon Williams [off-list ref] 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?