On 09/27, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> >> +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?
>
> Let me give my version of why the usual "the last one wins" would
> not necessarily a good idea. I would imagine that a client
> contacting the server may want to say "I understand v3, v2 (but not
> v1 nor v0)" and in order to influence the server's choice between
> the available two, it may want to somehow say it prefers v3 over v2
> (or v2 over v3).
>
> One way to implement such a behaviour would be "the first one that
> is understood is used", i.e. something along this line:
>
> enum protocol_version version = protocol_unknown;
>
> for_each_string_list_item(item, &list) {
> const char *value;
> enum protocol_version v;
> if (skip_prefix(item->string, "version=", &value)) {
> if (version == protocol_unknown) {
> v = parse_protocol_version(value);
> if (v != protocol_unknown)
> version = v;
> }
> }
> }
>
> if (version == protocol_unknown)
> version = protocol_v0;
>
> and not "the largest one wins" nor "the last one wins".
>
> I am not saying your code or the choice of "the largest one wins" is
> necessarily wrong. I am just illlustrating the way to explain
> "because I want to support a usecase like _this_, I define the way
> in which multiple values to the version variable is parsed like so,
> hence this code". IOW, I think this commit should mention how the
> "largest one wins" rule would be useful to the clients and the
> servers when they want to achieve X---and that X is left unexplained.
I believe I mentioned this elsewhere but I think that at some point this
logic will probably have to be tweaked again at some point so that a
server may be able to prefer one version to another.
That being said I can definitely add a comment indicating how this code
selects the version and that it can be used to ensure that the latest
and greatest protocol version is used.
--
Brandon Williams
