Subject: Re: [RFC/WIP PATCH 09/11] transport: get_refs_via_connect exchanges capabilities before refs.

From: Jeff King <[email-masked]>
Date: Wed, 27 May 2015 03:06:03 -0400

On Tue, May 26, 2015 at 03:01:13PM -0700, Stefan Beller wrote:

> +	switch (version) {
> +		default: /*
> +			  * Configured a protocol version > 2?
> +			  * Try version 2 as it's the most future proof.
> +			  */
> +			/* fall through */

Same comment here as earlier. If we do a v3, it might not be compatible
with v2. Shouldn't we bail if the user asked for it?

> +		case 2: /* first talk about capabilities, then get the heads */
> +			get_remote_capabilities(data->fd[0], NULL, 0);
> +			request_capabilities(data->fd[1]);
> +			get_remote_heads(data->fd[0], NULL, 0, &refs,
> +					 for_push ? REF_NORMAL : 0,
> +					 &data->extra_have,
> +					 &data->shallow);
> +			break;
> +		case 1: /* configured version 1, fall through */
> +		case 0: /* unconfigured, use first protocol */
> +			get_remote_heads(data->fd[0], NULL, 0, &refs,
> +					 for_push ? REF_NORMAL : 0,
> +					 &data->extra_have,
> +					 &data->shallow);
> +			break;
> +	}

I actually kind of wonder if we should just die("BUG") here on seeing
"0". Is this low level the right place to do the "default to v1"?
Because eventually we're going to want to default to v2, I would think
(after a few years have passed, at least).  It seems like we should be
making that decision centrally when we init the transport options.

-Peff