Re: [RFC/WIP PATCH 07/11] fetch-pack: use the configured transport protocol

[Jeff King]

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

 diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
 index 4a6b340..32dc8b0 100644
 --- a/builtin/fetch-pack.c
 +++ b/builtin/fetch-pack.c
 @@ -127,6 +127,10 @@ int cmd_fetch_pack(int argc, const char **argv, const 
 char *prefix)
   args.update_shallow = 1;
   continue;
   }
 + if (!strcmp(--transport-version, arg)) {
 + args.version = strtol(arg + 
 strlen(--transport-version), NULL, 0);
 + continue;
 + }

You strcmp() the arg here, so there can't be anything at arg +
strlen(...), can there? Did you want:

  starts_with(arg, --transports-version=)

here? Or better yet, skip_prefix().

 - get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
 +
 + switch (args.version) {
 + default:
 + case 2:
 + get_remote_capabilities(fd[0], NULL, 0);
 + request_capabilities(fd[1]);
 + break;

Should the default case be to complain about an unknown version,
rather than fall-through to 2?

-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

[RFC/WIP PATCH 07/11] fetch-pack: use the configured transport protocol

[Stefan Beller]

Signed-off-by: Stefan Beller sbel...@google.com
---
 builtin/fetch-pack.c | 17 -
 fetch-pack.h |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 4a6b340..32dc8b0 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -127,6 +127,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
args.update_shallow = 1;
continue;
}
+   if (!strcmp(--transport-version, arg)) {
+   args.version = strtol(arg + 
strlen(--transport-version), NULL, 0);
+   continue;
+   }
usage(fetch_pack_usage);
}
 
@@ -175,7 +179,18 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
if (!conn)
return args.diag_url ? 0 : 1;
}
-   get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
+
+   switch (args.version) {
+   default:
+   case 2:
+   get_remote_capabilities(fd[0], NULL, 0);
+   request_capabilities(fd[1]);
+   break;
+   case 1: /* fall through */
+   case 0:
+   get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
+   break;
+   }
 
ref = fetch_pack(args, fd, conn, ref, dest, sought, nr_sought,
 shallow, pack_lockfile_ptr);
diff --git a/fetch-pack.h b/fetch-pack.h
index bb7fd76..b48b4f5 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -10,6 +10,7 @@ struct fetch_pack_args {
const char *uploadpack;
int unpacklimit;
int depth;
+   int version;
unsigned quiet:1;
unsigned keep_pack:1;
unsigned lock_pack:1;
-- 
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

Re: [RFC/WIP PATCH 07/11] fetch-pack: use the configured transport protocol

[Junio C Hamano]

Stefan Beller sbel...@google.com writes:

 @@ -175,7 +179,18 @@ int cmd_fetch_pack(int argc, const char **argv, const 
 char *prefix)
   if (!conn)
   return args.diag_url ? 0 : 1;
   }
 - get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
 +
 + switch (args.version) {
 + default:
 + case 2:
 + get_remote_capabilities(fd[0], NULL, 0);
 + request_capabilities(fd[1]);
 + break;
 + case 1: /* fall through */
 + case 0:
 + get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
 + break;
 + }

Nice ;-)
--
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 07/11] fetch-pack: use the configured transport protocol

[Stefan Beller]

On Tue, May 26, 2015 at 3:19 PM, Junio C Hamano gits...@pobox.com wrote:
 Stefan Beller sbel...@google.com writes:

 @@ -175,7 +179,18 @@ int cmd_fetch_pack(int argc, const char **argv, const 
 char *prefix)
   if (!conn)
   return args.diag_url ? 0 : 1;
   }
 - get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
 +
 + switch (args.version) {
 + default:
 + case 2:
 + get_remote_capabilities(fd[0], NULL, 0);
 + request_capabilities(fd[1]);
 + break;

Actually this is wrong, we need to actually fall through from here as well,
so we not only talk capabilities negotiation, but then continue
with get_remote_heads.

 + case 1: /* fall through */
 + case 0:
 + get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
 + break;
 + }

 Nice ;-)
--
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