Quantcast

[WIP PATCH 00/14] Protocol v2 patches

classic Classic list List threaded Threaded
40 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[WIP PATCH 00/14] Protocol v2 patches

Stefan Beller-4
Hi David,

here are my patches for a protocol v2.

("Negotiate capabilities before doing anything else", or as code:

        static void upload_pack_version_2(void)
        {
                send_capabilities_version_2();
                receive_capabilities_version_2();

                /* The rest of the protocol stays the same, capabilities advertising
                   is disabled though. */
                advertise_capabilities = 0;
                upload_pack();
        }
)

They are rough and unfinished as you can see by the tailing WIPs.
However the plumbing (upload-pack and fetch-pack) works and we'd need to
integrate that into user porcelains, i.e. fetch, clone, push.

Also we need to add tests for all the options again, so we'd need to be smart
about testing that.

I am not sure if it makes sense to integrate that with the http series, though.

Thanks,
Stefan

Nguyễn Thái Ngọc Duy (1):
  upload-pack: make client capability parsing code a separate function

Stefan Beller (13):
  upload-pack.c: Refactor capability advertising
  upload-pack-2: Implement the version 2 of upload-pack
  connect: rewrite feature parsing to work on string_list
  transport: add infrastructure to support a protocol version number
  remote.h: add get_remote_capabilities, request_capabilities
  fetch-pack: move capability selection out of do_fetch_pack
  fetch-pack: factor out get_selected_capabilities_list
  fetch-pack: Add negotiate_capabilities
  do_fetch_pack: select capabilities for transport version 1 only
  builtin/fetch-pack: add argument for transport version
  Add test for fetch-pack
  WIP add test for git pull
  WIP test git fetch

 .gitignore             |   1 +
 Makefile               |   4 ++
 builtin/fetch-pack.c   |  20 ++++++-
 builtin/receive-pack.c |  15 +++--
 connect.c              | 141 +++++++++++++++++++++++++++++--------------
 connect.h              |   2 +-
 fetch-pack.c           | 102 ++++++++++++++++++++++++-------
 fetch-pack.h           |   7 +++
 remote.c               |   2 +
 remote.h               |   5 ++
 t/t5500-fetch-pack.sh  |  21 +++++++
 t/t5510-fetch.sh       |   5 ++
 t/t5520-pull.sh        |   6 ++
 transport-helper.c     |   1 +
 transport.c            |  20 ++++++-
 transport.h            |   8 +++
 upload-pack-2.c        |   1 +
 upload-pack.c          | 159 ++++++++++++++++++++++++++++++++++++-------------
 18 files changed, 403 insertions(+), 117 deletions(-)
 create mode 120000 upload-pack-2.c

--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 01/14] upload-pack: make client capability parsing code a separate function

Stefan Beller-4
From: Nguyễn Thái Ngọc Duy <[hidden email]>

Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]>
Signed-off-by: Stefan Beller <[hidden email]>
---
 upload-pack.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index dc802a0..aaaf883 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -535,6 +535,28 @@ error:
  }
 }
 
+static void parse_features(const char *features)
+{
+ if (parse_feature_request(features, "multi_ack_detailed"))
+ multi_ack = 2;
+ else if (parse_feature_request(features, "multi_ack"))
+ multi_ack = 1;
+ if (parse_feature_request(features, "no-done"))
+ no_done = 1;
+ if (parse_feature_request(features, "thin-pack"))
+ use_thin_pack = 1;
+ if (parse_feature_request(features, "ofs-delta"))
+ use_ofs_delta = 1;
+ if (parse_feature_request(features, "side-band-64k"))
+ use_sideband = LARGE_PACKET_MAX;
+ else if (parse_feature_request(features, "side-band"))
+ use_sideband = DEFAULT_PACKET_MAX;
+ if (parse_feature_request(features, "no-progress"))
+ no_progress = 1;
+ if (parse_feature_request(features, "include-tag"))
+ use_include_tag = 1;
+}
+
 static void receive_needs(void)
 {
  struct object_array shallows = OBJECT_ARRAY_INIT;
@@ -544,7 +566,6 @@ static void receive_needs(void)
  shallow_nr = 0;
  for (;;) {
  struct object *o;
- const char *features;
  unsigned char sha1_buf[20];
  char *line = packet_read_line(0, NULL);
  reset_timeout();
@@ -579,26 +600,7 @@ static void receive_needs(void)
  die("git upload-pack: protocol error, "
     "expected to get sha, not '%s'", line);
 
- features = line + 45;
-
- if (parse_feature_request(features, "multi_ack_detailed"))
- multi_ack = 2;
- else if (parse_feature_request(features, "multi_ack"))
- multi_ack = 1;
- if (parse_feature_request(features, "no-done"))
- no_done = 1;
- if (parse_feature_request(features, "thin-pack"))
- use_thin_pack = 1;
- if (parse_feature_request(features, "ofs-delta"))
- use_ofs_delta = 1;
- if (parse_feature_request(features, "side-band-64k"))
- use_sideband = LARGE_PACKET_MAX;
- else if (parse_feature_request(features, "side-band"))
- use_sideband = DEFAULT_PACKET_MAX;
- if (parse_feature_request(features, "no-progress"))
- no_progress = 1;
- if (parse_feature_request(features, "include-tag"))
- use_include_tag = 1;
+ parse_features(line + 45);
 
  o = parse_object(sha1_buf);
  if (!o)
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 02/14] upload-pack.c: Refactor capability advertising

Stefan Beller-4
In reply to this post by Stefan Beller-4
Instead of having the capabilities in a local string, keep them
in a struct outside the function. This will allow us in a later patch
to easily reuse the capabilities in version 2 of the protocol.

Signed-off-by: Stefan Beller <[hidden email]>
---
 upload-pack.c | 59 ++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index aaaf883..85381d5 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -719,37 +719,58 @@ static void format_symref_info(struct strbuf *buf, struct string_list *symref)
  strbuf_addf(buf, " symref=%s:%s", item->string, (char *)item->util);
 }
 
