{"thread":{"id":"42038","subject":"[PATCH/RFC 4/6] transport: add refspec list parameters to functions","startedAt":"2016-04-15T19:19:03Z","lastAt":"2016-04-29T23:12:01Z","messageCount":36,"participants":["David Turner","Stefan Beller","Eric Sunshine","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"283563","messageId":"1460747949-3514-1-git-send-email-dturner@twopensource.com","threadId":"42038","inReplyTo":null,"subject":"[PATCH/RFC 0/6] fetch with refspec","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-15T19:19:03Z","receivedAt":"2016-04-15T19:19:03Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"We've got a lot of refs, but pretty frequently we only want to fetch\none. It's silly for the server to send a bunch of refs that the client\nis just going to ignore. Here are some patches that fix that.\n\nLet me know if this seems reasonable.\n\n(and I'll start in on another round of index-helper as soon as this is sent!)\n\nDavid Turner (6):\n http-backend: use argv_array functions\n remote-curl.c: fix variable shadowing\n http-backend: handle refspec argument\n transport: add refspec list parameters to functions\n fetch: pass refspec to http server\n clone: send refspec for single-branch clones\n\n Documentation/technical/protocol-capabilities.txt | 23 +++++++\n builtin/clone.c | 16 ++++-\n builtin/fetch.c | 24 ++++++-\n builtin/ls-remote.c | 2 +-\n builtin/remote.c | 2 +-\n http-backend.c | 23 +++++--\n remote-curl.c | 25 ++++---\n t/t5552-http-fetch-branch.sh | 47 +++++++++++++\n transport-helper.c | 44 ++++++++----\n transport.c | 14 ++--\n transport.h | 4 +-\n upload-pack.c | 81 ++++++++++++++++++++++-\n 12 files changed, 261 insertions(+), 44 deletions(-)\n create mode 100755 t/t5552-http-fetch-branch.sh\n\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283568","messageId":"1460747949-3514-2-git-send-email-dturner@twopensource.com","threadId":"42038","inReplyTo":"1460747949-3514-1-git-send-email-dturner@twopensource.com","subject":"[PATCH/RFC 1/6] http-backend: use argv_array functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-15T19:19:04Z","receivedAt":"2016-04-15T19:19:04Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Signed-off-by: David Turner \n---\n http-backend.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex 8870a26..a4f0066 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -450,9 +450,7 @@ static void get_info_refs(char *arg)\n \thdr_nocache();\n \n \tif (service_name) {\n-\t\tconst char *argv[] = {NULL /* service name */,\n-\t\t\t\"--stateless-rpc\", \"--advertise-refs\",\n-\t\t\t\".\", NULL};\n+\t\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \t\tstruct rpc_service *svc = select_service(service_name);\n \n \t\tstrbuf_addf(&buf, \"application/x-git-%s-advertisement\",\n@@ -463,9 +461,13 @@ static void get_info_refs(char *arg)\n \t\tpacket_write(1, \"# service=git-%s\\n\", svc->name);\n \t\tpacket_flush(1);\n \n-\t\targv[0] = svc->name;\n-\t\trun_service(argv, 0);\n+\t\targv_array_push(&argv, svc->name);\n+\t\targv_array_push(&argv, \"--stateless-rpc\");\n+\t\targv_array_push(&argv, \"--advertise-refs\");\n \n+\t\targv_array_push(&argv, \".\");\n+\t\trun_service(argv.argv, 0);\n+\t\targv_array_clear(&argv);\n \t} else {\n \t\tselect_getanyfile();\n \t\tfor_each_namespaced_ref(show_text_ref, &buf);\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283565","messageId":"1460747949-3514-3-git-send-email-dturner@twopensource.com","threadId":"42038","inReplyTo":"1460747949-3514-1-git-send-email-dturner@twopensource.com","subject":"[PATCH/RFC 2/6] remote-curl.c: fix variable shadowing","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-15T19:19:05Z","receivedAt":"2016-04-15T19:19:05Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"The local variable 'options' was shadowing a global of the same name.\n\nSigned-off-by: David Turner \n---\n remote-curl.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 15e48e2..b9b6a90 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -254,7 +254,7 @@ static struct discovery *discover_refs(const char *service, int for_push)\n \tstruct strbuf effective_url = STRBUF_INIT;\n \tstruct discovery *last = last_discovery;\n \tint http_ret, maybe_smart = 0;\n-\tstruct http_get_options options;\n+\tstruct http_get_options get_options;\n \n \tif (last && !strcmp(service, last->service))\n \t\treturn last;\n@@ -271,15 +271,15 @@ static struct discovery *discover_refs(const char *service, int for_push)\n \t\tstrbuf_addf(&refs_url, \"service=%s\", service);\n \t}\n \n-\tmemset(&options, 0, sizeof(options));\n-\toptions.content_type = &type;\n-\toptions.charset = &charset;\n-\toptions.effective_url = &effective_url;\n-\toptions.base_url = &url;\n-\toptions.no_cache = 1;\n-\toptions.keep_error = 1;\n+\tmemset(&get_options, 0, sizeof(get_options));\n+\tget_options.content_type = &type;\n+\tget_options.charset = &charset;\n+\tget_options.effective_url = &effective_url;\n+\tget_options.base_url = &url;\n+\tget_options.no_cache = 1;\n+\tget_options.keep_error = 1;\n \n-\thttp_ret = http_get_strbuf(refs_url.buf, &buffer, &options);\n+\thttp_ret = http_get_strbuf(refs_url.buf, &buffer, &get_options);\n \tswitch (http_ret) {\n \tcase HTTP_OK:\n \t\tbreak;\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283566","messageId":"1460747949-3514-4-git-send-email-dturner@twopensource.com","threadId":"42038","inReplyTo":"1460747949-3514-1-git-send-email-dturner@twopensource.com","subject":"[PATCH/RFC 3/6] http-backend: handle refspec argument","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-15T19:19:06Z","receivedAt":"2016-04-15T19:19:06Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Allow clients to pass a \"refspec\" parameter through to upload-pack;\nupload-pack will only advertise refs which match that refspec.\n\nSigned-off-by: David Turner \n---\n http-backend.c | 9 +++++++\n upload-pack.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--\n 2 files changed, 88 insertions(+), 2 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex a4f0066..9731391 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -452,6 +452,7 @@ static void get_info_refs(char *arg)\n \tif (service_name) {\n \t\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \t\tstruct rpc_service *svc = select_service(service_name);\n+\t\tconst char *refspec;\n \n \t\tstrbuf_addf(&buf, \"application/x-git-%s-advertisement\",\n \t\t\tsvc->name);\n@@ -465,6 +466,14 @@ static void get_info_refs(char *arg)\n \t\targv_array_push(&argv, \"--stateless-rpc\");\n \t\targv_array_push(&argv, \"--advertise-refs\");\n \n+\t\trefspec = get_parameter(\"refspec\");\n+\t\tif (refspec) {\n+\t\t\tstruct strbuf interesting_refs = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&interesting_refs, \"--interesting-refs=\");\n+\t\t\tstrbuf_addstr(&interesting_refs, refspec);\n+\t\t\targv_array_push(&argv, interesting_refs.buf);\n+\t\t\tstrbuf_release(&interesting_refs);\n+\t\t}\n \t\targv_array_push(&argv, \".\");\n \t\trun_service(argv.argv, 0);\n \t\targv_array_clear(&argv);\ndiff --git a/upload-pack.c b/upload-pack.c\nindex b3f6653..da140c2 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -52,6 +52,7 @@ static int keepalive = 5;\n static int use_sideband;\n static int advertise_refs;\n static int stateless_rpc;\n+static struct string_list interesting_refspecs = STRING_LIST_INIT_DUP;\n \n static void reset_timeout(void)\n {\n@@ -687,16 +688,61 @@ static void receive_needs(void)\n \tfree(shallows.objects);\n }\n \n-/* return non-zero if the ref is hidden, otherwise 0 */\n+struct refspec_data {\n+\tint has_star;\n+\tsize_t prefixlen;\n+\tsize_t suffixlen;\n+};\n+\n+static int matches_refspec(const char *refspec, struct refspec_data *data,\n+\t\t const char *ref)\n+{\n+\tsize_t len;\n+\n+\tif (!data->has_star)\n+\t\treturn !strcmp(refspec, ref);\n+\n+\tif (strncmp(refspec, ref, data->prefixlen))\n+\t\treturn -1;\n+\n+\tlen = strlen(refspec);\n+\tif (len < data->prefixlen + data->suffixlen)\n+\t\treturn -1;\n+\n+\treturn strcmp(ref + (len - data->suffixlen),\n+\t\t refspec + data->prefixlen + 1);\n+}\n+\n+/*\n+ * return non-zero if the ref is hidden or outside the provided\n+ * refspecs, otherwise 0\n+*/\n static int mark_our_ref(const char *refname, const char *refname_full,\n \t\t\tconst struct object_id *oid)\n {\n \tstruct object *o = lookup_unknown_object(oid->hash);\n+\tstruct string_list_item *item;\n \n \tif (ref_is_hidden(refname, refname_full)) {\n \t\to->flags |= HIDDEN_REF;\n \t\treturn 1;\n \t}\n+\n+\tif (interesting_refspecs.nr) {\n+\t\tint found = 0;\n+\t\t/*\n+\t\t * TODO: this could be faster for large numbers of\n+\t\t * refspecs by using tries or a DFA.\n+\t\t */\n+\t\tfor_each_string_list_item(item, &interesting_refspecs)\n+\t\t\tif (matches_refspec(item->string, item->util, refname)) {\n+\t\t\t\tfound = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\tif (!found)\n+\t\t\treturn 1;\n+\n+\t}\n \to->flags |= OUR_REF;\n \treturn 0;\n }\n@@ -725,7 +771,7 @@ static int send_ref(const char *refname, const struct object_id *oid,\n {\n \tstatic const char *capabilities = \"multi_ack thin-pack side-band\"\n \t\t\" side-band-64k ofs-delta shallow no-progress\"\n-\t\t\" include-tag multi_ack_detailed\";\n+\t\t\" include-tag multi_ack_detailed interesting-refs\";\n \tconst char *refname_nons = strip_namespace(refname);\n \tstruct object_id peeled;\n \n@@ -820,6 +866,24 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \treturn parse_hide_refs_config(var, value, \"uploadpack\");\n }\n \n+static struct refspec_data *make_refspec_data(const char *refspec)\n+{\n+\tstruct refspec_data *data;\n+\tconst char *star;\n+\n+\tdata = xmalloc(sizeof(struct refspec_data));\n+\tstar = strchr(refspec, '*');\n+\tif (star) {\n+\t\tdata->has_star = 1;\n+\t\tdata->prefixlen = star - refspec;\n+\t\tdata->suffixlen = strlen(refspec) - (data->prefixlen + 1);\n+\t} else {\n+\t\tdata->has_star = 0;\n+\t}\n+\n+\treturn data;\n+}\n+\n int main(int argc, char **argv)\n {\n \tchar *dir;\n@@ -841,6 +905,19 @@ int main(int argc, char **argv)\n \t\t\tadvertise_refs = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (starts_with(arg, \"--interesting-refs=\")) {\n+\t\t\tstruct string_list_item *item;\n+\n+\t\t\tstring_list_split(&interesting_refspecs, arg + 19,\n+\t\t\t\t\t ' ', -1);\n+\t\t\tfor_each_string_list_item(item, &interesting_refspecs) {\n+\t\t\t\tif (check_refname_format(item->string,\n+\t\t\t\t\t\t\t REFNAME_REFSPEC_PATTERN))\n+\t\t\t\t\tdie(\"invalid refspec %s\", item->string);\n+\t\t\t\titem->util = make_refspec_data(item->string);\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--stateless-rpc\")) {\n \t\t\tstateless_rpc = 1;\n \t\t\tcontinue;\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283562","messageId":"1460747949-3514-5-git-send-email-dturner@twopensource.com","threadId":"42038","inReplyTo":"1460747949-3514-1-git-send-email-dturner@twopensource.com","subject":"[PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-15T19:19:07Z","receivedAt":"2016-04-15T19:19:07Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Add parameters for a list of refspecs to transport_get_remote_refs and\nget_refs_list. These parameters are presently unused -- soon, we will\nuse them to implement fetches which only learn about a subset of refs.\n\nSigned-off-by: David Turner \n---\n builtin/clone.c | 2 +-\n builtin/fetch.c | 6 ++++--\n builtin/ls-remote.c | 2 +-\n builtin/remote.c | 2 +-\n transport-helper.c | 24 ++++++++++++------------\n transport.c | 14 ++++++++------\n transport.h | 4 ++--\n 7 files changed, 29 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 6616392..91f668c 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1010,7 +1010,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (transport->smart_options && !option_depth)\n \t\ttransport->smart_options->check_self_contained_and_connected = 1;\n \n-\trefs = transport_get_remote_refs(transport);\n+\trefs = transport_get_remote_refs(transport, NULL, 0);\n \n \tif (refs) {\n \t\tmapped_refs = wanted_peer_refs(refs, refspec);\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e4639d8..cafab37 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -221,7 +221,8 @@ static void find_non_local_tags(struct transport *transport,\n \tstruct string_list_item *item = NULL;\n \n \tfor_each_ref(add_existing, &existing_refs);\n-\tfor (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {\n+\tfor (ref = transport_get_remote_refs(transport, NULL, 0);\n+\t ref; ref = ref->next) {\n \t\tif (!starts_with(ref->name, \"refs/tags/\"))\n \t\t\tcontinue;\n \n@@ -301,8 +302,9 @@ static struct ref *get_ref_map(struct transport *transport,\n \n \t/* opportunistically-updated references: */\n \tstruct ref *orefs = NULL, **oref_tail = &orefs;\n+\tconst struct ref *remote_refs;\n \n-\tconst struct ref *remote_refs = transport_get_remote_refs(transport);\n+\tremote_refs = transport_get_remote_refs(transport, NULL, 0);\n \n \tif (refspec_count) {\n \t\tstruct refspec *fetch_refspec;\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 66cdd45..bce706e 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -94,7 +94,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tif (uploadpack != NULL)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);\n \n-\tref = transport_get_remote_refs(transport);\n+\tref = transport_get_remote_refs(transport, NULL, 0);\n \tif (transport_disconnect(transport))\n \t\treturn 1;\n \ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex fda5c2e..5745e8b 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -862,7 +862,7 @@ static int get_remote_ref_states(const char *name,\n \tif (query) {\n \t\ttransport = transport_get(states->remote, states->remote->url_nr > 0 ?\n \t\t\tstates->remote->url[0] : NULL);\n-\t\tremote_refs = transport_get_remote_refs(transport);\n+\t\tremote_refs = transport_get_remote_refs(transport, NULL, 0);\n \t\ttransport_disconnect(transport);\n \n \t\tstates->queried = 1;\ndiff --git a/transport-helper.c b/transport-helper.c\nindex b934183..b5c91d2 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -99,7 +99,7 @@ static void do_take_over(struct transport *transport)\n \n static void standard_options(struct transport *t);\n \n-static struct child_process *get_helper(struct transport *transport)\n+static struct child_process *get_helper(struct transport *transport, const struct refspec *req_refspecs, int req_refspec_nr)\n {\n \tstruct helper_data *data = transport->data;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -267,7 +267,7 @@ static int set_helper_option(struct transport *transport,\n \tstruct strbuf buf = STRBUF_INIT;\n \tint i, ret, is_bool = 0;\n \n-\tget_helper(transport);\n+\tget_helper(transport, NULL, 0);\n \n \tif (!data->option)\n \t\treturn 1;\n@@ -395,7 +395,7 @@ static int fetch_with_fetch(struct transport *transport,\n \n static int get_importer(struct transport *transport, struct child_process *fastimport)\n {\n-\tstruct child_process *helper = get_helper(transport);\n+\tstruct child_process *helper = get_helper(transport, NULL, 0);\n \tstruct helper_data *data = transport->data;\n \tint cat_blob_fd, code;\n \tchild_process_init(fastimport);\n@@ -418,7 +418,7 @@ static int get_exporter(struct transport *transport,\n \t\t\tstruct string_list *revlist_args)\n {\n \tstruct helper_data *data = transport->data;\n-\tstruct child_process *helper = get_helper(transport);\n+\tstruct child_process *helper = get_helper(transport, NULL, 0);\n \tint i;\n \n \tchild_process_init(fastexport);\n@@ -451,7 +451,7 @@ static int fetch_with_import(struct transport *transport,\n \tstruct ref *posn;\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tget_helper(transport);\n+\tget_helper(transport, NULL, 0);\n \n \tif (get_importer(transport, &fastimport))\n \t\tdie(\"Couldn't run fast-import\");\n@@ -523,7 +523,7 @@ static int process_connect_service(struct transport *transport,\n \tint r, duped, ret = 0;\n \tFILE *input;\n \n-\thelper = get_helper(transport);\n+\thelper = get_helper(transport, NULL, 0);\n \n \t/*\n \t * Yes, dup the pipe another time, as we need unbuffered version\n@@ -599,7 +599,7 @@ static int connect_helper(struct transport *transport, const char *name,\n \tstruct helper_data *data = transport->data;\n \n \t/* Get_helper so connect is inited. */\n-\tget_helper(transport);\n+\tget_helper(transport, NULL, 0);\n \tif (!data->connect)\n \t\tdie(\"Operation not supported by protocol.\");\n \n@@ -805,7 +805,7 @@ static int push_refs_with_push(struct transport *transport,\n \tstruct string_list cas_options = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *cas_option;\n \n-\tget_helper(transport);\n+\tget_helper(transport, NULL, 0);\n \tif (!data->push)\n \t\treturn 1;\n \n@@ -888,7 +888,7 @@ static int push_refs_with_export(struct transport *transport,\n \t\t\twarning(\"helper %s does not support 'force'\", data->name);\n \t}\n \n-\thelper = get_helper(transport);\n+\thelper = get_helper(transport, NULL, 0);\n \n \twrite_constant(helper->in, \"export\\n\");\n \n@@ -992,7 +992,7 @@ static int has_attribute(const char *attrs, const char *attr) {\n \t}\n }\n \n-static struct ref *get_refs_list(struct transport *transport, int for_push)\n+static struct ref *get_refs_list(struct transport *transport, const struct refspec *refspecs, int refspec_count, int for_push)\n {\n \tstruct helper_data *data = transport->data;\n \tstruct child_process *helper;\n@@ -1001,11 +1001,11 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \tstruct ref *posn;\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\thelper = get_helper(transport);\n+\thelper = get_helper(transport, refspecs, refspec_count);\n \n \tif (process_connect(transport, for_push)) {\n \t\tdo_take_over(transport);\n-\t\treturn transport->get_refs_list(transport, for_push);\n+\t\treturn transport->get_refs_list(transport, refspecs, refspec_count, for_push);\n \t}\n \n \tif (data->push && for_push)\ndiff --git a/transport.c b/transport.c\nindex 095e61f..e241e42 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -70,7 +70,9 @@ struct bundle_transport_data {\n \tstruct bundle_header header;\n };\n \n-static struct ref *get_refs_from_bundle(struct transport *transport, int for_push)\n+static struct ref *get_refs_from_bundle(struct transport *transport,\n+\t\t\t\t\tconst struct refspec *refspecs,\n+\t\t\t\t\tint refspec_count, int for_push)\n {\n \tstruct bundle_transport_data *data = transport->data;\n \tstruct ref *result = NULL;\n@@ -177,7 +179,7 @@ static int connect_setup(struct transport *transport, int for_push)\n \treturn 0;\n }\n \n-static struct ref *get_refs_via_connect(struct transport *transport, int for_push)\n+static struct ref *get_refs_via_connect(struct transport *transport, const struct refspec *refspecs, int refspec_count, int for_push)\n {\n \tstruct git_transport_data *data = transport->data;\n \tstruct ref *refs;\n@@ -870,7 +872,7 @@ int transport_push(struct transport *transport,\n \t\tif (check_push_refs(local_refs, refspec_nr, refspec) < 0)\n \t\t\treturn -1;\n \n-\t\tremote_refs = transport->get_refs_list(transport, 1);\n+\t\tremote_refs = transport->get_refs_list(transport, NULL, 0, 1);\n \n \t\tif (flags & TRANSPORT_PUSH_ALL)\n \t\t\tmatch_flags |= MATCH_REFS_ALL;\n@@ -949,10 +951,10 @@ int transport_push(struct transport *transport,\n \treturn 1;\n }\n \n-const struct ref *transport_get_remote_refs(struct transport *transport)\n+const struct ref *transport_get_remote_refs(struct transport *transport, const struct refspec *refspecs, int refspec_count)\n {\n \tif (!transport->got_remote_refs) {\n-\t\ttransport->remote_refs = transport->get_refs_list(transport, 0);\n+\t\ttransport->remote_refs = transport->get_refs_list(transport, refspecs, refspec_count, 0);\n \t\ttransport->got_remote_refs = 1;\n \t}\n \n@@ -1099,7 +1101,7 @@ static int refs_from_alternate_cb(struct alternate_object_database *e,\n \tother[len - 8] = '\\0';\n \tremote = remote_get(other);\n \ttransport = transport_get(remote, other);\n-\tfor (extra = transport_get_remote_refs(transport);\n+\tfor (extra = transport_get_remote_refs(transport, NULL, 0);\n \t extra;\n \t extra = extra->next)\n \t\tcb->fn(extra, cb->data);\ndiff --git a/transport.h b/transport.h\nindex c681408..e53d860 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -65,7 +65,7 @@ struct transport {\n \t * the ref without a huge amount of effort, it should store it\n \t * in the ref's old_sha1 field; otherwise it should be all 0.\n \t **/\n-\tstruct ref *(*get_refs_list)(struct transport *transport, int for_push);\n+\tstruct ref *(*get_refs_list)(struct transport *transport, const struct refspec *refspecs, int refspec_count, int for_push);\n \n \t/**\n \t * Fetch the objects for the given refs. Note that this gets\n@@ -207,7 +207,7 @@ int transport_push(struct transport *connection,\n \t\t int refspec_nr, const char **refspec, int flags,\n \t\t unsigned int * reject_reasons);\n \n-const struct ref *transport_get_remote_refs(struct transport *transport);\n+const struct ref *transport_get_remote_refs(struct transport *transport, const struct refspec *refspecs, int refspec_count);\n \n int transport_fetch_refs(struct transport *transport, struct ref *refs);\n void transport_unlock_pack(struct transport *transport);\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283564","messageId":"1460747949-3514-6-git-send-email-dturner@twopensource.com","threadId":"42038","inReplyTo":"1460747949-3514-1-git-send-email-dturner@twopensource.com","subject":"[PATCH/RFC 5/6] fetch: pass refspec to http server","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-15T19:19:08Z","receivedAt":"2016-04-15T19:19:08Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"When fetching over http, send the requested refspec to the server.\nThe server will then only send refs matching that refspec. It is\npermitted for old servers to ignore that parameter, and the client\nwill automatically handle this.\n\nWhen the server has many refs, and the client only wants a few, this\ncan save bandwidth and reduce fetch latency.\n\nSigned-off-by: David Turner \n---\n Documentation/technical/protocol-capabilities.txt | 23 +++++++++++++\n builtin/fetch.c | 20 ++++++++++-\n remote-curl.c | 7 ++++\n t/t5552-http-fetch-branch.sh | 42 +++++++++++++++++++++++\n transport-helper.c | 8 ++++-\n 5 files changed, 98 insertions(+), 2 deletions(-)\n create mode 100755 t/t5552-http-fetch-branch.sh\n\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex eaab6b4..8c4a0b9 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -275,3 +275,26 @@ to accept a signed push certificate, and asks the to be\n included in the push certificate. A send-pack client MUST NOT\n send a push-cert packet unless the receive-pack server advertises\n this capability.\n+\n+interesting-refs\n+----------------\n+\n+For HTTP(S) servers only:\n+\n+Whether or not the upload-pack server advertises this capability,\n+fetch-pack may send a \"refspec\" parameter in the query string of a\n+fetch request. This capability indicates that such a parameter will\n+be respected, in case the client cares.\n+\n+Whenever the receive-pack server gets that parameter, it will not\n+advertise all refs and will instead only advertise refs that match\n+those listed in the header. The parameter is a space-separated list of\n+refs. A ref may optionally contain up to one wildcard.\n+Advertisements will still respect hideRefs.\n+\n+The presence or absence of the \"refspec\" parameter does not affect\n+what refs a client is permitted to fetch; this is still controlled in\n+the normal fashion.\n+\n+This saves time in the presence of a large number of refs, because the\n+client need not wait for the server to send the complete list of refs.\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex cafab37..c22a92f 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -302,9 +302,27 @@ static struct ref *get_ref_map(struct transport *transport,\n \n \t/* opportunistically-updated references: */\n \tstruct ref *orefs = NULL, **oref_tail = &orefs;\n+\tstruct refspec *qualified_refspecs;\n \tconst struct ref *remote_refs;\n \n-\tremote_refs = transport_get_remote_refs(transport, NULL, 0);\n+\tqualified_refspecs = xcalloc(refspec_count, sizeof(*qualified_refspecs));\n+\tfor (i = 0; i < refspec_count; i++) {\n+\t\tif (starts_with(refspecs[i].src, \"refs/\")) {\n+\t\t\tqualified_refspecs[i].src = xstrdup(refspecs[i].src);\n+\t\t} else {\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&buf, \"refs/heads/%s\", refspecs[i].src);\n+\t\t\tqualified_refspecs[i].src = strbuf_detach(&buf, NULL);\n+\t\t}\n+\t}\n+\n+\tremote_refs = transport_get_remote_refs(transport, qualified_refspecs,\n+\t\t\t\t\t\trefspec_count);\n+\n+\tfor (i = 0; i < refspec_count; i++) {\n+\t\tfree(qualified_refspecs[i].src);\n+\t}\n+\tfree(qualified_refspecs);\n \n \tif (refspec_count) {\n \t\tstruct refspec *fetch_refspec;\ndiff --git a/remote-curl.c b/remote-curl.c\nindex b9b6a90..e914d3f 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -20,6 +20,7 @@ static struct strbuf url = STRBUF_INIT;\n struct options {\n \tint verbosity;\n \tunsigned long depth;\n+\tchar *refspec;\n \tunsigned progress : 1,\n \t\tcheck_self_contained_and_connected : 1,\n \t\tcloning : 1,\n@@ -43,6 +44,10 @@ static int set_option(const char *name, const char *value)\n \t\toptions.verbosity = v;\n \t\treturn 0;\n \t}\n+\telse if (!strcmp(name, \"refspec\")) {\n+\t\toptions.refspec = xstrdup(value);\n+\t\treturn 0;\n+\t}\n \telse if (!strcmp(name, \"progress\")) {\n \t\tif (!strcmp(value, \"true\"))\n \t\t\toptions.progress = 1;\n@@ -269,6 +274,8 @@ static struct discovery *discover_refs(const char *service, int for_push)\n \t\telse\n \t\t\tstrbuf_addch(&refs_url, '&');\n \t\tstrbuf_addf(&refs_url, \"service=%s\", service);\n+\t\tif (options.refspec)\n+\t\t\tstrbuf_addf(&refs_url, \"&refspec=%s\", options.refspec);\n \t}\n \n \tmemset(&get_options, 0, sizeof(get_options));\ndiff --git a/t/t5552-http-fetch-branch.sh b/t/t5552-http-fetch-branch.sh\nnew file mode 100755\nindex 0000000..0e905d9\n--- /dev/null\n+++ b/t/t5552-http-fetch-branch.sh\n@@ -0,0 +1,42 @@\n+#!/bin/sh\n+\n+test_description='fetch just one branch'\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'setup repo' '\n+\tgit init \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\t(\n+\t\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\t\ttest_commit 1\n+\t)\n+'\n+\n+test_expect_success 'clone http repository' '\n+\tgit clone $HTTPD_URL/smart/repo.git clone\n+'\n+\n+test_expect_success 'make some more commits' '\n+\t(\n+\t\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\t\ttest_commit 2 &&\n+\t\tgit checkout -b another_branch &&\n+\t\ttest_commit 3\n+\t\tgit checkout -b a_third_branch &&\n+\t\ttest_commit 4\n+\t)\n+'\n+\n+test_expect_success 'fetch with refspec only fetches requested branch' '\n+\ttest_when_finished \"rm trace\" &&\n+\t(\n+\t\tcd clone &&\n+\t\tGIT_TRACE_PACKET=\"$TRASH_DIRECTORY/trace\" git fetch origin another_branch &&\n+\t\t! grep \"refs/heads/master\" ../trace\n+\t)\n+'\n+\n+stop_httpd\n+test_done\ndiff --git a/transport-helper.c b/transport-helper.c\nindex b5c91d2..7d75d64 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -40,6 +40,9 @@ struct helper_data {\n \tstruct git_transport_options transport_options;\n };\n \n+static int set_helper_option(struct transport *transport,\n+\t\t\t const char *name, const char *value);\n+\n static void sendline(struct helper_data *helper, struct strbuf *buffer)\n {\n \tif (debug)\n@@ -109,6 +112,7 @@ static struct child_process *get_helper(struct transport *transport, const struc\n \tint refspec_alloc = 0;\n \tint duped;\n \tint code;\n+\tint i;\n \n \tif (data->helper)\n \t\treturn data->helper;\n@@ -202,7 +206,6 @@ static struct child_process *get_helper(struct transport *transport, const struc\n \t\t}\n \t}\n \tif (refspecs) {\n-\t\tint i;\n \t\tdata->refspec_nr = refspec_nr;\n \t\tdata->refspecs = parse_fetch_refspec(refspec_nr, refspecs);\n \t\tfor (i = 0; i < refspec_nr; i++)\n@@ -214,6 +217,9 @@ static struct child_process *get_helper(struct transport *transport, const struc\n \tstrbuf_release(&buf);\n \tif (debug)\n \t\tfprintf(stderr, \"Debug: Capabilities complete.\\n\");\n+\n+\tfor (i = 0; i < req_refspec_nr; i++)\n+\t\tset_helper_option(transport, \"refspec\", req_refspecs[i].src);\n \tstandard_options(transport);\n \treturn data->helper;\n }\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283567","messageId":"1460747949-3514-7-git-send-email-dturner@twopensource.com","threadId":"42038","inReplyTo":"1460747949-3514-1-git-send-email-dturner@twopensource.com","subject":"[PATCH/RFC 6/6] clone: send refspec for single-branch clones","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-15T19:19:09Z","receivedAt":"2016-04-15T19:19:09Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"For single-branch clones (when we know in advance what the remote\nbranch name will be), send a refspec so that the server doesn't\ntell us about any other refs.\n\nSigned-off-by: David Turner \n---\n builtin/clone.c | 16 +++++++++++++++-\n t/t5552-http-fetch-branch.sh | 5 +++++\n transport-helper.c | 12 +++++++++++-\n 3 files changed, 31 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 91f668c..9a0291d 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1010,7 +1010,21 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (transport->smart_options && !option_depth)\n \t\ttransport->smart_options->check_self_contained_and_connected = 1;\n \n-\trefs = transport_get_remote_refs(transport, NULL, 0);\n+\tif (option_single_branch && option_branch) {\n+\t\tstruct refspec branch_refspec = {0};\n+\n+\t\tif (starts_with(option_branch, \"refs/\")) {\n+\t\t\tbranch_refspec.src = xstrdup(option_branch);\n+\t\t} else {\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&buf, \"refs/heads/%s\", option_branch);\n+\t\t\tbranch_refspec.src = strbuf_detach(&buf, NULL);\n+\t\t}\n+\t\trefs = transport_get_remote_refs(transport, &branch_refspec, 1);\n+\t\tfree(branch_refspec.src);\n+\t} else {\n+\t\trefs = transport_get_remote_refs(transport, NULL, 0);\n+\t}\n \n \tif (refs) {\n \t\tmapped_refs = wanted_peer_refs(refs, refspec);\ndiff --git a/t/t5552-http-fetch-branch.sh b/t/t5552-http-fetch-branch.sh\nindex 0e905d9..8a8e218 100755\n--- a/t/t5552-http-fetch-branch.sh\n+++ b/t/t5552-http-fetch-branch.sh\n@@ -38,5 +38,10 @@ test_expect_success 'fetch with refspec only fetches requested branch' '\n \t)\n '\n \n+test_expect_success 'single-branch clone only fetches requested branch' '\n+\tGIT_TRACE_PACKET=\"$TRASH_DIRECTORY/trace\" git clone --single-branch -b master $HTTPD_URL/smart/repo.git sbc &&\n+\t! grep \"refs/heads/another_branch\" trace\n+'\n+\n stop_httpd\n test_done\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 7d75d64..3775d63 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -28,6 +28,7 @@ struct helper_data {\n \t\tsigned_tags : 1,\n \t\tcheck_connectivity : 1,\n \t\tno_disconnect_req : 1,\n+\t\trefspec : 1,\n \t\tno_private_update : 1;\n \tchar *export_marks;\n \tchar *import_marks;\n@@ -114,8 +115,15 @@ static struct child_process *get_helper(struct transport *transport, const struc\n \tint code;\n \tint i;\n \n-\tif (data->helper)\n+\tif (data->helper) {\n+\t\tif (!data->refspec && req_refspec_nr) {\n+\t\t\tfor (i = 0; i < req_refspec_nr; i++)\n+\t\t\t\tset_helper_option(transport, \"refspec\",\n+\t\t\t\t\t\t req_refspecs[i].src);\n+\t\t\tdata->refspec = 1;\n+\t\t}\n \t\treturn data->helper;\n+\t}\n \n \thelper = xmalloc(sizeof(*helper));\n \tchild_process_init(helper);\n@@ -220,6 +228,8 @@ static struct child_process *get_helper(struct transport *transport, const struc\n \n \tfor (i = 0; i < req_refspec_nr; i++)\n \t\tset_helper_option(transport, \"refspec\", req_refspecs[i].src);\n+\tif (req_refspec_nr)\n+\t\tdata->refspec = 1;\n \tstandard_options(transport);\n \treturn data->helper;\n }\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283569","messageId":"CAGZ79kYwO0TVetGjqGgQQgJjtnATGdae3JsgZ2Pn89ExDSd_Qw@mail.gmail.com","threadId":"42038","inReplyTo":"1460747949-3514-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH/RFC 0/6] fetch with refspec","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-15T19:30:13Z","receivedAt":"2016-04-15T19:30:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 15, 2016 at 12:19 PM, David Turner wrote:\n> We've got a lot of refs, but pretty frequently we only want to fetch\n> one. It's silly for the server to send a bunch of refs that the client\n> is just going to ignore. Here are some patches that fix that.\n>\n> Let me know if this seems reasonable.\n\nThanks for working on this!\n\nI had a similar goal back then for non-http traffic and that series\nexploded in size[1]\nThe issue at my attempt was non http traffic would require a protocol\nupdate such that\nthe client speaks first to transport the refspec to the server. To\nmake \"client speaks first\"\nhappen, we'd need to have a protocol v2. So that attempt of mine\nstalled as it seemed like\na huge thing.\n\n[1] WIP at https://github.com/stefanbeller/git/tree/protocol2-10\n\nThis series looks small and reasonable from a cursory read.\n\nThanks,\nStefan\n\n>\n> (and I'll start in on another round of index-helper as soon as this is sent!)\n>\n> David Turner (6):\n> http-backend: use argv_array functions\n> remote-curl.c: fix variable shadowing\n> http-backend: handle refspec argument\n> transport: add refspec list parameters to functions\n> fetch: pass refspec to http server\n> clone: send refspec for single-branch clones\n>\n> Documentation/technical/protocol-capabilities.txt | 23 +++++++\n> builtin/clone.c | 16 ++++-\n> builtin/fetch.c | 24 ++++++-\n> builtin/ls-remote.c | 2 +-\n> builtin/remote.c | 2 +-\n> http-backend.c | 23 +++++--\n> remote-curl.c | 25 ++++---\n> t/t5552-http-fetch-branch.sh | 47 +++++++++++++\n> transport-helper.c | 44 ++++++++----\n> transport.c | 14 ++--\n> transport.h | 4 +-\n> upload-pack.c | 81 ++++++++++++++++++++++-\n> 12 files changed, 261 insertions(+), 44 deletions(-)\n> create mode 100755 t/t5552-http-fetch-branch.sh\n>\n> --\n> 2.4.2.767.g62658d5-twtrsrc\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"283660","messageId":"CAPig+cRSE-BCPdrSbrCYmTcT6EsabKnekr2GEAbmBsc5=jxnHQ@mail.gmail.com","threadId":"42038","inReplyTo":"1460747949-3514-4-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH/RFC 3/6] http-backend: handle refspec argument","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T01:51:29Z","receivedAt":"2016-04-17T01:51:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 15, 2016 at 3:19 PM, David Turner wrote:\n> Allow clients to pass a \"refspec\" parameter through to upload-pack;\n> upload-pack will only advertise refs which match that refspec.\n>\n> Signed-off-by: David Turner \n> ---\n> diff --git a/http-backend.c b/http-backend.c\n> @@ -465,6 +466,14 @@ static void get_info_refs(char *arg)\n> argv_array_push(&argv, \"--stateless-rpc\");\n> argv_array_push(&argv, \"--advertise-refs\");\n>\n> + refspec = get_parameter(\"refspec\");\n> + if (refspec) {\n> + struct strbuf interesting_refs = STRBUF_INIT;\n> + strbuf_addstr(&interesting_refs, \"--interesting-refs=\");\n> + strbuf_addstr(&interesting_refs, refspec);\n> + argv_array_push(&argv, interesting_refs.buf);\n> + strbuf_release(&interesting_refs);\n> + }\n\n if (refspec)\n argv_array_pushf(&interesting_refs,\n \"--interesting-refs=%s\", refspec);\n\n> argv_array_push(&argv, \".\");\n> run_service(argv.argv, 0);\n> argv_array_clear(&argv);\n> @@ -841,6 +905,19 @@ int main(int argc, char **argv)\n> + if (starts_with(arg, \"--interesting-refs=\")) {\n> + struct string_list_item *item;\n> +\n> + string_list_split(&interesting_refspecs, arg + 19,\n> + ' ', -1);\n> + for_each_string_list_item(item, &interesting_refspecs) {\n> + if (check_refname_format(item->string,\n> + REFNAME_REFSPEC_PATTERN))\n> + die(\"invalid refspec %s\", item->string);\n> + item->util = make_refspec_data(item->string);\n> + }\n> + continue;\n> + }\n\nIs this leaking the string list?\n\n> if (!strcmp(arg, \"--stateless-rpc\")) {\n> stateless_rpc = 1;\n> continue;\n> --\n> 2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283661","messageId":"CAPig+cSxkuWB5qqVY3-m9ESmmHMKwXM+qtbRsZ1j1osjp2i3HQ@mail.gmail.com","threadId":"42038","inReplyTo":"1460747949-3514-6-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH/RFC 5/6] fetch: pass refspec to http server","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T02:33:38Z","receivedAt":"2016-04-17T02:33:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 15, 2016 at 3:19 PM, David Turner wrote:\n> When fetching over http, send the requested refspec to the server.\n> The server will then only send refs matching that refspec. It is\n> permitted for old servers to ignore that parameter, and the client\n> will automatically handle this.\n>\n> When the server has many refs, and the client only wants a few, this\n> can save bandwidth and reduce fetch latency.\n>\n> Signed-off-by: David Turner \n> ---\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> @@ -302,9 +302,27 @@ static struct ref *get_ref_map(struct transport *transport,\n> - remote_refs = transport_get_remote_refs(transport, NULL, 0);\n> + qualified_refspecs = xcalloc(refspec_count, sizeof(*qualified_refspecs));\n> + for (i = 0; i < refspec_count; i++) {\n> + if (starts_with(refspecs[i].src, \"refs/\")) {\n> + qualified_refspecs[i].src = xstrdup(refspecs[i].src);\n> + } else {\n> + struct strbuf buf = STRBUF_INIT;\n> + strbuf_addf(&buf, \"refs/heads/%s\", refspecs[i].src);\n> + qualified_refspecs[i].src = strbuf_detach(&buf, NULL);\n\nAlternately, replace these three lines with:\n\n qualified_refspecs[i].src = xstrfmt(\"refs/heads/%s\", refspecs[i].src);\n\nand drop the braces.\n\n> + }\n> + }\n> +\n> + remote_refs = transport_get_remote_refs(transport, qualified_refspecs,\n> + refspec_count);\n> +\n> + for (i = 0; i < refspec_count; i++) {\n> + free(qualified_refspecs[i].src);\n> + }\n> + free(qualified_refspecs);\n> diff --git a/t/t5552-http-fetch-branch.sh b/t/t5552-http-fetch-branch.sh\n> @@ -0,0 +1,42 @@\n> +test_expect_success 'make some more commits' '\n> + (\n> + cd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> + test_commit 2 &&\n> + git checkout -b another_branch &&\n> + test_commit 3\n\nBroken &&-chain.\n\n> + git checkout -b a_third_branch &&\n> + test_commit 4\n> + )\n> +'\n> +\n> +test_expect_success 'fetch with refspec only fetches requested branch' '\n> + test_when_finished \"rm trace\" &&\n> + (\n> + cd clone &&\n> + GIT_TRACE_PACKET=\"$TRASH_DIRECTORY/trace\" git fetch origin another_branch &&\n> + ! grep \"refs/heads/master\" ../trace\n> + )\n> +'\n\nThis could be done without the subshell, perhaps?\n\n GIT_TRACE_PACKET=blah git -C clone fetch ...\n"},{"id":"283662","messageId":"CAPig+cRGDsBn4EKWn7Vqqmt_ZhZbYgUh9Tg-rgQPchn015SNMw@mail.gmail.com","threadId":"42038","inReplyTo":"1460747949-3514-7-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH/RFC 6/6] clone: send refspec for single-branch clones","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T02:36:06Z","receivedAt":"2016-04-17T02:36:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 15, 2016 at 3:19 PM, David Turner wrote:\n> For single-branch clones (when we know in advance what the remote\n> branch name will be), send a refspec so that the server doesn't\n> tell us about any other refs.\n>\n> Signed-off-by: David Turner \n> ---\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> @@ -1010,7 +1010,21 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> + if (option_single_branch && option_branch) {\n> + struct refspec branch_refspec = {0};\n> +\n> + if (starts_with(option_branch, \"refs/\")) {\n> + branch_refspec.src = xstrdup(option_branch);\n> + } else {\n> + struct strbuf buf = STRBUF_INIT;\n> + strbuf_addf(&buf, \"refs/heads/%s\", option_branch);\n> + branch_refspec.src = strbuf_detach(&buf, NULL);\n\nbranch_refspec.src = xstrfmt(\"refs/heads/%s\", option_branch);\n\n> + }\n> + refs = transport_get_remote_refs(transport, &branch_refspec, 1);\n> + free(branch_refspec.src);\n> + } else {\n> + refs = transport_get_remote_refs(transport, NULL, 0);\n> + }\n"},{"id":"283740","messageId":"xmqqinze6a1h.fsf@gitster.mtv.corp.google.com","threadId":"42038","inReplyTo":"1460747949-3514-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH/RFC 1/6] http-backend: use argv_array functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-18T18:34:18Z","receivedAt":"2016-04-18T18:34:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> Signed-off-by: David Turner \n> ---\n\nOK (it might be easier to read if you used the pushl form for the\n\"fixed initial segment\" like these calls, though).\n\n> http-backend.c | 12 +++++++-----\n> 1 file changed, 7 insertions(+), 5 deletions(-)\n>\n> diff --git a/http-backend.c b/http-backend.c\n> index 8870a26..a4f0066 100644\n> --- a/http-backend.c\n> +++ b/http-backend.c\n> @@ -450,9 +450,7 @@ static void get_info_refs(char *arg)\n> \thdr_nocache();\n> \n> \tif (service_name) {\n> -\t\tconst char *argv[] = {NULL /* service name */,\n> -\t\t\t\"--stateless-rpc\", \"--advertise-refs\",\n> -\t\t\t\".\", NULL};\n> +\t\tstruct argv_array argv = ARGV_ARRAY_INIT;\n> \t\tstruct rpc_service *svc = select_service(service_name);\n> \n> \t\tstrbuf_addf(&buf, \"application/x-git-%s-advertisement\",\n> @@ -463,9 +461,13 @@ static void get_info_refs(char *arg)\n> \t\tpacket_write(1, \"# service=git-%s\\n\", svc->name);\n> \t\tpacket_flush(1);\n> \n> -\t\targv[0] = svc->name;\n> -\t\trun_service(argv, 0);\n> +\t\targv_array_push(&argv, svc->name);\n> +\t\targv_array_push(&argv, \"--stateless-rpc\");\n> +\t\targv_array_push(&argv, \"--advertise-refs\");\n> \n> +\t\targv_array_push(&argv, \".\");\n> +\t\trun_service(argv.argv, 0);\n> +\t\targv_array_clear(&argv);\n> \t} else {\n> \t\tselect_getanyfile();\n> \t\tfor_each_namespaced_ref(show_text_ref, &buf);\n"},{"id":"283741","messageId":"xmqqega26a01.fsf@gitster.mtv.corp.google.com","threadId":"42038","inReplyTo":"1460747949-3514-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH/RFC 2/6] remote-curl.c: fix variable shadowing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-18T18:35:10Z","receivedAt":"2016-04-18T18:35:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> The local variable 'options' was shadowing a global of the same name.\n>\n> Signed-off-by: David Turner \n> ---\n\nOK. In general, giving a longer and more descriptive name to the\nglobal would be a direction to lead to more readable code, though.\n\n> remote-curl.c | 18 +++++++++---------\n> 1 file changed, 9 insertions(+), 9 deletions(-)\n>\n> diff --git a/remote-curl.c b/remote-curl.c\n> index 15e48e2..b9b6a90 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -254,7 +254,7 @@ static struct discovery *discover_refs(const char *service, int for_push)\n> \tstruct strbuf effective_url = STRBUF_INIT;\n> \tstruct discovery *last = last_discovery;\n> \tint http_ret, maybe_smart = 0;\n> -\tstruct http_get_options options;\n> +\tstruct http_get_options get_options;\n> \n> \tif (last && !strcmp(service, last->service))\n> \t\treturn last;\n> @@ -271,15 +271,15 @@ static struct discovery *discover_refs(const char *service, int for_push)\n> \t\tstrbuf_addf(&refs_url, \"service=%s\", service);\n> \t}\n> \n> -\tmemset(&options, 0, sizeof(options));\n> -\toptions.content_type = &type;\n> -\toptions.charset = &charset;\n> -\toptions.effective_url = &effective_url;\n> -\toptions.base_url = &url;\n> -\toptions.no_cache = 1;\n> -\toptions.keep_error = 1;\n> +\tmemset(&get_options, 0, sizeof(get_options));\n> +\tget_options.content_type = &type;\n> +\tget_options.charset = &charset;\n> +\tget_options.effective_url = &effective_url;\n> +\tget_options.base_url = &url;\n> +\tget_options.no_cache = 1;\n> +\tget_options.keep_error = 1;\n> \n> -\thttp_ret = http_get_strbuf(refs_url.buf, &buffer, &options);\n> +\thttp_ret = http_get_strbuf(refs_url.buf, &buffer, &get_options);\n> \tswitch (http_ret) {\n> \tcase HTTP_OK:\n> \t\tbreak;\n"},{"id":"283742","messageId":"xmqqa8kq69i5.fsf@gitster.mtv.corp.google.com","threadId":"42038","inReplyTo":"1460747949-3514-5-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-18T18:45:54Z","receivedAt":"2016-04-18T18:45:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> Add parameters for a list of refspecs to transport_get_remote_refs and\n> get_refs_list. These parameters are presently unused -- soon, we will\n> use them to implement fetches which only learn about a subset of refs.\n>\n> Signed-off-by: David Turner \n> ---\n\nWhat the code tries to do I am more than halfway happy. It is\nunfortunate that we cannot do this natively without upgrading the\nprotocol in a fundamental way, but this is a nice way to work it\naround only for Git-over-HTTP transport without having to break the\nprotocol.\n \nAs a POC it is OK, but I am moderately unhappy with the use of\n\"refspec\" here.\n\nAt the transport layer, we shouldn't care what the receiving end\nintends to do with the objects that sits at the tip of the refs at\nthe other end, so sending \"refspecs\" down feels somewhat wrong for\nthis feature. At one layer up in the next patch, you do use\n\"interesting refs\" which makes it clear that only the left-hand-side\nof a refspec, i.e. what they call it, matters, and I think that is a\nmuch better phrasing of the concept (and the passed data should only\nbe the left-hand-side of refspecs).\n\nThanks.\n"},{"id":"283772","messageId":"20160419071403.GA22577@sigill.intra.peff.net","threadId":"42038","inReplyTo":"xmqqa8kq69i5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-19T07:14:03Z","receivedAt":"2016-04-19T07:14:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 18, 2016 at 11:45:54AM -0700, Junio C Hamano wrote:\n\n> David Turner writes:\n> \n> > Add parameters for a list of refspecs to transport_get_remote_refs and\n> > get_refs_list. These parameters are presently unused -- soon, we will\n> > use them to implement fetches which only learn about a subset of refs.\n> >\n> > Signed-off-by: David Turner \n> > ---\n> \n> What the code tries to do I am more than halfway happy. It is\n> unfortunate that we cannot do this natively without upgrading the\n> protocol in a fundamental way, but this is a nice way to work it\n> around only for Git-over-HTTP transport without having to break the\n> protocol.\n\nI dunno, I am a bit negative on bringing new features to Git-over-HTTP\n(which is already less efficient than the other protocols!) without any\nplan for supporting them in the other protocols.\n\nI thought Stefan's v2 protocol work looked quite good, but it seems to\nhave stalled. The hardest part of that topic is figuring out the upgrade\npath. But for git-over-http, we can solve that in the same way that\nDavid is passing in the extra refspecs.\n\nSo I'd rather see something like:\n\n 1. Support for v2 \"capabilities only\" initial negotiation, followed\n by ref advertisement.\n\n 2. Support for refspec-limiting capability.\n\n 3. HTTP-only option from client to trigger v2 on the server.\n\nThat's still HTTP-specific, but it has a clear path for converging with\nthe ssh and git protocols eventually, rather than having to support\nmagic out-of-band capabilities forever.\n\nIt does require an extra round of HTTP request/response, though.\n\n-Peff\n"},{"id":"283818","messageId":"CAGZ79kZOY9guHWYmxh+J4-BXzFCMWLFhF0sDS3Kau3oXqWsSxg@mail.gmail.com","threadId":"42038","inReplyTo":"20160419071403.GA22577@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-19T18:04:13Z","receivedAt":"2016-04-19T18:04:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Apr 19, 2016 at 12:14 AM, Jeff King wrote:\n> On Mon, Apr 18, 2016 at 11:45:54AM -0700, Junio C Hamano wrote:\n>\n>> David Turner writes:\n>>\n>> > Add parameters for a list of refspecs to transport_get_remote_refs and\n>> > get_refs_list. These parameters are presently unused -- soon, we will\n>> > use them to implement fetches which only learn about a subset of refs.\n>> >\n>> > Signed-off-by: David Turner \n>> > ---\n>>\n>> What the code tries to do I am more than halfway happy. It is\n>> unfortunate that we cannot do this natively without upgrading the\n>> protocol in a fundamental way, but this is a nice way to work it\n>> around only for Git-over-HTTP transport without having to break the\n>> protocol.\n>\n> I dunno, I am a bit negative on bringing new features to Git-over-HTTP\n> (which is already less efficient than the other protocols!) without any\n> plan for supporting them in the other protocols.\n>\n> I thought Stefan's v2 protocol work looked quite good, but it seems to\n> have stalled. The hardest part of that topic is figuring out the upgrade\n> path. But for git-over-http, we can solve that in the same way that\n> David is passing in the extra refspecs.\n\nYeah it stalled, though I hope to revive it eventually.\n\nI was positive about these changes for that same reason: If http and native\nprotocol move apart even more, it will be easier to make the native only\nv2 protocol without needing to fiddle with http, i.e. this series would reduce\nscope of the v2 series drastically?\n\n>\n> So I'd rather see something like:\n>\n> 1. Support for v2 \"capabilities only\" initial negotiation, followed\n> by ref advertisement.\n\nAnd the client needs to talk in between capabilities and ref advertisement.\nEven if it is just a flush for now. That can be extended later to the actual\ndesired capabilities, but the server needs to at least wait for a client packet\nin here.\n\nNote that the server side for v2 capabilites only first is done, the client side\nis missing as I found that to be the hard part.\n\n\n>\n> 2. Support for refspec-limiting capability.\n>\n> 3. HTTP-only option from client to trigger v2 on the server.\n>\n> That's still HTTP-specific, but it has a clear path for converging with\n> the ssh and git protocols eventually, rather than having to support\n> magic out-of-band capabilities forever.\n>\n> It does require an extra round of HTTP request/response, though.\n>\n> -Peff\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"283827","messageId":"1461092277.5540.108.camel@twopensource.com","threadId":"42038","inReplyTo":"CAPig+cRSE-BCPdrSbrCYmTcT6EsabKnekr2GEAbmBsc5=jxnHQ@mail.gmail.com","subject":"Re: [PATCH/RFC 3/6] http-backend: handle refspec argument","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T18:57:57Z","receivedAt":"2016-04-19T18:57:57Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Sat, 2016-04-16 at 21:51 -0400, Eric Sunshine wrote:\n> On Fri, Apr 15, 2016 at 3:19 PM, David Turner <\n> dturner@twopensource.com> wrote:\n> > + if (refspec) {\n> > + struct strbuf interesting_refs =\n> > STRBUF_INIT;\n> > + strbuf_addstr(&interesting_refs, \"-\n> > -interesting-refs=\");\n> > + strbuf_addstr(&interesting_refs, refspec);\n> > + argv_array_push(&argv,\n> > interesting_refs.buf);\n> > + strbuf_release(&interesting_refs);\n> > + }\n> \n> if (refspec)\n> argv_array_pushf(&interesting_refs,\n> \"--interesting-refs=%s\", refspec);\n\n\nWill fix, thanks.\n\n> > argv_array_push(&argv, \".\");\n> > run_service(argv.argv, 0);\n> > argv_array_clear(&argv);\n> > @@ -841,6 +905,19 @@ int main(int argc, char **argv)\n> > + if (starts_with(arg, \"--interesting-refs=\")) {\n> > ...\n> > + continue;\n> > + }\n> \n> Is this leaking the string list?\n\nYes, intentionally. interesting_refspec is a global that we look at\nlater. \n"},{"id":"283833","messageId":"1461093084.5540.109.camel@twopensource.com","threadId":"42038","inReplyTo":"xmqqinze6a1h.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC 1/6] http-backend: use argv_array functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T19:11:24Z","receivedAt":"2016-04-19T19:11:24Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2016-04-18 at 11:34 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > Signed-off-by: David Turner \n> > ---\n> \n> OK (it might be easier to read if you used the pushl form for the\n> \"fixed initial segment\" like these calls, though).\n\nGood idea.\n"},{"id":"283834","messageId":"1461093271.5540.110.camel@twopensource.com","threadId":"42038","inReplyTo":"xmqqega26a01.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC 2/6] remote-curl.c: fix variable shadowing","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T19:14:31Z","receivedAt":"2016-04-19T19:14:31Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2016-04-18 at 11:35 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > The local variable 'options' was shadowing a global of the same\n> > name.\n> > \n> > Signed-off-by: David Turner \n> > ---\n> \n> OK. In general, giving a longer and more descriptive name to the\n> global would be a direction to lead to more readable code, though.\n\nOK, will do that instead.\n"},{"id":"283835","messageId":"1461094263.5540.113.camel@twopensource.com","threadId":"42038","inReplyTo":"xmqqa8kq69i5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T19:31:03Z","receivedAt":"2016-04-19T19:31:03Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2016-04-18 at 11:45 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > Add parameters for a list of refspecs to transport_get_remote_refs\n> > and\n> > get_refs_list. These parameters are presently unused -- soon, we\n> > will\n> > use them to implement fetches which only learn about a subset of\n> > refs.\n> > \n> > Signed-off-by: David Turner \n> > ---\n> \n> What the code tries to do I am more than halfway happy. It is\n> unfortunate that we cannot do this natively without upgrading the\n> protocol in a fundamental way, but this is a nice way to work it\n> around only for Git-over-HTTP transport without having to break the\n> protocol.\n> \n> As a POC it is OK, but I am moderately unhappy with the use of\n> \"refspec\" here.\n> \n> At the transport layer, we shouldn't care what the receiving end\n> intends to do with the objects that sits at the tip of the refs at\n> the other end, so sending \"refspecs\" down feels somewhat wrong for\n> this feature. At one layer up in the next patch, you do use\n> \"interesting refs\" which makes it clear that only the left-hand-side\n> of a refspec, i.e. what they call it, matters, and I think that is a\n> much better phrasing of the concept (and the passed data should only\n> be the left-hand-side of refspecs).\n\nI will rename the parameter to \"interesting_refs\".\n"},{"id":"283848","messageId":"xmqqlh49z5cg.fsf@gitster.mtv.corp.google.com","threadId":"42038","inReplyTo":"20160419071403.GA22577@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T20:55:11Z","receivedAt":"2016-04-19T20:55:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> What the code tries to do I am more than halfway happy. It is\n>> unfortunate that we cannot do this natively without upgrading the\n>> protocol in a fundamental way, but this is a nice way to work it\n>> around only for Git-over-HTTP transport without having to break the\n>> protocol.\n>\n> I dunno, I am a bit negative on bringing new features to Git-over-HTTP\n> (which is already less efficient than the other protocols!) without any\n> plan for supporting them in the other protocols.\n>\n> I thought Stefan's v2 protocol work looked quite good, but it seems to\n> have stalled. The hardest part of that topic is figuring out the upgrade\n> path. But for git-over-http, we can solve that in the same way that\n> David is passing in the extra refspecs.\n\nYeah, I had the same feeling.\n\n> So I'd rather see something like:\n>\n> 1. Support for v2 \"capabilities only\" initial negotiation, followed\n> by ref advertisement.\n>\n> 2. Support for refspec-limiting capability.\n>\n> 3. HTTP-only option from client to trigger v2 on the server.\n\nI like that; reducing the initial scope of v2 so that we can do this\nnew feature as its first use case would be a good way to move things\nforward.\n\n> That's still HTTP-specific, but it has a clear path for converging with\n> the ssh and git protocols eventually, rather than having to support\n> magic out-of-band capabilities forever.\n>\n> It does require an extra round of HTTP request/response, though.\n\nYes. And as we discussed before, we can do \"upload-pack\" that\nadvertises \"by the way, upload-pack-v2 is available next to me\" and\na separate \"upload-pack-v2\" that talks v2 (i.e. its initial message\nis limited to capabilities and nothing else) would probably be a\nsufficient migration path for native protocol.\n\n \n"},{"id":"283851","messageId":"1461101064.5540.118.camel@twopensource.com","threadId":"42038","inReplyTo":"CAPig+cRGDsBn4EKWn7Vqqmt_ZhZbYgUh9Tg-rgQPchn015SNMw@mail.gmail.com","subject":"Re: [PATCH/RFC 6/6] clone: send refspec for single-branch clones","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T21:24:24Z","receivedAt":"2016-04-19T21:24:24Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Sat, 2016-04-16 at 22:36 -0400, Eric Sunshine wrote:\n> On Fri, Apr 15, 2016 at 3:19 PM, David Turner <\n> dturner@twopensource.com> wrote:\n> > For single-branch clones (when we know in advance what the remote\n> > branch name will be), send a refspec so that the server doesn't\n> > tell us about any other refs.\n> > \n> > Signed-off-by: David Turner \n> > ---\n> > diff --git a/builtin/clone.c b/builtin/clone.c\n> > @@ -1010,7 +1010,21 @@ int cmd_clone(int argc, const char **argv,\n> > const char *prefix)\n> > + if (option_single_branch && option_branch) {\n> > + struct refspec branch_refspec = {0};\n> > +\n> > + if (starts_with(option_branch, \"refs/\")) {\n> > + branch_refspec.src =\n> > xstrdup(option_branch);\n> > + } else {\n> > + struct strbuf buf = STRBUF_INIT;\n> > + strbuf_addf(&buf, \"refs/heads/%s\",\n> > option_branch);\n> > + branch_refspec.src = strbuf_detach(&buf,\n> > NULL);\n> \n> branch_refspec.src = xstrfmt(\"refs/heads/%s\", option_branch);\n\n\nWill fix, thanks.\n"},{"id":"283853","messageId":"1461101146.5540.120.camel@twopensource.com","threadId":"42038","inReplyTo":"CAPig+cSxkuWB5qqVY3-m9ESmmHMKwXM+qtbRsZ1j1osjp2i3HQ@mail.gmail.com","subject":"Re: [PATCH/RFC 5/6] fetch: pass refspec to http server","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T21:25:46Z","receivedAt":"2016-04-19T21:25:46Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Sat, 2016-04-16 at 22:33 -0400, Eric Sunshine wrote:\n> On Fri, Apr 15, 2016 at 3:19 PM, David Turner <\n> dturner@twopensource.com> wrote:\n> > When fetching over http, send the requested refspec to the server.\n> > The server will then only send refs matching that refspec. It is\n> > permitted for old servers to ignore that parameter, and the client\n> > will automatically handle this.\n> > \n> > When the server has many refs, and the client only wants a few,\n> > this\n> > can save bandwidth and reduce fetch latency.\n> > \n> > Signed-off-by: David Turner \n> > ---\n> > diff --git a/builtin/fetch.c b/builtin/fetch.c\n> > @@ -302,9 +302,27 @@ static struct ref *get_ref_map(struct\n> > transport *transport,\n> > - remote_refs = transport_get_remote_refs(transport, NULL,\n> > 0);\n> > + qualified_refspecs = xcalloc(refspec_count,\n> > sizeof(*qualified_refspecs));\n> > + for (i = 0; i < refspec_count; i++) {\n> > + if (starts_with(refspecs[i].src, \"refs/\")) {\n> > + qualified_refspecs[i].src =\n> > xstrdup(refspecs[i].src);\n> > + } else {\n> > + struct strbuf buf = STRBUF_INIT;\n> > + strbuf_addf(&buf, \"refs/heads/%s\",\n> > refspecs[i].src);\n> > + qualified_refspecs[i].src =\n> > strbuf_detach(&buf, NULL);\n> \n> Alternately, replace these three lines with:\n> \n> qualified_refspecs[i].src = xstrfmt(\"refs/heads/%s\",\n> refspecs[i].src);\n> \n> and drop the braces.\n\nYep, good point.\n\n> > + }\n> > + }\n> > +\n> > + remote_refs = transport_get_remote_refs(transport,\n> > qualified_refspecs,\n> > + refspec_count);\n> > +\n> > + for (i = 0; i < refspec_count; i++) {\n> > + free(qualified_refspecs[i].src);\n> > + }\n> > + free(qualified_refspecs);\n> > diff --git a/t/t5552-http-fetch-branch.sh b/t/t5552-http-fetch\n> > -branch.sh\n> > @@ -0,0 +1,42 @@\n> > +test_expect_success 'make some more commits' '\n> > + (\n> > + cd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> > + test_commit 2 &&\n> > + git checkout -b another_branch &&\n> > + test_commit 3\n> \n> Broken &&-chain.\n\nThanks.\n\n> > + git checkout -b a_third_branch &&\n> > + test_commit 4\n> > + )\n> > +'\n> > +\n> > +test_expect_success 'fetch with refspec only fetches requested\n> > branch' '\n> > + test_when_finished \"rm trace\" &&\n> > + (\n> > + cd clone &&\n> > + GIT_TRACE_PACKET=\"$TRASH_DIRECTORY/trace\" git fetch\n> > origin another_branch &&\n> > + ! grep \"refs/heads/master\" ../trace\n> > + )\n> > +'\n> \n> This could be done without the subshell, perhaps?\n> \n> GIT_TRACE_PACKET=blah git -C clone fetch ...\n\n\nWill fix these, thanks.\n"},{"id":"283856","messageId":"1461102001.5540.125.camel@twopensource.com","threadId":"42038","inReplyTo":"20160419071403.GA22577@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T21:40:01Z","receivedAt":"2016-04-19T21:40:01Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2016-04-19 at 03:14 -0400, Jeff King wrote:\n> On Mon, Apr 18, 2016 at 11:45:54AM -0700, Junio C Hamano wrote:\n> \n> > David Turner writes:\n> > \n> > > Add parameters for a list of refspecs to\n> > > transport_get_remote_refs and\n> > > get_refs_list. These parameters are presently unused -- soon, we\n> > > will\n> > > use them to implement fetches which only learn about a subset of\n> > > refs.\n> > > \n> > > Signed-off-by: David Turner \n> > > ---\n> > \n> > What the code tries to do I am more than halfway happy. It is\n> > unfortunate that we cannot do this natively without upgrading the\n> > protocol in a fundamental way, but this is a nice way to work it\n> > around only for Git-over-HTTP transport without having to break the\n> > protocol.\n> \n> I dunno, I am a bit negative on bringing new features to Git-over\n> -HTTP\n> (which is already less efficient than the other protocols!) without\n> any\n> plan for supporting them in the other protocols.\n\nInteresting -- can you expand on git-over-http being less efficient?\nThis is the first I'd heard of it. Is it documented somewhere?\n\n> I thought Stefan's v2 protocol work looked quite good, but it seems\n> to\n> have stalled. The hardest part of that topic is figuring out the\n> upgrade\n> path. But for git-over-http, we can solve that in the same way that\n> David is passing in the extra refspecs.\n> \n> So I'd rather see something like:\n> \n> 1. Support for v2 \"capabilities only\" initial negotiation, followed\n> by ref advertisement.\n> \n> 2. Support for refspec-limiting capability.\n> \n> 3. HTTP-only option from client to trigger v2 on the server.\n> \n> That's still HTTP-specific, but it has a clear path for converging\n> with\n> the ssh and git protocols eventually, rather than having to support\n> magic out-of-band capabilities forever.\n> \n> It does require an extra round of HTTP request/response, though.\n\nThis seems way more complicated to me, and not necessarily super\n-efficient. That is, it seems like rather a lot of work to add a whole\nround of negotiation and a new protocol, when all we really need is one\nlittle tweak.\n\nI do think a fetch v2 protocol is potentially interesting for other\nreasons, but I don't know that I have the time to fully implement it. \n\nI wonder if it would be possible to just add these tweaks to v1, and\nsave the v2 work for when someone has the time to implement it?\n"},{"id":"283874","messageId":"20160419232243.GF18255@sigill.intra.peff.net","threadId":"42038","inReplyTo":"1461102001.5540.125.camel@twopensource.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-19T23:22:43Z","receivedAt":"2016-04-19T23:22:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 19, 2016 at 05:40:01PM -0400, David Turner wrote:\n\n> > I dunno, I am a bit negative on bringing new features to Git-over\n> > -HTTP\n> > (which is already less efficient than the other protocols!) without\n> > any\n> > plan for supporting them in the other protocols.\n> \n> Interesting -- can you expand on git-over-http being less efficient?\n> This is the first I'd heard of it. Is it documented somewhere?\n\nI don't know offhand of thorough discussion I can link to. But\nbasically, the issue is the tip negotiation that happens during a fetch.\nIn the normal git-over-ssh and git-over-tcp protocols, we're\nfull-duplex, and both sides remember all of the state. So the client is\nspewing \"want\" and \"have\" lines at the server, which is responding\nasynchronously with acks or naks until they reach a shared point to\ngenerate the pack from.\n\nIn the HTTP protocol, this negotiation has to happen via synchronous\nrequest/response pairs. So the client says \"here are some haves; what do\nyou think?\" and gets back the response. Then it prepares another of\nhaves, and so on, until the server says \"OK, I've seen enough; here's\nthe pack\". But because the server is stateless, each request has to\nsummarize the findings of the prior request. And so each request gets\nslightly bigger as we iterate.\n\nThere are some tunable parameters there (e.g., how many haves to send in\nthe first batch?), and the current settings are meant to be a mix of not\nwasting too much time preparing a request, but also putting enough into\nit that common requests can complete with only a single round trip.\n\nI don't have numbers on how often we have to fall back multiple\nrequests, or how big they can grow. I know I have very occasionally seen\npathological cases where we outgrew the HTTP buffer sizes, and re-trying\nthe fetch via ssh just worked.\n\nI'm cc-ing Shawn, who designed all of this, and can probably give more\ndetails (and may also have opinions on new http-only protocol features,\nas he'd probably end up implementing them in JGit, too).\n\nIt would be nice if we could do a true full-duplex conversation over\nHTTP. I looked into Websockets at one point, but IIRC there wasn't\nlibcurl support for them.\n\n> > So I'd rather see something like:\n> > \n> > 1. Support for v2 \"capabilities only\" initial negotiation, followed\n> > by ref advertisement.\n> > \n> > 2. Support for refspec-limiting capability.\n> > \n> > 3. HTTP-only option from client to trigger v2 on the server.\n> > \n> > That's still HTTP-specific, but it has a clear path for converging\n> > with\n> > the ssh and git protocols eventually, rather than having to support\n> > magic out-of-band capabilities forever.\n> > \n> > It does require an extra round of HTTP request/response, though.\n> \n> This seems way more complicated to me, and not necessarily super\n> -efficient. That is, it seems like rather a lot of work to add a whole\n> round of negotiation and a new protocol, when all we really need is one\n> little tweak.\n\nIt is less efficient because of the extra round. If the new protocol\nwere truly client-speaks-first, we could drop that round (which is\nessentially what your proposal is doing; you're just sticking the\nfirst-speak part into HTTP parameters).\n\nI don't know how much that round costs if it's part of the same TCP\nsession, or part of the same pipelined HTTP connection.\n\n> I wonder if it would be possible to just add these tweaks to v1, and\n> save the v2 work for when someone has the time to implement it?\n\nI don't think it's possible for the non-HTTP protocols. The single\nchange in v2 is to add a phase before the ref advertisement starts.\nWithout that, the server is going to start spewing advertisements.\n\nYou can find previous discussion on the list, but I think the options\nbasically are:\n\n 1. Something like v2, where the client gets a chance to speak before\n the advertisement.\n\n 2. Some out-of-band way of getting values from the client to the\n server (so maybe extra command-line arguments for git-over-ssh, and\n maybe shoving something after the \"\\0\" for git-daemon, and of\n course extra parameters for HTTP).\n\n 3. The client saying \"stop spewing refs at me, I want to give you a\n ref filter\" asynchronously, and accepting a little spew at the\n beginning of each conversation. That obviously only works for the\n full-duplex transports, so you'd probably fall back to (2) for\n http.\n\n-Peff\n"},{"id":"283891","messageId":"1461109391.5540.138.camel@twopensource.com","threadId":"42038","inReplyTo":"20160419232243.GF18255@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:43:11Z","receivedAt":"2016-04-19T23:43:11Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2016-04-19 at 19:22 -0400, Jeff King wrote:\n> You can find previous discussion on the list, but I think the options\n> basically are:\n> \n> 1. Something like v2, where the client gets a chance to speak\n> before\n> the advertisement.\n> \n> 2. Some out-of-band way of getting values from the client to the\n> server (so maybe extra command-line arguments for git-over-ssh,\n> and\n> maybe shoving something after the \"\\0\" for git-daemon, and of\n> course extra parameters for HTTP).\n> \n> 3. The client saying \"stop spewing refs at me, I want to give you a\n> ref filter\" asynchronously, and accepting a little spew at the\n> beginning of each conversation. That obviously only works for\n> the\n> full-duplex transports, so you'd probably fall back to (2) for\n> http.\n\nOK, so (2) seems like what I'm doing -- it just happens that I only\nimplemented it for one protocol. \n"},{"id":"283900","messageId":"20160420011740.GA29839@sigill.intra.peff.net","threadId":"42038","inReplyTo":"1461109391.5540.138.camel@twopensource.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-20T01:17:40Z","receivedAt":"2016-04-20T01:17:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 19, 2016 at 07:43:11PM -0400, David Turner wrote:\n\n> On Tue, 2016-04-19 at 19:22 -0400, Jeff King wrote:\n> > You can find previous discussion on the list, but I think the options\n> > basically are:\n> > \n> > 1. Something like v2, where the client gets a chance to speak\n> > before\n> > the advertisement.\n> > \n> > 2. Some out-of-band way of getting values from the client to the\n> > server (so maybe extra command-line arguments for git-over-ssh,\n> > and\n> > maybe shoving something after the \"\\0\" for git-daemon, and of\n> > course extra parameters for HTTP).\n> > \n> > 3. The client saying \"stop spewing refs at me, I want to give you a\n> > ref filter\" asynchronously, and accepting a little spew at the\n> > beginning of each conversation. That obviously only works for\n> > the\n> > full-duplex transports, so you'd probably fall back to (2) for\n> > http.\n> \n> OK, so (2) seems like what I'm doing -- it just happens that I only\n> implemented it for one protocol.\n\nRight. And I don't mind that approach _if_ we can figure out a way to do\nit for all protocols. But I think there are some complications with the\nother ones, which means that HTTP will have the ability to grow features\nthe other protocols do not.\n\n-Peff\n"},{"id":"283998","messageId":"1461185215.5540.180.camel@twopensource.com","threadId":"42038","inReplyTo":"20160420011740.GA29839@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-20T20:46:55Z","receivedAt":"2016-04-20T20:46:55Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2016-04-19 at 21:17 -0400, Jeff King wrote:\n> On Tue, Apr 19, 2016 at 07:43:11PM -0400, David Turner wrote:\n> \n> > On Tue, 2016-04-19 at 19:22 -0400, Jeff King wrote:\n> > > You can find previous discussion on the list, but I think the\n> > > options\n> > > basically are:\n> > > \n> > > 1. Something like v2, where the client gets a chance to speak\n> > > before\n> > > the advertisement.\n> > > \n> > > 2. Some out-of-band way of getting values from the client to\n> > > the\n> > > server (so maybe extra command-line arguments for git-over\n> > > -ssh,\n> > > and\n> > > maybe shoving something after the \"\\0\" for git-daemon, and\n> > > of\n> > > course extra parameters for HTTP).\n> > > \n> > > 3. The client saying \"stop spewing refs at me, I want to give\n> > > you a\n> > > ref filter\" asynchronously, and accepting a little spew at\n> > > the\n> > > beginning of each conversation. That obviously only works\n> > > for\n> > > the\n> > > full-duplex transports, so you'd probably fall back to (2)\n> > > for\n> > > http.\n> > \n> > OK, so (2) seems like what I'm doing -- it just happens that I only\n> > implemented it for one protocol.\n> \n> Right. And I don't mind that approach _if_ we can figure out a way to\n> do\n> it for all protocols. But I think there are some complications with\n> the\n> other ones, which means that HTTP will have the ability to grow\n> features\n> the other protocols do not.\n\nAs you note, it appears that git-daemon does sort-of have support for\nextra args -- see parse_host_arg. So it wouldn't be hard to add\nsomething here. Unfortunately, current versions of git die on unknown\nargs. So this change would not be backwards-compatible. We could put\na decider on it so that clients would only try it when explicitly\nenabled. Or we could have clients try it with, and in the event of an\nerror, retry without. Neither is ideal, but both are possible.\n\nIf I read this code correctly, git-over-ssh will pass through arbitrary\narguments. So this should be trivial.\n"},{"id":"283999","messageId":"20160420205726.GA17876@sigill.intra.peff.net","threadId":"42038","inReplyTo":"1461185215.5540.180.camel@twopensource.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-20T20:57:26Z","receivedAt":"2016-04-20T20:57:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:\n\n> As you note, it appears that git-daemon does sort-of have support for\n> extra args -- see parse_host_arg. So it wouldn't be hard to add\n> something here. Unfortunately, current versions of git die on unknown\n> args. So this change would not be backwards-compatible. We could put\n> a decider on it so that clients would only try it when explicitly\n> enabled. Or we could have clients try it with, and in the event of an\n> error, retry without. Neither is ideal, but both are possible.\n\nRight. This ends up being the same difficulty that the v2 protocol\nencountered; how do you figure out what you can speak without resorting\nto expensive fallbacks, when do you flip the switch, do you remember the\nprotocol you used last time with this server, etc.\n\nWhich isn't to say it's necessarily a bad thing. Maybe the path forward\ninstead of v2 is to shoe-horn this data into the pre-protocol\nconversation, and go from there. The protocol accepts that \"somehow\" it\ngot some extra data from the transport layer, and acts on its uniformly.\n\n> If I read this code correctly, git-over-ssh will pass through arbitrary\n> arguments. So this should be trivial.\n\nIt does if you are ssh-ing to a real shell-level account on the server,\nbut if you are using git-shell or some other wrapper to restrict clients\nfrom running arbitrary commands, it will likely reject it.\n\n-Peff\n"},{"id":"284433","messageId":"1461602647.25914.2.camel@twopensource.com","threadId":"42038","inReplyTo":"20160420205726.GA17876@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-25T16:44:07Z","receivedAt":"2016-04-25T16:44:07Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 16:57 -0400, Jeff King wrote:\n> On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:\n> \n> > As you note, it appears that git-daemon does sort-of have support\n> > for\n> > extra args -- see parse_host_arg. So it wouldn't be hard to add\n> > something here. Unfortunately, current versions of git die on\n> > unknown\n> > args. So this change would not be backwards-compatible. We could\n> > put\n> > a decider on it so that clients would only try it when explicitly\n> > enabled. Or we could have clients try it with, and in the event of\n> > an\n> > error, retry without. Neither is ideal, but both are possible.\n> \n> Right. This ends up being the same difficulty that the v2 protocol\n> encountered; how do you figure out what you can speak without\n> resorting\n> to expensive fallbacks, when do you flip the switch, do you remember\n> the\n> protocol you used last time with this server, etc.\n\nRight.\n\n[moved]\n> > If I read this code correctly, git-over-ssh will pass through\n> > arbitrary\n> > arguments. So this should be trivial.\n> \n> It does if you are ssh-ing to a real shell-level account on the\n> server,\n> but if you are using git-shell or some other wrapper to restrict\n> clients\n> from running arbitrary commands, it will likely reject it.\n\nOh, I see how I was mis-reading shell.c. Oops.\n[/moved]\n\n\n> Which isn't to say it's necessarily a bad thing. Maybe the path\n> forward\n> instead of v2 is to shoe-horn this data into the pre-protocol\n> conversation, and go from there. The protocol accepts that \"somehow\"\n> it\n> got some extra data from the transport layer, and acts on its\n> uniformly.\n\nOK, so it seems like only HTTP (and non-git-shell-git://) allow backwar\nds-compatible optional pre-protocol messages. So we don't have good\noptions; we only have bad ones. We have to decide which particular\nkind of badness we're willing to accept, and to what degree we care\nabout extensibility. As I see it, the badness options are (in no\nparticular order):\n\n1. Nothing changes.\n2. HTTP grows more extensions; other protocols stagnate.\n3. HTTP grows extensions; other protocols get extensions but:\n a. only use them on explicit client configuration or\n b. try/fail/remember per-remote\n4. A backwards-incompatible protocol v2 is introduced, which\n hits alternate endpoints (with the same a/b as above). This is\n different from 3. in that protocol v2 is explicitly designed around\n a capabilities negotiation phase rather than unilateral client-side\n decisions.\n5. Think of another way to make fetch performant with many refs, and \t\n defer the extension decision.\n"},{"id":"284499","messageId":"CAGZ79kYc+uMCJq5tTHujp1YYG5OcNSPiyWuGvVzipuBcwtLNYA@mail.gmail.com","threadId":"42038","inReplyTo":"1461602647.25914.2.camel@twopensource.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-25T22:10:37Z","receivedAt":"2016-04-25T22:10:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Apr 25, 2016 at 9:44 AM, David Turner wrote:\n> On Wed, 2016-04-20 at 16:57 -0400, Jeff King wrote:\n>> On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:\n>>\n>> > As you note, it appears that git-daemon does sort-of have support\n>> > for\n>> > extra args -- see parse_host_arg. So it wouldn't be hard to add\n>> > something here. Unfortunately, current versions of git die on\n>> > unknown\n>> > args. So this change would not be backwards-compatible. We could\n>> > put\n>> > a decider on it so that clients would only try it when explicitly\n>> > enabled. Or we could have clients try it with, and in the event of\n>> > an\n>> > error, retry without. Neither is ideal, but both are possible.\n>>\n>> Right. This ends up being the same difficulty that the v2 protocol\n>> encountered; how do you figure out what you can speak without\n>> resorting\n>> to expensive fallbacks, when do you flip the switch, do you remember\n>> the\n>> protocol you used last time with this server, etc.\n>\n> Right.\n>\n> [moved]\n>> > If I read this code correctly, git-over-ssh will pass through\n>> > arbitrary\n>> > arguments. So this should be trivial.\n>>\n>> It does if you are ssh-ing to a real shell-level account on the\n>> server,\n>> but if you are using git-shell or some other wrapper to restrict\n>> clients\n>> from running arbitrary commands, it will likely reject it.\n>\n> Oh, I see how I was mis-reading shell.c. Oops.\n> [/moved]\n>\n>\n>> Which isn't to say it's necessarily a bad thing. Maybe the path\n>> forward\n>> instead of v2 is to shoe-horn this data into the pre-protocol\n>> conversation, and go from there. The protocol accepts that \"somehow\"\n>> it\n>> got some extra data from the transport layer, and acts on its\n>> uniformly.\n>\n> OK, so it seems like only HTTP (and non-git-shell-git://) allow backwar\n> ds-compatible optional pre-protocol messages. So we don't have good\n> options; we only have bad ones. We have to decide which particular\n> kind of badness we're willing to accept, and to what degree we care\n> about extensibility. As I see it, the badness options are (in no\n> particular order):\n>\n> 1. Nothing changes.\n> 2. HTTP grows more extensions; other protocols stagnate.\n> 3. HTTP grows extensions; other protocols get extensions but:\n> a. only use them on explicit client configuration or\n> b. try/fail/remember per-remote\n> 4. A backwards-incompatible protocol v2 is introduced, which\n> hits alternate endpoints (with the same a/b as above). This is\n> different from 3. in that protocol v2 is explicitly designed around\n> a capabilities negotiation phase rather than unilateral client-side\n> decisions.\n> 5. Think of another way to make fetch performant with many refs, and\n> defer the extension decision.\n\nI'd prefer 2,3,4 over 1,5.\n\nSpeaking about 2,3,4:\n\nMaybe we can do a mix of 2 and 4:\n\n 1) HTTP grows more extensions; other protocols stagnate for now.\n 2) Come up with a backwards-incompatible protocol v2, foccussed on\n capabilities negotiation phase, hitting alternative end points\n (non http only, or rather a subset of protocols only)\n 3) if HTTP sees the benefits of the native protocol v2, we may switch\n HTTP, too\n\n(in time order of execution. Each point is decoupled from the others and may\nbe done by different people at different times.)\n\n\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"284625","messageId":"CAGZ79kYGi9bhRfGype7te4cGkxvnKww269kYoykB+76HNnTarg@mail.gmail.com","threadId":"42038","inReplyTo":"CAGZ79kYc+uMCJq5tTHujp1YYG5OcNSPiyWuGvVzipuBcwtLNYA@mail.gmail.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-27T03:59:22Z","receivedAt":"2016-04-27T03:59:22Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Apr 25, 2016 at 3:10 PM, Stefan Beller wrote:\n> On Mon, Apr 25, 2016 at 9:44 AM, David Turner wrote:\n>> On Wed, 2016-04-20 at 16:57 -0400, Jeff King wrote:\n>>> On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:\n>>>\n>>> > As you note, it appears that git-daemon does sort-of have support\n>>> > for\n>>> > extra args -- see parse_host_arg. So it wouldn't be hard to add\n>>> > something here. Unfortunately, current versions of git die on\n>>> > unknown\n>>> > args. So this change would not be backwards-compatible. We could\n>>> > put\n>>> > a decider on it so that clients would only try it when explicitly\n>>> > enabled. Or we could have clients try it with, and in the event of\n>>> > an\n>>> > error, retry without. Neither is ideal, but both are possible.\n>>>\n>>> Right. This ends up being the same difficulty that the v2 protocol\n>>> encountered; how do you figure out what you can speak without\n>>> resorting\n>>> to expensive fallbacks, when do you flip the switch, do you remember\n>>> the\n>>> protocol you used last time with this server, etc.\n>>\n>> Right.\n>>\n>> [moved]\n>>> > If I read this code correctly, git-over-ssh will pass through\n>>> > arbitrary\n>>> > arguments. So this should be trivial.\n>>>\n>>> It does if you are ssh-ing to a real shell-level account on the\n>>> server,\n>>> but if you are using git-shell or some other wrapper to restrict\n>>> clients\n>>> from running arbitrary commands, it will likely reject it.\n>>\n>> Oh, I see how I was mis-reading shell.c. Oops.\n>> [/moved]\n>>\n>>\n>>> Which isn't to say it's necessarily a bad thing. Maybe the path\n>>> forward\n>>> instead of v2 is to shoe-horn this data into the pre-protocol\n>>> conversation, and go from there. The protocol accepts that \"somehow\"\n>>> it\n>>> got some extra data from the transport layer, and acts on its\n>>> uniformly.\n>>\n>> OK, so it seems like only HTTP (and non-git-shell-git://) allow backwar\n>> ds-compatible optional pre-protocol messages. So we don't have good\n>> options; we only have bad ones. We have to decide which particular\n>> kind of badness we're willing to accept, and to what degree we care\n>> about extensibility. As I see it, the badness options are (in no\n>> particular order):\n>>\n>> 1. Nothing changes.\n>> 2. HTTP grows more extensions; other protocols stagnate.\n>> 3. HTTP grows extensions; other protocols get extensions but:\n>> a. only use them on explicit client configuration or\n>> b. try/fail/remember per-remote\n>> 4. A backwards-incompatible protocol v2 is introduced, which\n>> hits alternate endpoints (with the same a/b as above). This is\n>> different from 3. in that protocol v2 is explicitly designed around\n>> a capabilities negotiation phase rather than unilateral client-side\n>> decisions.\n>> 5. Think of another way to make fetch performant with many refs, and\n>> defer the extension decision.\n>\n> I'd prefer 2,3,4 over 1,5.\n>\n> Speaking about 2,3,4:\n>\n> Maybe we can do a mix of 2 and 4:\n>\n> 1) HTTP grows more extensions; other protocols stagnate for now.\n> 2) Come up with a backwards-incompatible protocol v2, foccussed on\n> capabilities negotiation phase, hitting alternative end points\n> (non http only, or rather a subset of protocols only)\n> 3) if HTTP sees the benefits of the native protocol v2, we may switch\n> HTTP, too\n>\n> (in time order of execution. Each point is decoupled from the others and may\n> be done by different people at different times.)\n>\n\nToday I rebased protocol-v2[1] and it was fewer conflicts than expected.\nI am surprised by myself that there is even a test case for v2 already,\nso I think it is more progressed that I had in mind. Maybe we can do 1)\nfor now and hope that the non http catches up eventually?\n\n\n[1] https://github.com/stefanbeller/git/tree/protocol-v2\n"},{"id":"284626","messageId":"20160427041138.GA22920@sigill.intra.peff.net","threadId":"42038","inReplyTo":"CAGZ79kYGi9bhRfGype7te4cGkxvnKww269kYoykB+76HNnTarg@mail.gmail.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-27T04:11:38Z","receivedAt":"2016-04-27T04:11:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 26, 2016 at 08:59:22PM -0700, Stefan Beller wrote:\n\n> > Maybe we can do a mix of 2 and 4:\n> >\n> > 1) HTTP grows more extensions; other protocols stagnate for now.\n> > 2) Come up with a backwards-incompatible protocol v2, foccussed on\n> > capabilities negotiation phase, hitting alternative end points\n> > (non http only, or rather a subset of protocols only)\n> > 3) if HTTP sees the benefits of the native protocol v2, we may switch\n> > HTTP, too\n> >\n> > (in time order of execution. Each point is decoupled from the others and may\n> > be done by different people at different times.)\n> >\n> \n> Today I rebased protocol-v2[1] and it was fewer conflicts than expected.\n> I am surprised by myself that there is even a test case for v2 already,\n> so I think it is more progressed that I had in mind. Maybe we can do 1)\n> for now and hope that the non http catches up eventually?\n\nIf the plan is something like:\n\n 1. v2 exists, but client/server don't know when they should use it.\n\n 2. smart-http grows an extra parameter for the client to say \"hey, I\n know v2\"\n\n 3. Other protocols get some manual v2 support (e.g., client asks for\n \"upload-pack2\" if instructed by the user, server either speaks v2\n then or says \"eh, what?\").\n\nI like that very much. It lets us \"cheat\" on the hard part of the\nproblem for http, which is what David's series is doing, but it provides\na clear path forward for the protocols eventually reaching feature\nparity (namely that eventually all servers speak v2, and the client can\nstart asking for v2 by default).\n\n-Peff\n"},{"id":"284650","messageId":"xmqqmvofay46.fsf@gitster.mtv.corp.google.com","threadId":"42038","inReplyTo":"20160427041138.GA22920@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-27T15:07:53Z","receivedAt":"2016-04-27T15:07:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If the plan is something like:\n>\n> 1. v2 exists, but client/server don't know when they should use it.\n>\n> 2. smart-http grows an extra parameter for the client to say \"hey, I\n> know v2\"\n>\n> 3. Other protocols get some manual v2 support (e.g., client asks for\n> \"upload-pack2\" if instructed by the user, server either speaks v2\n> then or says \"eh, what?\").\n>\n> I like that very much. It lets us \"cheat\" on the hard part of the\n> problem for http, which is what David's series is doing, but it provides\n> a clear path forward for the protocols eventually reaching feature\n> parity (namely that eventually all servers speak v2, and the client can\n> start asking for v2 by default).\n\nYup, thanks for clearly writing it down. I like the above very much,\ntoo.\n"},{"id":"284988","messageId":"1461971144.4123.38.camel@twopensource.com","threadId":"42038","inReplyTo":"CAGZ79kYGi9bhRfGype7te4cGkxvnKww269kYoykB+76HNnTarg@mail.gmail.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-29T23:05:44Z","receivedAt":"2016-04-29T23:05:44Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2016-04-26 at 20:59 -0700, Stefan Beller wrote:\n> On Mon, Apr 25, 2016 at 3:10 PM, Stefan Beller \n> wrote:\n> > On Mon, Apr 25, 2016 at 9:44 AM, David Turner <\n> > dturner@twopensource.com> wrote:\n> > > On Wed, 2016-04-20 at 16:57 -0400, Jeff King wrote:\n> > > > On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:\n> > > > \n> > > > > As you note, it appears that git-daemon does sort-of have\n> > > > > support\n> > > > > for\n> > > > > extra args -- see parse_host_arg. So it wouldn't be hard to\n> > > > > add\n> > > > > something here. Unfortunately, current versions of git die on\n> > > > > unknown\n> > > > > args. So this change would not be backwards-compatible. We\n> > > > > could\n> > > > > put\n> > > > > a decider on it so that clients would only try it when\n> > > > > explicitly\n> > > > > enabled. Or we could have clients try it with, and in the\n> > > > > event of\n> > > > > an\n> > > > > error, retry without. Neither is ideal, but both are\n> > > > > possible.\n> > > > \n> > > > Right. This ends up being the same difficulty that the v2\n> > > > protocol\n> > > > encountered; how do you figure out what you can speak without\n> > > > resorting\n> > > > to expensive fallbacks, when do you flip the switch, do you\n> > > > remember\n> > > > the\n> > > > protocol you used last time with this server, etc.\n> > > \n> > > Right.\n> > > \n> > > [moved]\n> > > > > If I read this code correctly, git-over-ssh will pass through\n> > > > > arbitrary\n> > > > > arguments. So this should be trivial.\n> > > > \n> > > > It does if you are ssh-ing to a real shell-level account on the\n> > > > server,\n> > > > but if you are using git-shell or some other wrapper to\n> > > > restrict\n> > > > clients\n> > > > from running arbitrary commands, it will likely reject it.\n> > > \n> > > Oh, I see how I was mis-reading shell.c. Oops.\n> > > [/moved]\n> > > \n> > > \n> > > > Which isn't to say it's necessarily a bad thing. Maybe the path\n> > > > forward\n> > > > instead of v2 is to shoe-horn this data into the pre-protocol\n> > > > conversation, and go from there. The protocol accepts that\n> > > > \"somehow\"\n> > > > it\n> > > > got some extra data from the transport layer, and acts on its\n> > > > uniformly.\n> > > \n> > > OK, so it seems like only HTTP (and non-git-shell-git://) allow\n> > > backwar\n> > > ds-compatible optional pre-protocol messages. So we don't have\n> > > good\n> > > options; we only have bad ones. We have to decide which\n> > > particular\n> > > kind of badness we're willing to accept, and to what degree we\n> > > care\n> > > about extensibility. As I see it, the badness options are (in no\n> > > particular order):\n> > > \n> > > 1. Nothing changes.\n> > > 2. HTTP grows more extensions; other protocols stagnate.\n> > > 3. HTTP grows extensions; other protocols get extensions but:\n> > > a. only use them on explicit client configuration or\n> > > b. try/fail/remember per-remote\n> > > 4. A backwards-incompatible protocol v2 is introduced, which\n> > > hits alternate endpoints (with the same a/b as above). This\n> > > is\n> > > different from 3. in that protocol v2 is explicitly designed\n> > > around\n> > > a capabilities negotiation phase rather than unilateral client\n> > > -side\n> > > decisions.\n> > > 5. Think of another way to make fetch performant with many refs,\n> > > and\n> > > defer the extension decision.\n> > \n> > I'd prefer 2,3,4 over 1,5.\n> > \n> > Speaking about 2,3,4:\n> > \n> > Maybe we can do a mix of 2 and 4:\n> > \n> > 1) HTTP grows more extensions; other protocols stagnate for now.\n> > 2) Come up with a backwards-incompatible protocol v2, foccussed\n> > on\n> > capabilities negotiation phase, hitting alternative end\n> > points\n> > (non http only, or rather a subset of protocols only)\n> > 3) if HTTP sees the benefits of the native protocol v2, we may\n> > switch\n> > HTTP, too\n> > \n> > (in time order of execution. Each point is decoupled from the\n> > others and may\n> > be done by different people at different times.)\n> > \n> \n> Today I rebased protocol-v2[1] and it was fewer conflicts than\n> expected.\n> I am surprised by myself that there is even a test case for v2\n> already,\n> so I think it is more progressed that I had in mind. Maybe we can do\n> 1)\n> for now and hope that the non http catches up eventually?\n> \n> \n> [1] https://github.com/stefanbeller/git/tree/protocol-v2\n\n\nDo you plan to send these patches to the mailing list? What's the next\nstep here?\n"},{"id":"284989","messageId":"CAGZ79kYy0TxmUAr-QdAw-KNqD0DyxfvdjB6Jkgg3bzRDTGWdrA@mail.gmail.com","threadId":"42038","inReplyTo":"1461971144.4123.38.camel@twopensource.com","subject":"Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-29T23:12:01Z","receivedAt":"2016-04-29T23:12:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 29, 2016 at 4:05 PM, David Turner wrote:\n> On Tue, 2016-04-26 at 20:59 -0700, Stefan Beller wrote:\n>> On Mon, Apr 25, 2016 at 3:10 PM, Stefan Beller \n>> wrote:\n>> > On Mon, Apr 25, 2016 at 9:44 AM, David Turner <\n>> > dturner@twopensource.com> wrote:\n>> > > On Wed, 2016-04-20 at 16:57 -0400, Jeff King wrote:\n>> > > > On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:\n>> > > >\n>> > > > > As you note, it appears that git-daemon does sort-of have\n>> > > > > support\n>> > > > > for\n>> > > > > extra args -- see parse_host_arg. So it wouldn't be hard to\n>> > > > > add\n>> > > > > something here. Unfortunately, current versions of git die on\n>> > > > > unknown\n>> > > > > args. So this change would not be backwards-compatible. We\n>> > > > > could\n>> > > > > put\n>> > > > > a decider on it so that clients would only try it when\n>> > > > > explicitly\n>> > > > > enabled. Or we could have clients try it with, and in the\n>> > > > > event of\n>> > > > > an\n>> > > > > error, retry without. Neither is ideal, but both are\n>> > > > > possible.\n>> > > >\n>> > > > Right. This ends up being the same difficulty that the v2\n>> > > > protocol\n>> > > > encountered; how do you figure out what you can speak without\n>> > > > resorting\n>> > > > to expensive fallbacks, when do you flip the switch, do you\n>> > > > remember\n>> > > > the\n>> > > > protocol you used last time with this server, etc.\n>> > >\n>> > > Right.\n>> > >\n>> > > [moved]\n>> > > > > If I read this code correctly, git-over-ssh will pass through\n>> > > > > arbitrary\n>> > > > > arguments. So this should be trivial.\n>> > > >\n>> > > > It does if you are ssh-ing to a real shell-level account on the\n>> > > > server,\n>> > > > but if you are using git-shell or some other wrapper to\n>> > > > restrict\n>> > > > clients\n>> > > > from running arbitrary commands, it will likely reject it.\n>> > >\n>> > > Oh, I see how I was mis-reading shell.c. Oops.\n>> > > [/moved]\n>> > >\n>> > >\n>> > > > Which isn't to say it's necessarily a bad thing. Maybe the path\n>> > > > forward\n>> > > > instead of v2 is to shoe-horn this data into the pre-protocol\n>> > > > conversation, and go from there. The protocol accepts that\n>> > > > \"somehow\"\n>> > > > it\n>> > > > got some extra data from the transport layer, and acts on its\n>> > > > uniformly.\n>> > >\n>> > > OK, so it seems like only HTTP (and non-git-shell-git://) allow\n>> > > backwar\n>> > > ds-compatible optional pre-protocol messages. So we don't have\n>> > > good\n>> > > options; we only have bad ones. We have to decide which\n>> > > particular\n>> > > kind of badness we're willing to accept, and to what degree we\n>> > > care\n>> > > about extensibility. As I see it, the badness options are (in no\n>> > > particular order):\n>> > >\n>> > > 1. Nothing changes.\n>> > > 2. HTTP grows more extensions; other protocols stagnate.\n>> > > 3. HTTP grows extensions; other protocols get extensions but:\n>> > > a. only use them on explicit client configuration or\n>> > > b. try/fail/remember per-remote\n>> > > 4. A backwards-incompatible protocol v2 is introduced, which\n>> > > hits alternate endpoints (with the same a/b as above). This\n>> > > is\n>> > > different from 3. in that protocol v2 is explicitly designed\n>> > > around\n>> > > a capabilities negotiation phase rather than unilateral client\n>> > > -side\n>> > > decisions.\n>> > > 5. Think of another way to make fetch performant with many refs,\n>> > > and\n>> > > defer the extension decision.\n>> >\n>> > I'd prefer 2,3,4 over 1,5.\n>> >\n>> > Speaking about 2,3,4:\n>> >\n>> > Maybe we can do a mix of 2 and 4:\n>> >\n>> > 1) HTTP grows more extensions; other protocols stagnate for now.\n>> > 2) Come up with a backwards-incompatible protocol v2, foccussed\n>> > on\n>> > capabilities negotiation phase, hitting alternative end\n>> > points\n>> > (non http only, or rather a subset of protocols only)\n>> > 3) if HTTP sees the benefits of the native protocol v2, we may\n>> > switch\n>> > HTTP, too\n>> >\n>> > (in time order of execution. Each point is decoupled from the\n>> > others and may\n>> > be done by different people at different times.)\n>> >\n>>\n>> Today I rebased protocol-v2[1] and it was fewer conflicts than\n>> expected.\n>> I am surprised by myself that there is even a test case for v2\n>> already,\n>> so I think it is more progressed that I had in mind. Maybe we can do\n>> 1)\n>> for now and hope that the non http catches up eventually?\n>>\n>>\n>> [1] https://github.com/stefanbeller/git/tree/protocol-v2\n>\n>\n> Do you plan to send these patches to the mailing list? What's the next\n> step here?\n\nI can send them out if you want to. As I flip flop between\n\"they are perfect\" and \"they are horrible, nobody should see them\",\nI haven't done it so far (also time constraints).\n\nWill send them later today, in case it's urgent you can fetch them from\nmy github account.\n\nWhat's the next step?\nI don't know. I guess we can either collaborate on a large\nseries (do-it-all) or rather work on many smaller series'\nthat solve it partially one by one.\n\nI'll send the patches out later today.\n\nThanks,\nStefan\n"}]}