Re: [PATCH v3 10/35] protocol: introduce enum protocol_version value protocol_v2

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 27, 2018, 06:18 UTC
Message-ID: <20180227061813.GF65699@aiede.svl.corp.google.com>
In-Reply-To: <20180207011312.189834-11-bmwill@google.com>

Hi,

Brandon Williams wrote:

Show 5 quoted lines

> Introduce protocol_v2, a new value for 'enum protocol_version'.
> Subsequent patches will fill in the implementation of protocol_v2.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---

Yay!

[...]

Show 8 quoted lines

> +++ b/builtin/fetch-pack.c
> @@ -201,6 +201,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
>  			   PACKET_READ_GENTLE_ON_EOF);
>  
>  	switch (discover_version(&reader)) {
> +	case protocol_v2:
> +		die("support for protocol v2 not implemented yet");
> +		break;

This code goes away in a later patch, so no need to do anything about this, but the 'break' is redundant after the 'die'.

[...]

Show 12 quoted lines

> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1963,6 +1963,12 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>  		unpack_limit = receive_unpack_limit;
>  
>  	switch (determine_protocol_version_server()) {
> +	case protocol_v2:
> +		/*
> +		 * push support for protocol v2 has not been implemented yet,
> +		 * so ignore the request to use v2 and fallback to using v0.
> +		 */
> +		break;

As you mentioned in the cover letter, it's probably worth doing the same fallback on the client side (send-pack), too.

Otherwise when this client talks to a new-enough server, it would request protocol v2 and then get confused when the server responds with the protocol v2 it requested.

Thanks, Jonathan