+static int advertise_capabilities = 1;
+const char *all_capabilities[] = {
+ "multi_ack",
+ "thin-pack",
+ "side-band",
+ "side-band-64k",
+ "ofs-delta",
+ "shallow",
+ "no-progress",
+ "include-tag",
+ "multi_ack_detailed",
+ "allow-tip-sha1-in-want",
+ "allow-reachable-sha1-in-want",
+ "no-done",
+};
+
 static int send_ref(const char *refname, const struct object_id *oid,
     int flag, void *cb_data)
 {
- static const char *capabilities = "multi_ack thin-pack side-band"
- " side-band-64k ofs-delta shallow no-progress"
- " include-tag multi_ack_detailed";
  const char *refname_nons = strip_namespace(refname);
  struct object_id peeled;
 
  if (mark_our_ref(refname_nons, refname, oid))
  return 0;
 
- if (capabilities) {
- struct strbuf symref_info = STRBUF_INIT;
-
- format_symref_info(&symref_info, cb_data);
- packet_write(1, "%s %s%c%s%s%s%s%s agent=%s\n",
-     oid_to_hex(oid), refname_nons,
-     0, capabilities,
-     (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?
-     " allow-tip-sha1-in-want" : "",
-     (allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1) ?
-     " allow-reachable-sha1-in-want" : "",
-     stateless_rpc ? " no-done" : "",
-     symref_info.buf,
-     git_user_agent_sanitized());
- strbuf_release(&symref_info);
+ if (advertise_capabilities) {
+ int i;
+ struct strbuf capabilities = STRBUF_INIT;
+
+ for (i = 0; i < ARRAY_SIZE(all_capabilities); i++) {
+ const char *cap = all_capabilities[i];
+ if (!strcmp(cap, "allow-tip-sha1-in-want")
+    && !(allow_unadvertised_object_request & ALLOW_TIP_SHA1))
+ continue;
+ if (!strcmp(cap, "allow-reachable-sha1-in-want")
+    && !(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1))
+ continue;
+ if (!strcmp(cap, "no-done") && !stateless_rpc)
+ continue;
+ strbuf_addf(&capabilities, " %s", cap);
+ }
+
+ format_symref_info(&capabilities, cb_data);
+ strbuf_addf(&capabilities, " agent=%s", git_user_agent_sanitized());
+
+ packet_write(1, "%s %s%c%s\n", oid_to_hex(oid), refname_nons,
+     0, capabilities.buf);
+ strbuf_release(&capabilities);
+ advertise_capabilities = 0;
  } else {
  packet_write(1, "%s %s\n", oid_to_hex(oid), refname_nons);
  }
- capabilities = NULL;
  if (!peel_ref(refname, peeled.hash))
  packet_write(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
  return 0;
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 03/14] upload-pack-2: Implement the version 2 of upload-pack

Stefan Beller-4
In reply to this post by Stefan Beller-4
In upload-pack-2 we send each capability in its own packet buffer.
The construction of upload-pack-2 is a bit unfortunate as I would like
it to not be depending on a symlink linking to upload-pack.c, but I did
not find another easy way to do it. I would like it to generate
upload-pack-2.o from upload-pack.c but with '-DTRANSPORT_VERSION=2' set.

Signed-off-by: Stefan Beller <[hidden email]>
---
 .gitignore      |  1 +
 Makefile        |  4 ++++
 upload-pack-2.c |  1 +
 upload-pack.c   | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 54 insertions(+), 1 deletion(-)
 create mode 120000 upload-pack-2.c

diff --git a/.gitignore b/.gitignore
index 5087ce1..9a6ea8c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -166,6 +166,7 @@
 /git-update-server-info
 /git-upload-archive
 /git-upload-pack
+/git-upload-pack-2
 /git-var
 /git-verify-commit
 /git-verify-pack
diff --git a/Makefile b/Makefile
index a83e322..3d96126 100644
--- a/Makefile
+++ b/Makefile
@@ -580,6 +580,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o
 PROGRAM_OBJS += shell.o
 PROGRAM_OBJS += show-index.o
 PROGRAM_OBJS += upload-pack.o
+PROGRAM_OBJS += upload-pack-2.o
 PROGRAM_OBJS += remote-testsvn.o
 
 # Binary suffix, set to .exe for Windows builds
@@ -648,6 +649,7 @@ OTHER_PROGRAMS = git$X
 # what test wrappers are needed and 'install' will install, in bindir
 BINDIR_PROGRAMS_NEED_X += git
 BINDIR_PROGRAMS_NEED_X += git-upload-pack
+BINDIR_PROGRAMS_NEED_X += git-upload-pack-2
 BINDIR_PROGRAMS_NEED_X += git-receive-pack
 BINDIR_PROGRAMS_NEED_X += git-upload-archive
 BINDIR_PROGRAMS_NEED_X += git-shell
@@ -1730,6 +1732,8 @@ $(BUILT_INS): git$X
  ln -s $< $@ 2>/dev/null || \
  cp $< $@
 
+$X%-2.o: EXTRA_CPPFLAGS = '-DTRANSPORT_VERSION=2'
+
 common-cmds.h: generate-cmdlist.sh command-list.txt
 
 common-cmds.h: $(wildcard Documentation/git-*.txt)
diff --git a/upload-pack-2.c b/upload-pack-2.c
new file mode 120000
index 0000000..e30a871
--- /dev/null
+++ b/upload-pack-2.c
@@ -0,0 +1 @@
+upload-pack.c
\ No newline at end of file
diff --git a/upload-pack.c b/upload-pack.c
index 85381d5..edfd417 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -820,6 +820,45 @@ static void upload_pack(void)
  }
 }
 
+#if (TRANSPORT_VERSION == 2)
+static void send_capabilities_version_2(void)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(all_capabilities); i++) {
+ const char *cap = all_capabilities[i];
+ if (!strcmp(cap, "allow-tip-sha1-in-want")
+    && !(allow_unadvertised_object_request & ALLOW_TIP_SHA1))
+ continue;
+ if (!strcmp(cap, "no-done") && !stateless_rpc)
+ continue;
+ packet_write(1, "%s\n", cap);
+ }
+
+ packet_write(1, "agent=%s\n", git_user_agent_sanitized());
+ packet_flush(1);
+}
+
+static void receive_capabilities_version_2(void)
+{
+ char *line = packet_read_line(0, NULL);
+ while (line) {
+ parse_features(line);
+ line = packet_read_line(0, NULL);
+ }
+}
+
+static void upload_pack_version_2(void)
+{
+ send_capabilities_version_2();
+ receive_capabilities_version_2();
+
+ /* The rest of the protocol stays the same, capabilities advertising
+   is disabled though. */
+ advertise_capabilities = 0;
+ upload_pack();
+}
+#endif
+
 static int upload_pack_config(const char *var, const char *value, void *unused)
 {
  if (!strcmp("uploadpack.allowtipsha1inwant", var)) {
@@ -847,8 +886,11 @@ int main(int argc, char **argv)
  int strict = 0;
 
  git_setup_gettext();
-
+#if TRANSPORT_VERSION == 2
+ packet_trace_identity("upload-pack-2");
+#else
  packet_trace_identity("upload-pack");
+#endif
  git_extract_argv0_path(argv[0]);
  check_replace_refs = 0;
 
@@ -891,6 +933,11 @@ int main(int argc, char **argv)
  die("'%s' does not appear to be a git repository", dir);
 
  git_config(upload_pack_config, NULL);
+
+#if TRANSPORT_VERSION == 2
+ upload_pack_version_2();
+#else
  upload_pack();
+#endif
  return 0;
 }
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 04/14] connect: rewrite feature parsing to work on string_list

