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

2015-05-27 Thread Jeff King
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-26 Thread Eric Sunshine
On Tue, May 26, 2015 at 6:01 PM, Stefan Beller sbel...@google.com wrote:
 transport: get_refs_via_connect exchanges capabilities before refs.

s/exchanges/exchange/
s/\.$//

 Signed-off-by: Stefan Beller sbel...@google.com
 ---
  transport.c | 29 +
  1 file changed, 25 insertions(+), 4 deletions(-)

 diff --git a/transport.c b/transport.c
 index 33644a6..1cd9b77 100644
 --- a/transport.c
 +++ b/transport.c
 @@ -526,12 +526,33 @@ static struct ref *get_refs_via_connect(struct 
 transport *transport, int for_pus
  {
 struct git_transport_data *data = transport-data;
 struct ref *refs;
 +   int version = 0;

 +   if (transport-smart_options)
 +   version = transport-smart_options-transport_version;
 connect_setup(transport, for_push, 0);
 -   get_remote_heads(data-fd[0], NULL, 0, refs,
 -for_push ? REF_NORMAL : 0,
 -data-extra_have,
 -data-shallow);
 +   switch (version) {
 +   default: /*
 + * Configured a protocol version  2?
 + * Try version 2 as it's the most future proof.
 + */
 +   /* fall through */
 +   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;
 +   }
 data-got_remote_heads = 1;

 return refs;
 --
 2.4.1.345.gab207b6.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html