Stefan Beller-4
In reply to this post by Stefan Beller-4
Later on when we introduce the version 2 transport protocol, the
capabilities will not be transported in one lone string but each
capability will be carried in its own pkt line.

To reuse existing infrastructure we would either need to join the
capabilities into a single string again later or refactor the current
capability parsing to be using a data structure which fits both
versions of the transport protocol. We chose to implement the later.

Signed-off-by: Stefan Beller <[hidden email]>
---
 builtin/receive-pack.c | 15 ++++++---
 connect.c              | 82 +++++++++++++++++++++++---------------------------
 connect.h              |  2 +-
 upload-pack.c          | 13 ++++++--
 4 files changed, 58 insertions(+), 54 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a744437..4e98514 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1429,16 +1429,21 @@ static struct command *read_head_info(struct sha1_array *shallow)
 
  linelen = strlen(line);
  if (linelen < len) {
- const char *feature_list = line + linelen + 1;
- if (parse_feature_request(feature_list, "report-status"))
+ struct string_list feature_list = STRING_LIST_INIT_DUP;
+ string_list_split(&feature_list,
+  line + linelen + 1,
+  ' ', -1);
+ if (parse_feature_request(&feature_list, "report-status"))
  report_status = 1;
- if (parse_feature_request(feature_list, "side-band-64k"))
+ if (parse_feature_request(&feature_list, "side-band-64k"))
  use_sideband = LARGE_PACKET_MAX;
- if (parse_feature_request(feature_list, "quiet"))
+ if (parse_feature_request(&feature_list, "quiet"))
  quiet = 1;
  if (advertise_atomic_push
-    && parse_feature_request(feature_list, "atomic"))
+    && parse_feature_request(&feature_list, "atomic"))
  use_atomic = 1;
+
+ string_list_clear(&feature_list, 1);
  }
 
  if (!strcmp(line, "push-cert")) {
diff --git a/connect.c b/connect.c
index c53f3f1..79505fb 100644
--- a/connect.c
+++ b/connect.c
@@ -11,8 +11,8 @@
 #include "sha1-array.h"
 #include "transport.h"
 
-static char *server_capabilities;
-static const char *parse_feature_value(const char *, const char *, int *);
+struct string_list server_capabilities = STRING_LIST_INIT_DUP;
+static const char *parse_feature_value(struct string_list *, const char *, int *);
 
 static int check_ref(const char *name, unsigned int flags)
 {
@@ -81,18 +81,18 @@ reject:
 
 static void annotate_refs_with_symref_info(struct ref *ref)
 {
+ struct string_list_item *item;
  struct string_list symref = STRING_LIST_INIT_DUP;
- const char *feature_list = server_capabilities;
 
- while (feature_list) {
- int len;
+ for_each_string_list_item(item, &server_capabilities) {
  const char *val;
 
- val = parse_feature_value(feature_list, "symref", &len);
- if (!val)
- break;
- parse_one_symref_info(&symref, val, len);
- feature_list = val + 1;
+ if (skip_prefix(item->string, "symref", &val)) {
+ if (!val)
+ continue;
+ val++; /* skip the = */
+ parse_one_symref_info(&symref, val, strlen(val));
+ }
  }
  string_list_sort(&symref);
 
@@ -155,9 +155,12 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
  name = buffer + GIT_SHA1_HEXSZ + 1;
 
  name_len = strlen(name);
+
  if (len != name_len + GIT_SHA1_HEXSZ + 1) {
- free(server_capabilities);
- server_capabilities = xstrdup(name + name_len + 1);
+ string_list_clear(&server_capabilities, 1);
+ string_list_split(&server_capabilities,
+  name + name_len + 1,
+  ' ', -1);
  }
 
  if (extra_have && !strcmp(name, ".have")) {
@@ -179,51 +182,40 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
  return list;
 }
 
-static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)
+static const char *parse_feature_value(struct string_list *feature_list, const char *feature, int *lenp)
 {
- int len;
-
- if (!feature_list)
- return NULL;
-
- len = strlen(feature);
- while (*feature_list) {
- const char *found = strstr(feature_list, feature);
- if (!found)
- return NULL;
- if (feature_list == found || isspace(found[-1])) {
- const char *value = found + len;
- /* feature with no value (e.g., "thin-pack") */
- if (!*value || isspace(*value)) {
- if (lenp)
- *lenp = 0;
- return value;
- }
- /* feature with a value (e.g., "agent=git/1.2.3") */
- else if (*value == '=') {
- value++;
- if (lenp)
- *lenp = strcspn(value, " \t\n");
- return value;
- }
- /*
- * otherwise we matched a substring of another feature;
- * keep looking
- */
+ const char *value;
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, feature_list) {
+ if (!skip_prefix(item->string, feature, &value))
+ continue;
+
+ /* feature with no value (e.g., "thin-pack") */
+ if (!*value) {
+ if (lenp)
+ *lenp = 0;
+ return value;
+ }
+ /* feature with a value (e.g., "agent=git/1.2.3") */
+ else if (*value == '=') {
+ value++;
+ if (lenp)
+ *lenp = strlen(value);
+ return value;
  }
- feature_list = found + 1;
  }
  return NULL;
 }
 
-int parse_feature_request(const char *feature_list, const char *feature)
+int parse_feature_request(struct string_list *feature_list, const char *feature)
 {
  return !!parse_feature_value(feature_list, feature, NULL);
 }
 
 const char *server_feature_value(const char *feature, int *len)
 {
- return parse_feature_value(server_capabilities, feature, len);
+ return parse_feature_value(&server_capabilities, feature, len);
 }
 
 int server_supports(const char *feature)
diff --git a/connect.h b/connect.h
index 01f14cd..eaf21c2 100644
--- a/connect.h
+++ b/connect.h
@@ -9,7 +9,7 @@ extern struct child_process *git_connect(int fd[2], const char *url, const char
 extern int finish_connect(struct child_process *conn);
 extern int git_connection_is_socket(struct child_process *conn);
 extern int server_supports(const char *feature);
-extern int parse_feature_request(const char *features, const char *feature);
+extern int parse_feature_request(struct string_list *, const char *feature);
 extern const char *server_feature_value(const char *feature, int *len_ret);
 extern int url_is_local_not_ssh(const char *url);
 
diff --git a/upload-pack.c b/upload-pack.c
index edfd417..4e69b8e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -535,7 +535,7 @@ error:
  }
 }
 
-static void parse_features(const char *features)
+static void parse_features(struct string_list *features)
 {
  if (parse_feature_request(features, "multi_ack_detailed"))
  multi_ack = 2;
@@ -567,7 +567,9 @@ static void receive_needs(void)
  for (;;) {
  struct object *o;
  unsigned char sha1_buf[20];
+ struct string_list list = STRING_LIST_INIT_DUP;
  char *line = packet_read_line(0, NULL);
+
  reset_timeout();
  if (!line)
  break;
@@ -600,7 +602,9 @@ static void receive_needs(void)
  die("git upload-pack: protocol error, "
     "expected to get sha, not '%s'", line);
 
- parse_features(line + 45);
+ string_list_split(&list, line + 45, ' ', -1);
+ parse_features(&list);
+ string_list_clear(&list, 1);
 
  o = parse_object(sha1_buf);
  if (!o)
@@ -840,9 +844,12 @@ static void send_capabilities_version_2(void)
 
 static void receive_capabilities_version_2(void)
 {
+ struct string_list list = STRING_LIST_INIT_NODUP;
  char *line = packet_read_line(0, NULL);
  while (line) {
- parse_features(line);
+ string_list_append(&list, line);
+ parse_features(&list);
+ string_list_clear(&list, 1);
  line = packet_read_line(0, NULL);
  }
 }
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 05/14] transport: add infrastructure to support a protocol version number

Stefan Beller-4
In reply to this post by Stefan Beller-4
Signed-off-by: Stefan Beller <[hidden email]>
---
 fetch-pack.h       |  1 +
 remote.c           |  2 ++
 remote.h           |  2 ++
 transport-helper.c |  1 +
 transport.c        | 20 ++++++++++++++++++--
 transport.h        |  8 ++++++++
 6 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/fetch-pack.h b/fetch-pack.h
index bb7fd76..3314362 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -8,6 +8,7 @@ struct sha1_array;
 
 struct fetch_pack_args {
  const char *uploadpack;
+ int transport_version;
  int unpacklimit;
  int depth;
  unsigned quiet:1;
diff --git a/remote.c b/remote.c
index 28fd676..760611d 100644
--- a/remote.c
+++ b/remote.c
@@ -9,6 +9,7 @@
 #include "string-list.h"
 #include "mergesort.h"
 #include "argv-array.h"
+#include "transport.h"
 
 enum map_direction { FROM_SRC, FROM_DST };
 
@@ -164,6 +165,7 @@ static struct remote *make_remote(const char *name, int len)
  return ret;
 
  ret = xcalloc(1, sizeof(struct remote));
+ ret->transport_version = DEFAULT_TRANSPORT_VERSION;
  ret->prune = -1;  /* unspecified */
  ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
  remotes[remotes_nr++] = ret;
diff --git a/remote.h b/remote.h
index c21fd37..cdb25d0 100644
--- a/remote.h
+++ b/remote.h
@@ -51,6 +51,8 @@ struct remote {
  const char *receivepack;
  const char *uploadpack;
 
+ int transport_version;
+
  /*
  * for curl remotes only
  */
diff --git a/transport-helper.c b/transport-helper.c
index b934183..3cb386f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -248,6 +248,7 @@ static int disconnect_helper(struct transport *transport)
 }
 
 static const char *unsupported_options[] = {
+ TRANS_OPT_TRANSPORTVERSION,
  TRANS_OPT_UPLOADPACK,
  TRANS_OPT_RECEIVEPACK,
  TRANS_OPT_THIN,
diff --git a/transport.c b/transport.c
index 095e61f..608b92c 100644
--- a/transport.c
+++ b/transport.c
@@ -151,6 +151,16 @@ static int set_git_option(struct git_transport_options *opts,
  die("transport: invalid depth option '%s'", value);
  }
  return 0;
+ } else if (!strcmp(name, TRANS_OPT_TRANSPORTVERSION)) {
+ if (!value)
+ opts->transport_version = DEFAULT_TRANSPORT_VERSION;
+ else {
+ char *end;
+ opts->transport_version = strtol(value, &end, 0);
+ if (*end)
+ die("transport: invalid transport version option '%s'", value);
+ }
+ return 0;
  }
  return 1;
 }
@@ -203,6 +213,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 
  memset(&args, 0, sizeof(args));
  args.uploadpack = data->options.uploadpack;
+ args.transport_version = data->options.transport_version;
  args.keep_pack = data->options.keep;
  args.lock_pack = 1;
  args.use_thin_pack = data->options.thin;
@@ -694,6 +705,8 @@ struct transport *transport_get(struct remote *remote, const char *url)
  ret->connect = connect_git;
  ret->disconnect = disconnect_git;
  ret->smart_options = &(data->options);
+ ret->smart_options->transport_version =
+ DEFAULT_TRANSPORT_VERSION;
 
  data->conn = NULL;
  data->got_remote_heads = 0;
@@ -706,12 +719,15 @@ struct transport *transport_get(struct remote *remote, const char *url)
 
  if (ret->smart_options) {
  ret->smart_options->thin = 1;
- ret->smart_options->uploadpack = "git-upload-pack";
+ ret->smart_options->uploadpack = DEFAULT_TRANSPORT_UPLOAD_PACK;
  if (remote->uploadpack)
  ret->smart_options->uploadpack = remote->uploadpack;
- ret->smart_options->receivepack = "git-receive-pack";
+ ret->smart_options->receivepack = DEFAULT_TRANSPORT_RECEIVE_PACK;
  if (remote->receivepack)
  ret->smart_options->receivepack = remote->receivepack;
+ if (remote->transport_version)
+ ret->smart_options->transport_version =
+ remote->transport_version;
  }
 
  return ret;
diff --git a/transport.h b/transport.h
index c681408..af90529 100644
--- a/transport.h
+++ b/transport.h
@@ -13,6 +13,7 @@ struct git_transport_options {
  unsigned self_contained_and_connected : 1;
  unsigned update_shallow : 1;
  int depth;
+ int transport_version;
  const char *uploadpack;
  const char *receivepack;
  struct push_cas_option *cas;
@@ -188,6 +189,13 @@ int transport_restrict_protocols(void);
 /* Send push certificates */
 #define TRANS_OPT_PUSH_CERT "pushcert"
 
+/* Use a new version of the git protocol */
+#define TRANS_OPT_TRANSPORTVERSION "transportversion"
+
+#define DEFAULT_TRANSPORT_VERSION 1
+#define DEFAULT_TRANSPORT_UPLOAD_PACK "git-upload-pack"
+#define DEFAULT_TRANSPORT_RECEIVE_PACK "git-receive-pack"
+
 /**
  * Returns 0 if the option was used, non-zero otherwise. Prints a
  * message to stderr if the option is not used.
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 06/14] remote.h: add get_remote_capabilities, request_capabilities

Stefan Beller-4
In reply to this post by Stefan Beller-4
Instead of calling get_remote_heads as a first command during the
protocol exchange, we need to have fine grained control over the
capability negotiation in version 2 of the protocol.

Introduce get_remote_capabilities, which will just listen to
capabilities of the remote and request_capabilities which will
tell the selection of capabilities to the remote.

Signed-off-by: Stefan Beller <[hidden email]>
---
 connect.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h  |  3 +++
 2 files changed, 62 insertions(+)

diff --git a/connect.c b/connect.c
index 79505fb..1ba9a0f 100644
--- a/connect.c
+++ b/connect.c
@@ -10,6 +10,7 @@
 #include "string-list.h"
 #include "sha1-array.h"
 #include "transport.h"
+#include "version.h"
 
 struct string_list server_capabilities = STRING_LIST_INIT_DUP;
 static const char *parse_feature_value(struct string_list *, const char *, int *);
@@ -106,6 +107,64 @@ static void annotate_refs_with_symref_info(struct ref *ref)
  string_list_clear(&symref, 0);
 }
 
+const char *known_capabilities[] = {
+ "multi_ack",
+ "thin-pack",
+ "side-band",
+ "side-band-64k",
+ "ofs-delta",
+ "shallow",
+ "no-progress",
+ "include-tag",
+ "multi_ack_detailed",
+ "allow-tip-sha1-in-want",
+ "allow-reachable-sha1-in-want",
+ "no-done",
+};
+
+static int keep_capability(char *line)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(known_capabilities); i++)
+ if (starts_with(line, known_capabilities[i]))
+ return 1;
+ return 0;
+}
+
+void get_remote_capabilities(int in, char *src_buf, size_t src_len)
+{
+ for (;;) {
+ int len;
+ char *line = packet_buffer;
+
+ len = packet_read(in, &src_buf, &src_len,
+  packet_buffer, sizeof(packet_buffer),
+  PACKET_READ_GENTLE_ON_EOF |
+  PACKET_READ_CHOMP_NEWLINE);
+ if (len < 0)
+ die_initial_contact(0);
+
+ if (!len)
+ break;
+
+ /*
+ * We need to ignore and drop unknown capabilities as they
+ * may be huge.
+ */
+ if (keep_capability(line))
+ string_list_append(&server_capabilities, line);
+ }
+}
+
+
+void request_capabilities(int out, struct string_list *list)
+{
+ struct string_list_item *item;
+ for_each_string_list_item(item, list)
+ packet_write(out, "%s\n", item->string);
+ packet_flush(out);
+}
+
 /*
  * Read all the refs from the other end
  */
diff --git a/remote.h b/remote.h
index cdb25d0..534282b 100644
--- a/remote.h
+++ b/remote.h
@@ -153,6 +153,9 @@ extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
      struct sha1_array *extra_have,
      struct sha1_array *shallow);
 
+extern void get_remote_capabilities(int in, char *src_buf, size_t src_len);
+extern void request_capabilities(int out, struct string_list*);
+
 int resolve_remote_symref(struct ref *ref, struct ref *list);
 int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);
 
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 07/14] fetch-pack: move capability selection out of do_fetch_pack

Stefan Beller-4
In reply to this post by Stefan Beller-4
Later in version 2 of the pack protocol the selection of capabilities
happens at another step of the protocol, so move out the current
capability selection, so we can reuse it later more easily.

Signed-off-by: Stefan Beller <[hidden email]>
---
 fetch-pack.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index f96f6df..53f6384 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -796,21 +796,11 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
  return strcmp(a->name, b->name);
 }
 
-static struct ref *do_fetch_pack(struct fetch_pack_args *args,
- int fd[2],
- const struct ref *orig_ref,
- struct ref **sought, int nr_sought,
- struct shallow_info *si,
- char **pack_lockfile)
+static void select_capabilities(struct fetch_pack_args *args)
 {
- struct ref *ref = copy_ref_list(orig_ref);
- unsigned char sha1[20];
  const char *agent_feature;
  int agent_len;
 
- sort_ref_list(&ref, ref_compare_name);
- qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
-
  if ((args->depth > 0 || is_repository_shallow()) && !server_supports("shallow"))
  die("Server does not support shallow clients");
  if (server_supports("multi_ack_detailed")) {
@@ -867,6 +857,22 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
  fprintf(stderr, "Server version is %.*s\n",
  agent_len, agent_feature);
  }
+}
+
+static struct ref *do_fetch_pack(struct fetch_pack_args *args,
+ int fd[2],
+ const struct ref *orig_ref,
+ struct ref **sought, int nr_sought,
+ struct shallow_info *si,
+ char **pack_lockfile)
+{
+ struct ref *ref = copy_ref_list(orig_ref);
+ unsigned char sha1[20];
+
+ sort_ref_list(&ref, ref_compare_name);
+ qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
+
+ select_capabilities(args);
 
  if (everything_local(args, &ref, sought, nr_sought)) {
  packet_flush(fd[1]);
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 08/14] fetch-pack: factor out get_selected_capabilities_list

Stefan Beller-4
In reply to this post by Stefan Beller-4
This will be used later by both versions of the transport protocol.

Signed-off-by: Stefan Beller <[hidden email]>
---
 fetch-pack.c | 51 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 53f6384..b43490f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -257,6 +257,9 @@ static int next_flush(struct fetch_pack_args *args, int count)
  return count;
 }
 
+static void get_selected_capabilities_list(struct string_list *list,
+   struct fetch_pack_args *args);
+
 static int find_common(struct fetch_pack_args *args,
        int fd[2], unsigned char *result_sha1,
        struct ref *refs)
@@ -302,18 +305,15 @@ static int find_common(struct fetch_pack_args *args,
 
  remote_hex = sha1_to_hex(remote);
  if (!fetching) {
+ struct string_list list = STRING_LIST_INIT_NODUP;
  struct strbuf c = STRBUF_INIT;
- if (multi_ack == 2)     strbuf_addstr(&c, " multi_ack_detailed");
- if (multi_ack == 1)     strbuf_addstr(&c, " multi_ack");
- if (no_done)            strbuf_addstr(&c, " no-done");
- if (use_sideband == 2)  strbuf_addstr(&c, " side-band-64k");
- if (use_sideband == 1)  strbuf_addstr(&c, " side-band");
- if (args->use_thin_pack) strbuf_addstr(&c, " thin-pack");
- if (args->no_progress)   strbuf_addstr(&c, " no-progress");
- if (args->include_tag)   strbuf_addstr(&c, " include-tag");
- if (prefer_ofs_delta)   strbuf_addstr(&c, " ofs-delta");
- if (agent_supported)    strbuf_addf(&c, " agent=%s",
-    git_user_agent_sanitized());
+ struct string_list_item *item;
+ get_selected_capabilities_list(&list, args);
+ for_each_string_list_item(item, &list) {
+ strbuf_addstr(&c, " ");
+ strbuf_addstr(&c, item->string);
+ }
+
  packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
  strbuf_release(&c);
  } else
@@ -859,6 +859,35 @@ static void select_capabilities(struct fetch_pack_args *args)
  }
 }
 
+static void get_selected_capabilities_list(struct string_list *list,
+   struct fetch_pack_args *args)
+{
+ if (multi_ack == 2)
+ string_list_append(list, "multi_ack_detailed");
+ if (multi_ack == 1)
+ string_list_append(list, "multi_ack");
+ if (no_done)
+ string_list_append(list, "no-done");
+ if (use_sideband == 2)
+ string_list_append(list, "side-band-64k");
+ if (use_sideband == 1)
+ string_list_append(list, "side-band");
+ if (args->use_thin_pack)
+ string_list_append(list, "thin-pack");
+ if (args->no_progress)
+ string_list_append(list, "no-progress");
+ if (args->include_tag)
+ string_list_append(list, "include-tag");
+ if (prefer_ofs_delta)
+ string_list_append(list, "ofs-delta");
+ if (agent_supported) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(&buf, "agent=%s", git_user_agent_sanitized());
+ string_list_append(list, buf.buf);
+ strbuf_release(&buf);
+ }
+}
+
 static struct ref *do_fetch_pack(struct fetch_pack_args *args,
  int fd[2],
  const struct ref *orig_ref,
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 09/14] fetch-pack: Add negotiate_capabilities

Stefan Beller-4
In reply to this post by Stefan Beller-4
As in the pack protocol version 2 we want to
negotiate the capabilities before any other exchange
we will have a function which will take care of the
whole negotiation process.

It will be placed in fetch-pack.c for now as there
we have access to its internal variables and we'll
work on a `struct fetch_pack_args`. Eventually we
want to move it to a better place such as transport.c

Signed-off-by: Stefan Beller <[hidden email]>
---
 fetch-pack.c | 14 ++++++++++++++
 fetch-pack.h |  6 ++++++
 2 files changed, 20 insertions(+)

diff --git a/fetch-pack.c b/fetch-pack.c
index b43490f..1544629 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -888,6 +888,20 @@ static void get_selected_capabilities_list(struct string_list *list,
  }
 }
 
+void negotiate_capabilities(int fd[2], struct fetch_pack_args *args)
+{
+ struct string_list list = STRING_LIST_INIT_NODUP;
+
+ get_remote_capabilities(fd[0], NULL, 0);
+
+ select_capabilities(args);
+ get_selected_capabilities_list(&list, args);
+
+ request_capabilities(fd[1], &list);
+
+ string_list_clear(&list, 1);
+}
+
 static struct ref *do_fetch_pack(struct fetch_pack_args *args,
  int fd[2],
  const struct ref *orig_ref,
diff --git a/fetch-pack.h b/fetch-pack.h
index 3314362..198498a 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -29,6 +29,12 @@ struct fetch_pack_args {
 };
 
 /*
+ * In version 2 of the pack protocol we negotiate the capabilities
+ * before the actual transfer of refs and packs.
+ */
+void negotiate_capabilities(int fd[2], struct fetch_pack_args *args);
+
+/*
  * sought represents remote references that should be updated from.
  * On return, the names that were found on the remote will have been
  * marked as such.
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 10/14] do_fetch_pack: select capabilities for transport version 1 only

Stefan Beller-4
In reply to this post by Stefan Beller-4
`select_capabilities` would set local variables depending on the
user selection provided by `args` and the server advertisement which
is kept in a list in connect.c

When talking pack protocol version 2 however this has already happend
before during 'negotiate_capabilities'

Signed-off-by: Stefan Beller <[hidden email]>
---
 fetch-pack.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 1544629..5ca1e97 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -915,7 +915,16 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
  sort_ref_list(&ref, ref_compare_name);
  qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
 
- select_capabilities(args);
+ switch (args->transport_version) {
+ case 2:
+ /* capability selection already happend */
+ break;
+ case 1:
+ select_capabilities(args);
+ break;
+ default:
+ die ("transport version %d not supported", args->transport_version);
+ }
 
  if (everything_local(args, &ref, sought, nr_sought)) {
  packet_flush(fd[1]);
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 11/14] builtin/fetch-pack: add argument for transport version

Stefan Beller-4
In reply to this post by Stefan Beller-4
Signed-off-by: Stefan Beller <[hidden email]>
---
 builtin/fetch-pack.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index bfd0be4..afb614b 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -4,6 +4,7 @@
 #include "remote.h"
 #include "connect.h"
 #include "sha1-array.h"
+#include "transport.h"
 
 static const char fetch_pack_usage[] =
 "git fetch-pack [--all] [--stdin] [--quiet | -q] [--keep | -k] [--thin] "
@@ -56,6 +57,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 
  memset(&args, 0, sizeof(args));
  args.uploadpack = "git-upload-pack";
+ args.transport_version = DEFAULT_TRANSPORT_VERSION;
 
  for (i = 1; i < argc && *argv[i] == '-'; i++) {
  const char *arg = argv[i];
@@ -130,6 +132,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
  args.update_shallow = 1;
  continue;
  }
+ if (starts_with(arg, "--transport-version=")) {
+ args.transport_version = strtol(arg + 20, NULL, 0);
+ continue;
+ }
  usage(fetch_pack_usage);
  }
 
@@ -178,7 +184,19 @@ 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.transport_version) {
+ case 2: /* first talk about capabilities, then get the refs */
+ negotiate_capabilities(fd, &args);
+ /* fall through */
+ case 1:
+ get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);
+ break;
+ default:
+ die("BUG: Transport version %d not supported",
+ args.transport_version);
+ break;
+ }
 
  ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,
  &shallow, pack_lockfile_ptr);
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 12/14] Add test for fetch-pack

Stefan Beller-4
In reply to this post by Stefan Beller-4
Signed-off-by: Stefan Beller <[hidden email]>
---
 t/t5500-fetch-pack.sh | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 91a69fc..2c704ef 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -519,6 +519,7 @@ test_expect_success 'test --all, --depth, and explicit tag' '
 '
 
 test_expect_success 'shallow fetch with tags does not break the repository' '
+ test_when_finished "rm -rf repo1" &&
  mkdir repo1 &&
  (
  cd repo1 &&
@@ -547,6 +548,26 @@ test_expect_success 'fetch-pack can fetch a raw sha1' '
  git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)
 '
 
+test_expect_success 'fetch-pack with protocol version 2' '
+ test_when_finished "rm -rf repo1" &&
+ mkdir repo1 &&
+ (
+ cd repo1 &&
+ git init &&
+ test_commit 1 &&
+ test_commit 2 &&
+ test_commit 3 &&
+ echo "$(git rev-parse master) refs/heads/master" >expected &&
+ mkdir repo2 &&
+ (
+ cd repo2 &&
+ git init &&
+ git fetch-pack --transport-version=2 --upload-pack=git-upload-pack-2 ../.git refs/heads/master >../actual
+ ) &&
+ test_cmp expected actual
+ )
+'
+
 check_prot_path () {
  cat >expected <<-EOF &&
  Diag: url=$1
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 13/14] WIP add test for git pull

Stefan Beller-4
In reply to this post by Stefan Beller-4
Signed-off-by: Stefan Beller <[hidden email]>
---
 t/t5520-pull.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 739c089..9bd1feb 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -599,4 +599,10 @@ test_expect_success 'git pull --rebase against local branch' '
  test file = "$(cat file2)"
 '
 
+test_expect_success 'git pull with protocol version 2' '
+ test_pause &&
+ git pull --transport-version=2
+
+'
+
 test_done
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 14/14] WIP test git fetch

Stefan Beller-4
In reply to this post by Stefan Beller-4
Signed-off-by: Stefan Beller <[hidden email]>
---
 t/t5510-fetch.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 38321d1..ab5d3da 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -687,4 +687,9 @@ test_expect_success 'fetching with auto-gc does not lock up' '
  )
 '
 
+test_expect_success 'fetching with transport protocol 2 works' '
+ test_pause
+ git fetch --transport-protocol=2
+'
+
 test_done
--
2.8.0.32.g71f8beb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 02/14] upload-pack.c: Refactor capability advertising

David Turner
In reply to this post by Stefan Beller-4
On Fri, 2016-04-29 at 16:34 -0700, Stefan Beller wrote:
> Instead of having the capabilities in a local string, keep them
> in a struct outside the function. This will allow us in a later patch

nit: s/struct/array/


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 03/14] upload-pack-2: Implement the version 2 of upload-pack

David Turner
In reply to this post by Stefan Beller-4
On Fri, 2016-04-29 at 16:34 -0700, Stefan Beller wrote:
> In upload-pack-2 we send each capability in its own packet buffer.
> The construction of upload-pack-2 is a bit unfortunate as I would
> like
> it to not be depending on a symlink linking to upload-pack.c, but I
> did
> not find another easy way to do it. I would like it to generate
> upload-pack-2.o from upload-pack.c but with '-DTRANSPORT_VERSION=2'
> set.

Couldn't we check argv[0] and use that to determine protocol?  Then we
could symlink executables rather than source code.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 03/14] upload-pack-2: Implement the version 2 of upload-pack

Stefan Beller-4
On Mon, May 2, 2016 at 10:43 AM, David Turner <[hidden email]> wrote:

> On Fri, 2016-04-29 at 16:34 -0700, Stefan Beller wrote:
>> In upload-pack-2 we send each capability in its own packet buffer.
>> The construction of upload-pack-2 is a bit unfortunate as I would
>> like
>> it to not be depending on a symlink linking to upload-pack.c, but I
>> did
>> not find another easy way to do it. I would like it to generate
>> upload-pack-2.o from upload-pack.c but with '-DTRANSPORT_VERSION=2'
>> set.
>
> Couldn't we check argv[0] and use that to determine protocol?  Then we
> could symlink executables rather than source code.

IIRC I proposed a similar thing earlier, i.e.

    if (argv[0] ends with 2)
        do_protocol_v_2(...)

but that may break (and confuse a lot!) some use cases.
`git fetch` has the documented --upload-pack switch, so as a server-admin
you are free to have git-upload-pack linking to
"git-upload-pack-2.8" but additionally you still have
"git-upload-pack-1.7" or "git-upload-pack-custom-2".

so I think we should not do that :(
I do like to symlink the executables though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 04/14] connect: rewrite feature parsing to work on string_list

David Turner
In reply to this post by Stefan Beller-4
On Fri, 2016-04-29 at 16:34 -0700, Stefan Beller wrote:
> + if (skip_prefix(item->string, "symref", &val)) {
> + if (!val)
> + continue;

This if should never happen (skip_prefix returns 0 in that case).  You
probably meant !*val -- but:

> + val++; /* skip the = */

I think you should instead skip_prefix "symref=" because:
(a) it saves some code.
(b) it allows for capabilities like symref_foo to later be added.

> + struct string_list list = STRING_LIST_INIT_NODUP;

Maybe move the scope of list into the while loop below?

>   char *line = packet_read_line(0, NULL);
>   while (line) {
> - parse_features(line);
> + string_list_append(&list, line);
> + parse_features(&list);
> + string_list_clear(&list, 1);
>   line = packet_read_line(0, NULL);

This is a bit convoluted in the one-feature-per-line case, but I guess
I understand that for the sake of generality it's useful.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 04/14] connect: rewrite feature parsing to work on string_list

Stefan Beller-4
On Mon, May 2, 2016 at 11:18 AM, David Turner <[hidden email]> wrote:

> On Fri, 2016-04-29 at 16:34 -0700, Stefan Beller wrote:
>> +             if (skip_prefix(item->string, "symref", &val)) {
>> +                     if (!val)
>> +                             continue;
>
> This if should never happen (skip_prefix returns 0 in that case).  You
> probably meant !*val -- but:
>
>> +                     val++; /* skip the = */
>
> I think you should instead skip_prefix "symref=" because:
> (a) it saves some code.
> (b) it allows for capabilities like symref_foo to later be added.
>
>> +     struct string_list list = STRING_LIST_INIT_NODUP;
>
> Maybe move the scope of list into the while loop below?
>
>>       char *line = packet_read_line(0, NULL);
>>       while (line) {
>> -             parse_features(line);
>> +             string_list_append(&list, line);
>> +             parse_features(&list);
>> +             string_list_clear(&list, 1);
>>               line = packet_read_line(0, NULL);
>
> This is a bit convoluted in the one-feature-per-line case, but I guess
> I understand that for the sake of generality it's useful.

Thanks for the review,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
12
Loading...