{"thread":{"id":"39489","subject":"[RFCv2 07/16] transport: add infrastructure to support a protocol version number","startedAt":"2015-06-02T00:02:02Z","lastAt":"2015-06-03T00:16:16Z","messageCount":44,"participants":["Stefan Beller","Duy Nguyen","Ilari Liusvaara","Eric Sunshine","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"262680","messageId":"1433203338-27493-1-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":null,"subject":"[RFCv2 00/16] Protocol version 2","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:02Z","receivedAt":"2015-06-02T00:02:02Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"patches 1-4 are for the uploading side (upload-pack)\nand 5-13 for the fetching side.\n\npatches 14-16 are documentation and the tiny test.\n\nNguyễn Thái Ngọc Duy (1):\n upload-pack: make client capability parsing code a separate function\n\nStefan Beller (15):\n stringlist: add from_space_separated_string\n connect: rewrite feature parsing to work on string_list\n upload-pack-2: Implement the version 2 of upload-pack\n remote.h: Change get_remote_heads return to void\n remote.h: add new struct for options\n transport: add infrastructure to support a protocol version number\n transport: select transport version via command line or config\n remote.h: add get_remote_capabilities, request_capabilities\n transport: connect_setup appends protocol version number\n remote: have preselect_capabilities\n transport: get_refs_via_connect exchanges capabilities before refs.\n fetch-pack: use the configured transport protocol\n t5544: add a test case for the new protocol\n Documentation/technical/pack-protocol: Mention http as possible\n protocol\n Document protocol version 2\n\n .gitignore | 1 +\n Documentation/technical/pack-protocol.txt | 85 ++++++++++--\n Documentation/technical/protocol-capabilities.txt | 15 ---\n Makefile | 2 +\n builtin/fetch-pack.c | 22 +++-\n builtin/fetch.c | 6 +\n builtin/receive-pack.c | 13 +-\n connect.c | 154 +++++++++++++++-------\n connect.h | 2 +-\n fetch-pack.c | 109 ++++++++-------\n fetch-pack.h | 1 +\n remote.c | 2 +\n remote.h | 28 +++-\n string-list.c | 12 ++\n string-list.h | 1 +\n t/t5544-fetch-2.sh | 40 ++++++\n transport-helper.c | 1 +\n transport.c | 60 ++++++++-\n transport.h | 11 ++\n upload-pack-2.c | 1 +\n upload-pack.c | 151 +++++++++++++++------\n 21 files changed, 535 insertions(+), 182 deletions(-)\n create mode 100755 t/t5544-fetch-2.sh\n create mode 120000 upload-pack-2.c\n\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262670","messageId":"1433203338-27493-2-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 01/16] stringlist: add from_space_separated_string","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:03Z","receivedAt":"2015-06-02T00:02:03Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This function parses a space separated string into a string list.\n\nSigned-off-by: Stefan Beller \n---\n string-list.c | 12 ++++++++++++\n string-list.h | 1 +\n 2 files changed, 13 insertions(+)\n\ndiff --git a/string-list.c b/string-list.c\nindex 2a32a3f..f71384f 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -7,6 +7,18 @@ void string_list_init(struct string_list *list, int strdup_strings)\n \tlist->strdup_strings = strdup_strings;\n }\n \n+void from_space_separated_string(struct string_list *list, char *line)\n+{\n+\tchar *save_ptr;\n+\tconst char delimiters[] = \" \\t\\n\";\n+\tconst char *token = strtok_r(line, delimiters, &save_ptr);\n+\n+\twhile (token) {\n+\t\tstring_list_append(list, xstrdup(token));\n+\t\ttoken = strtok_r(NULL, delimiters, &save_ptr);\n+\t}\n+}\n+\n /* if there is no exact match, point to the index where the entry could be\n * inserted */\n static int get_entry_index(const struct string_list *list, const char *string,\ndiff --git a/string-list.h b/string-list.h\nindex d3809a1..88c18e9 100644\n--- a/string-list.h\n+++ b/string-list.h\n@@ -19,6 +19,7 @@ struct string_list {\n #define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }\n \n void string_list_init(struct string_list *list, int strdup_strings);\n+void from_space_separated_string(struct string_list *list, char *line);\n \n void print_string_list(const struct string_list *p, const char *text);\n void string_list_clear(struct string_list *list, int free_util);\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262674","messageId":"1433203338-27493-3-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 02/16] upload-pack: make client capability parsing code a separate function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:04Z","receivedAt":"2015-06-02T00:02:04Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Stefan Beller \n---\n upload-pack.c | 44 +++++++++++++++++++++++---------------------\n 1 file changed, 23 insertions(+), 21 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 745fda8..5449ff7 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -531,6 +531,28 @@ error:\n \t}\n }\n \n+static void parse_features(const char *features)\n+{\n+\tif (parse_feature_request(features, \"multi_ack_detailed\"))\n+\t\tmulti_ack = 2;\n+\telse if (parse_feature_request(features, \"multi_ack\"))\n+\t\tmulti_ack = 1;\n+\tif (parse_feature_request(features, \"no-done\"))\n+\t\tno_done = 1;\n+\tif (parse_feature_request(features, \"thin-pack\"))\n+\t\tuse_thin_pack = 1;\n+\tif (parse_feature_request(features, \"ofs-delta\"))\n+\t\tuse_ofs_delta = 1;\n+\tif (parse_feature_request(features, \"side-band-64k\"))\n+\t\tuse_sideband = LARGE_PACKET_MAX;\n+\telse if (parse_feature_request(features, \"side-band\"))\n+\t\tuse_sideband = DEFAULT_PACKET_MAX;\n+\tif (parse_feature_request(features, \"no-progress\"))\n+\t\tno_progress = 1;\n+\tif (parse_feature_request(features, \"include-tag\"))\n+\t\tuse_include_tag = 1;\n+}\n+\n static void receive_needs(void)\n {\n \tstruct object_array shallows = OBJECT_ARRAY_INIT;\n@@ -540,7 +562,6 @@ static void receive_needs(void)\n \tshallow_nr = 0;\n \tfor (;;) {\n \t\tstruct object *o;\n-\t\tconst char *features;\n \t\tunsigned char sha1_buf[20];\n \t\tchar *line = packet_read_line(0, NULL);\n \t\treset_timeout();\n@@ -575,26 +596,7 @@ static void receive_needs(void)\n \t\t\tdie(\"git upload-pack: protocol error, \"\n \t\t\t \"expected to get sha, not '%s'\", line);\n \n-\t\tfeatures = line + 45;\n-\n-\t\tif (parse_feature_request(features, \"multi_ack_detailed\"))\n-\t\t\tmulti_ack = 2;\n-\t\telse if (parse_feature_request(features, \"multi_ack\"))\n-\t\t\tmulti_ack = 1;\n-\t\tif (parse_feature_request(features, \"no-done\"))\n-\t\t\tno_done = 1;\n-\t\tif (parse_feature_request(features, \"thin-pack\"))\n-\t\t\tuse_thin_pack = 1;\n-\t\tif (parse_feature_request(features, \"ofs-delta\"))\n-\t\t\tuse_ofs_delta = 1;\n-\t\tif (parse_feature_request(features, \"side-band-64k\"))\n-\t\t\tuse_sideband = LARGE_PACKET_MAX;\n-\t\telse if (parse_feature_request(features, \"side-band\"))\n-\t\t\tuse_sideband = DEFAULT_PACKET_MAX;\n-\t\tif (parse_feature_request(features, \"no-progress\"))\n-\t\t\tno_progress = 1;\n-\t\tif (parse_feature_request(features, \"include-tag\"))\n-\t\t\tuse_include_tag = 1;\n+\t\tparse_features(line + 45);\n \n \t\to = parse_object(sha1_buf);\n \t\tif (!o)\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262667","messageId":"1433203338-27493-4-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 03/16] connect: rewrite feature parsing to work on string_list","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:05Z","receivedAt":"2015-06-02T00:02:05Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n builtin/receive-pack.c | 13 +++++----\n connect.c | 79 ++++++++++++++++++++++----------------------------\n connect.h | 2 +-\n upload-pack.c | 11 +++++--\n 4 files changed, 51 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d2ec52b..518ce85 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1391,16 +1391,19 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \n \t\tlinelen = strlen(line);\n \t\tif (linelen < len) {\n-\t\t\tconst char *feature_list = line + linelen + 1;\n-\t\t\tif (parse_feature_request(feature_list, \"report-status\"))\n+\t\t\tstruct string_list feature_list = STRING_LIST_INIT_DUP;\n+\t\t\tfrom_space_separated_string(&feature_list, line + linelen + 1);\n+\t\t\tif (parse_feature_request(&feature_list, \"report-status\"))\n \t\t\t\treport_status = 1;\n-\t\t\tif (parse_feature_request(feature_list, \"side-band-64k\"))\n+\t\t\tif (parse_feature_request(&feature_list, \"side-band-64k\"))\n \t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n-\t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n+\t\t\tif (parse_feature_request(&feature_list, \"quiet\"))\n \t\t\t\tquiet = 1;\n \t\t\tif (advertise_atomic_push\n-\t\t\t && parse_feature_request(feature_list, \"atomic\"))\n+\t\t\t && parse_feature_request(&feature_list, \"atomic\"))\n \t\t\t\tuse_atomic = 1;\n+\n+\t\t\tstring_list_clear(&feature_list, 1);\n \t\t}\n \n \t\tif (!strcmp(line, \"push-cert\")) {\ndiff --git a/connect.c b/connect.c\nindex c0144d8..4295ba1 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -10,8 +10,8 @@\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n \n-static char *server_capabilities;\n-static const char *parse_feature_value(const char *, const char *, int *);\n+struct string_list server_capabilities = STRING_LIST_INIT_DUP;\n+static const char *parse_feature_value(struct string_list *, const char *, int *);\n \n static int check_ref(const char *name, unsigned int flags)\n {\n@@ -80,18 +80,18 @@ reject:\n \n static void annotate_refs_with_symref_info(struct ref *ref)\n {\n+\tstruct string_list_item *item;\n \tstruct string_list symref = STRING_LIST_INIT_DUP;\n-\tconst char *feature_list = server_capabilities;\n \n-\twhile (feature_list) {\n-\t\tint len;\n+\tfor_each_string_list_item(item, &server_capabilities) {\n \t\tconst char *val;\n \n-\t\tval = parse_feature_value(feature_list, \"symref\", &len);\n-\t\tif (!val)\n-\t\t\tbreak;\n-\t\tparse_one_symref_info(&symref, val, len);\n-\t\tfeature_list = val + 1;\n+\t\tif (skip_prefix(item->string, \"symref\", &val)) {\n+\t\t\tif (!val)\n+\t\t\t\tcontinue;\n+\t\t\tval++; /* skip the = */\n+\t\t\tparse_one_symref_info(&symref, val, strlen(val));\n+\t\t}\n \t}\n \tstring_list_sort(&symref);\n \n@@ -153,8 +153,8 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \n \t\tname_len = strlen(name);\n \t\tif (len != name_len + 41) {\n-\t\t\tfree(server_capabilities);\n-\t\t\tserver_capabilities = xstrdup(name + name_len + 1);\n+\t\t\tstring_list_clear(&server_capabilities, 1);\n+\t\t\tfrom_space_separated_string(&server_capabilities, name + name_len + 1);\n \t\t}\n \n \t\tif (extra_have && !strcmp(name, \".have\")) {\n@@ -176,51 +176,40 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \treturn list;\n }\n \n-static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)\n+static const char *parse_feature_value(struct string_list *feature_list, const char *feature, int *lenp)\n {\n-\tint len;\n-\n-\tif (!feature_list)\n-\t\treturn NULL;\n-\n-\tlen = strlen(feature);\n-\twhile (*feature_list) {\n-\t\tconst char *found = strstr(feature_list, feature);\n-\t\tif (!found)\n-\t\t\treturn NULL;\n-\t\tif (feature_list == found || isspace(found[-1])) {\n-\t\t\tconst char *value = found + len;\n-\t\t\t/* feature with no value (e.g., \"thin-pack\") */\n-\t\t\tif (!*value || isspace(*value)) {\n-\t\t\t\tif (lenp)\n-\t\t\t\t\t*lenp = 0;\n-\t\t\t\treturn value;\n-\t\t\t}\n-\t\t\t/* feature with a value (e.g., \"agent=git/1.2.3\") */\n-\t\t\telse if (*value == '=') {\n-\t\t\t\tvalue++;\n-\t\t\t\tif (lenp)\n-\t\t\t\t\t*lenp = strcspn(value, \" \\t\\n\");\n-\t\t\t\treturn value;\n-\t\t\t}\n-\t\t\t/*\n-\t\t\t * otherwise we matched a substring of another feature;\n-\t\t\t * keep looking\n-\t\t\t */\n+\tconst char *value;\n+\tstruct string_list_item *item;\n+\n+\tfor_each_string_list_item(item, feature_list) {\n+\t\tif (!skip_prefix(item->string, feature, &value))\n+\t\t\tcontinue;\n+\n+\t\t/* feature with no value (e.g., \"thin-pack\") */\n+\t\tif (!*value) {\n+\t\t\tif (lenp)\n+\t\t\t\t*lenp = 0;\n+\t\t\treturn value;\n+\t\t}\n+\t\t/* feature with a value (e.g., \"agent=git/1.2.3\") */\n+\t\telse if (*value == '=') {\n+\t\t\tvalue++;\n+\t\t\tif (lenp)\n+\t\t\t\t*lenp = strlen(value);\n+\t\t\treturn value;\n \t\t}\n-\t\tfeature_list = found + 1;\n \t}\n \treturn NULL;\n }\n \n-int parse_feature_request(const char *feature_list, const char *feature)\n+int parse_feature_request(struct string_list *feature_list, const char *feature)\n {\n \treturn !!parse_feature_value(feature_list, feature, NULL);\n }\n \n const char *server_feature_value(const char *feature, int *len)\n {\n-\treturn parse_feature_value(server_capabilities, feature, len);\n+\treturn parse_feature_value(&server_capabilities, feature, len);\n }\n \n int server_supports(const char *feature)\ndiff --git a/connect.h b/connect.h\nindex c41a685..47492e5 100644\n--- a/connect.h\n+++ b/connect.h\n@@ -7,7 +7,7 @@ extern struct child_process *git_connect(int fd[2], const char *url, const char\n extern int finish_connect(struct child_process *conn);\n extern int git_connection_is_socket(struct child_process *conn);\n extern int server_supports(const char *feature);\n-extern int parse_feature_request(const char *features, const char *feature);\n+extern int parse_feature_request(struct string_list *, const char *feature);\n extern const char *server_feature_value(const char *feature, int *len_ret);\n extern int url_is_local_not_ssh(const char *url);\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex 5449ff7..2493964 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -531,7 +531,7 @@ error:\n \t}\n }\n \n-static void parse_features(const char *features)\n+static void parse_features(struct string_list *features)\n {\n \tif (parse_feature_request(features, \"multi_ack_detailed\"))\n \t\tmulti_ack = 2;\n@@ -563,7 +563,10 @@ static void receive_needs(void)\n \tfor (;;) {\n \t\tstruct object *o;\n \t\tunsigned char sha1_buf[20];\n-\t\tchar *line = packet_read_line(0, NULL);\n+\t\tint pkt_len;\n+\t\tstruct string_list list = STRING_LIST_INIT_DUP;\n+\t\tchar *line = packet_read_line(0, &pkt_len);\n+\n \t\treset_timeout();\n \t\tif (!line)\n \t\t\tbreak;\n@@ -596,7 +599,9 @@ static void receive_needs(void)\n \t\t\tdie(\"git upload-pack: protocol error, \"\n \t\t\t \"expected to get sha, not '%s'\", line);\n \n-\t\tparse_features(line + 45);\n+\t\tfrom_space_separated_string(&list, xstrdup(line + 45));\n+\t\tparse_features(&list);\n+\t\tstring_list_clear(&list, 1);\n \n \t\to = parse_object(sha1_buf);\n \t\tif (!o)\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262666","messageId":"1433203338-27493-5-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:06Z","receivedAt":"2015-06-02T00:02:06Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In upload-pack-2 we send each capability in its own packet buffer.\n\nSigned-off-by: Stefan Beller \n---\n\nNotes:\n Moved the capabilities into a struct containing all the capabilities,\n and then we selectively cancel out unwanted capabilities.\n\n .gitignore | 1 +\n Makefile | 2 ++\n upload-pack-2.c | 1 +\n upload-pack.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++----------\n 4 files changed, 86 insertions(+), 18 deletions(-)\n create mode 120000 upload-pack-2.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 422c538..2213af4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -165,6 +165,7 @@\n /git-update-server-info\n /git-upload-archive\n /git-upload-pack\n+/git-upload-pack-2\n /git-var\n /git-verify-commit\n /git-verify-pack\ndiff --git a/Makefile b/Makefile\nindex 323c401..f83f02d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -560,6 +560,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o\n PROGRAM_OBJS += shell.o\n PROGRAM_OBJS += show-index.o\n PROGRAM_OBJS += upload-pack.o\n+PROGRAM_OBJS += upload-pack-2.o\n PROGRAM_OBJS += remote-testsvn.o\n \n # Binary suffix, set to .exe for Windows builds\n@@ -626,6 +627,7 @@ OTHER_PROGRAMS = git$X\n # what test wrappers are needed and 'install' will install, in bindir\n BINDIR_PROGRAMS_NEED_X += git\n BINDIR_PROGRAMS_NEED_X += git-upload-pack\n+BINDIR_PROGRAMS_NEED_X += git-upload-pack-2\n BINDIR_PROGRAMS_NEED_X += git-receive-pack\n BINDIR_PROGRAMS_NEED_X += git-upload-archive\n BINDIR_PROGRAMS_NEED_X += git-shell\ndiff --git a/upload-pack-2.c b/upload-pack-2.c\nnew file mode 120000\nindex 0000000..e30a871\n--- /dev/null\n+++ b/upload-pack-2.c\n@@ -0,0 +1 @@\n+upload-pack.c\n\\ No newline at end of file\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 2493964..7477ca3 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -716,33 +716,69 @@ static void format_symref_info(struct strbuf *buf, struct string_list *symref)\n \t\tstrbuf_addf(buf, \" symref=%s:%s\", item->string, (char *)item->util);\n }\n \n+static int advertise_capabilities = 1;\n+const char *all_capabilities[] = {\n+\t\"multi_ack\",\n+\t\"thin-pack\",\n+\t\"side-band\",\n+\t\"side-band-64k\",\n+\t\"ofs-delta\",\n+\t\"shallow\",\n+\t\"no-progress\",\n+\t\"include-tag\",\n+\t\"multi_ack_detailed\",\n+\t\"allow-tip-sha1-in-want\",\n+\t\"no-done\",\n+};\n+\n+static void send_capabilities(void)\n+{\n+\tint i;\n+\tfor (i = 0; i < ARRAY_SIZE(all_capabilities); i++) {\n+\t\tconst char *cap = all_capabilities[i];\n+\t\tif (!strcmp(cap, \"allow-tip-sha1-in-want\") && !allow_tip_sha1_in_want)\n+\t\t\tcontinue;\n+\t\tif (!strcmp(cap, \"no-done\") && !stateless_rpc)\n+\t\t\tcontinue;\n+\t\tpacket_write(1,\"%s\", cap);\n+\t}\n+\n+\tpacket_write(1, \"agent=%s\\n\", git_user_agent_sanitized());\n+\tpacket_flush(1);\n+}\n+\n static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\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+\n \tconst char *refname_nons = strip_namespace(refname);\n \tunsigned char peeled[20];\n \n \tif (mark_our_ref(refname, sha1))\n \t\treturn 0;\n \n-\tif (capabilities) {\n-\t\tstruct strbuf symref_info = STRBUF_INIT;\n-\n-\t\tformat_symref_info(&symref_info, cb_data);\n-\t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n-\t\t\t sha1_to_hex(sha1), refname_nons,\n-\t\t\t 0, capabilities,\n-\t\t\t allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n-\t\t\t stateless_rpc ? \" no-done\" : \"\",\n-\t\t\t symref_info.buf,\n-\t\t\t git_user_agent_sanitized());\n-\t\tstrbuf_release(&symref_info);\n+\tif (advertise_capabilities) {\n+\t\tint i;\n+\t\tstruct strbuf capabilities = STRBUF_INIT;\n+\n+\t\tfor (i = 0; i < ARRAY_SIZE(all_capabilities); i++) {\n+\t\t\tconst char *cap = all_capabilities[i];\n+\t\t\tif (!strcmp(cap, \"allow-tip-sha1-in-want\") && !allow_tip_sha1_in_want)\n+\t\t\t\tcontinue;\n+\t\t\tif (!strcmp(cap, \"no-done\") && !stateless_rpc)\n+\t\t\t\tcontinue;\n+\t\t\tstrbuf_addf(&capabilities, \" %s\", cap);\n+\t\t}\n+\n+\t\tformat_symref_info(&capabilities, cb_data);\n+\t\tstrbuf_addf(&capabilities, \" agent=%s\", git_user_agent_sanitized());\n+\n+\t\tpacket_write(1, \"%s %s%c%s\\n\", sha1_to_hex(sha1), refname_nons,\n+\t\t\t 0, capabilities.buf);\n+\t\tstrbuf_release(&capabilities);\n+\t\tadvertise_capabilities = 0;\n \t} else {\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), refname_nons);\n \t}\n-\tcapabilities = NULL;\n \tif (!peel_ref(refname, peeled))\n \t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(peeled), refname_nons);\n \treturn 0;\n@@ -792,6 +828,29 @@ static void upload_pack(void)\n \t}\n }\n \n+static void receive_capabilities(void)\n+{\n+\tstruct string_list list = STRING_LIST_INIT_DUP;\n+\tchar *line = packet_read_line(0, NULL);\n+\twhile (line) {\n+\t\tstring_list_append(&list, line);\n+\t\tline = packet_read_line(0, NULL);\n+\t}\n+\tparse_features(&list);\n+\tstring_list_clear(&list, 1);\n+}\n+\n+static void upload_pack_version_2(void)\n+{\n+\tsend_capabilities();\n+\treceive_capabilities();\n+\n+\t/* The rest of the protocol stays the same, capabilities advertising\n+\t is disabled though. */\n+\tadvertise_capabilities = 0;\n+\tupload_pack();\n+}\n+\n static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n@@ -807,13 +866,14 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n int main(int argc, char **argv)\n {\n \tchar *dir;\n+\tconst char *cmd;\n \tint i;\n \tint strict = 0;\n \n \tgit_setup_gettext();\n \n \tpacket_trace_identity(\"upload-pack\");\n-\tgit_extract_argv0_path(argv[0]);\n+\tcmd = git_extract_argv0_path(argv[0]);\n \tcheck_replace_refs = 0;\n \n \tfor (i = 1; i < argc; i++) {\n@@ -855,6 +915,10 @@ int main(int argc, char **argv)\n \t\tdie(\"'%s' does not appear to be a git repository\", dir);\n \n \tgit_config(upload_pack_config, NULL);\n-\tupload_pack();\n+\n+\tif (ends_with(cmd, \"-2\"))\n+\t\tupload_pack_version_2();\n+\telse\n+\t\tupload_pack();\n \treturn 0;\n }\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262671","messageId":"1433203338-27493-6-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 05/16] remote.h: Change get_remote_heads return to void","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:07Z","receivedAt":"2015-06-02T00:02:07Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"No function uses the return value of get_remote_heads, so we don't want\nto confuse readers by it.\n\nSigned-off-by: Stefan Beller \n---\n connect.c | 10 ++++------\n remote.h | 8 ++++----\n 2 files changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 4295ba1..a2c777e 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -108,10 +108,10 @@ static void annotate_refs_with_symref_info(struct ref *ref)\n /*\n * Read all the refs from the other end\n */\n-struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n-\t\t\t struct ref **list, unsigned int flags,\n-\t\t\t struct sha1_array *extra_have,\n-\t\t\t struct sha1_array *shallow_points)\n+void get_remote_heads(int in, char *src_buf, size_t src_len,\n+\t\t struct ref **list, unsigned int flags,\n+\t\t struct sha1_array *extra_have,\n+\t\t struct sha1_array *shallow_points)\n {\n \tstruct ref **orig_list = list;\n \tint got_at_least_one_head = 0;\n@@ -172,8 +172,6 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t}\n \n \tannotate_refs_with_symref_info(*orig_list);\n-\n-\treturn list;\n }\n \n static const char *parse_feature_value(struct string_list *feature_list, const char *feature, int *lenp)\ndiff --git a/remote.h b/remote.h\nindex 02d66ce..d5242b0 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -144,10 +144,10 @@ int check_ref_type(const struct ref *ref, int flags);\n void free_refs(struct ref *ref);\n \n struct sha1_array;\n-extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n-\t\t\t\t struct ref **list, unsigned int flags,\n-\t\t\t\t struct sha1_array *extra_have,\n-\t\t\t\t struct sha1_array *shallow);\n+extern void get_remote_heads(int in, char *src_buf, size_t src_len,\n+\t\t\t struct ref **list, unsigned int flags,\n+\t\t\t struct sha1_array *extra_have,\n+\t\t\t struct sha1_array *shallow);\n \n int resolve_remote_symref(struct ref *ref, struct ref *list);\n int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262677","messageId":"1433203338-27493-7-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 06/16] remote.h: add new struct for options","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:08Z","receivedAt":"2015-06-02T00:02:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n remote.h | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/remote.h b/remote.h\nindex d5242b0..16cacfe 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -56,6 +56,20 @@ struct remote {\n \tchar *http_proxy;\n };\n \n+/* todo: discuss if this should be merged with\n+ * git_transport_options in transport.c */\n+struct transport_options {\n+\tunsigned multi_ack : 2;\n+\tunsigned no_done : 1;\n+\tunsigned use_thin_pack : 1;\n+\tunsigned no_progress : 1;\n+\tunsigned include_tag : 1;\n+\tunsigned prefer_ofs_delta : 1;\n+\tunsigned agent_supported : 1;\n+\tunsigned allow_tip_sha1_in_want : 1;\n+\tunsigned use_sideband;\n+};\n+\n struct remote *remote_get(const char *name);\n struct remote *pushremote_get(const char *name);\n int remote_is_configured(const char *name);\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262664","messageId":"1433203338-27493-8-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 07/16] transport: add infrastructure to support a protocol version number","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:09Z","receivedAt":"2015-06-02T00:02:09Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n\nNotes:\n This patch has been split up into 2 parts.\n This first part only introduces the infrastructure\n without exposing it to the user at all (no command line\n option nor a repository configuration option).\n \n The exposure to the user will be added in a later second\n patch. This allows us some time in between to build all\n the features we want without the requirement to be\n functional.\n\n remote.h | 2 ++\n transport-helper.c | 1 +\n transport.c | 15 +++++++++++++++\n transport.h | 5 +++++\n 4 files changed, 23 insertions(+)\n\ndiff --git a/remote.h b/remote.h\nindex 16cacfe..3767bed 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -50,6 +50,8 @@ struct remote {\n \tconst char *receivepack;\n \tconst char *uploadpack;\n \n+\tint transport_version;\n+\n \t/*\n \t * for curl remotes only\n \t */\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 5d99a6b..ab3cd5b 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -247,6 +247,7 @@ static int disconnect_helper(struct transport *transport)\n }\n \n static const char *unsupported_options[] = {\n+\tTRANS_OPT_TRANSPORTVERSION,\n \tTRANS_OPT_UPLOADPACK,\n \tTRANS_OPT_RECEIVEPACK,\n \tTRANS_OPT_THIN,\ndiff --git a/transport.c b/transport.c\nindex f080e93..651f0ac 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -479,6 +479,16 @@ static int set_git_option(struct git_transport_options *opts,\n \t} else if (!strcmp(name, TRANS_OPT_PUSH_CERT)) {\n \t\topts->push_cert = !!value;\n \t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_TRANSPORTVERSION)) {\n+\t\tif (!value)\n+\t\t\topts->transport_version = DEFAULT_TRANSPORT_VERSION;\n+\t\telse {\n+\t\t\tchar *end;\n+\t\t\topts->transport_version = strtol(value, &end, 0);\n+\t\t\tif (*end)\n+\t\t\t\tdie(\"transport: invalid transport version option '%s'\", value);\n+\t\t}\n+\t\treturn 0;\n \t}\n \treturn 1;\n }\n@@ -970,6 +980,8 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->connect = connect_git;\n \t\tret->disconnect = disconnect_git;\n \t\tret->smart_options = &(data->options);\n+\t\tret->smart_options->transport_version =\n+\t\t\tDEFAULT_TRANSPORT_VERSION;\n \n \t\tdata->conn = NULL;\n \t\tdata->got_remote_heads = 0;\n@@ -988,6 +1000,9 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->smart_options->receivepack = \"git-receive-pack\";\n \t\tif (remote->receivepack)\n \t\t\tret->smart_options->receivepack = remote->receivepack;\n+\t\tif (remote->transport_version)\n+\t\t\tret->smart_options->transport_version =\n+\t\t\t\tremote->transport_version;\n \t}\n \n \treturn ret;\ndiff --git a/transport.h b/transport.h\nindex 18d2cf8..6095d7a 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -14,6 +14,7 @@ struct git_transport_options {\n \tunsigned update_shallow : 1;\n \tunsigned push_cert : 1;\n \tint depth;\n+\tint transport_version;\n \tconst char *uploadpack;\n \tconst char *receivepack;\n \tstruct push_cas_option *cas;\n@@ -162,6 +163,10 @@ struct transport *transport_get(struct remote *, const char *);\n /* Send push certificates */\n #define TRANS_OPT_PUSH_CERT \"pushcert\"\n \n+/* Use a new version of the git protocol */\n+#define TRANS_OPT_TRANSPORTVERSION \"transportversion\"\n+#define DEFAULT_TRANSPORT_VERSION 1\n+\n /**\n * Returns 0 if the option was used, non-zero otherwise. Prints a\n * message to stderr if the option is not used.\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262668","messageId":"1433203338-27493-9-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 08/16] transport: select transport version via command line or config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:10Z","receivedAt":"2015-06-02T00:02:10Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The transport version set via command line argument in\ngit fetch takes precedence over the configured version.\n\nSigned-off-by: Stefan Beller \n---\n\nNotes:\n removed -y as the short --transport-version\n \n This patch has been split up and is the second part\n carrying only the exposure to the user.\n\n builtin/fetch.c | 6 ++++++\n remote.c | 2 ++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 7910419..a558563 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -46,6 +46,7 @@ static const char *recurse_submodules_default;\n static int shown_url = 0;\n static int refmap_alloc, refmap_nr;\n static const char **refmap_array;\n+static const char *transport_version;\n \n static int option_parse_recurse_submodules(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -121,6 +122,9 @@ static struct option builtin_fetch_options[] = {\n \t\t N_(\"default mode for recursion\"), PARSE_OPT_HIDDEN },\n \tOPT_BOOL(0, \"update-shallow\", &update_shallow,\n \t\t N_(\"accept refs that update .git/shallow\")),\n+\tOPT_STRING(0, \"transport-version\", &transport_version,\n+\t\t N_(\"transport-version\"),\n+\t\t N_(\"specify transport version to be used\")),\n \t{ OPTION_CALLBACK, 0, \"refmap\", NULL, N_(\"refmap\"),\n \t N_(\"specify fetch refmap\"), PARSE_OPT_NONEG, parse_refmap_arg },\n \tOPT_END()\n@@ -848,6 +852,8 @@ static struct transport *prepare_transport(struct remote *remote)\n \tstruct transport *transport;\n \ttransport = transport_get(remote, NULL);\n \ttransport_set_verbosity(transport, verbosity, progress);\n+\tif (transport_version)\n+\t\tset_option(transport, TRANS_OPT_TRANSPORTVERSION, transport_version);\n \tif (upload_pack)\n \t\tset_option(transport, TRANS_OPT_UPLOADPACK, upload_pack);\n \tif (keep)\ndiff --git a/remote.c b/remote.c\nindex 68901b0..2914d9d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -476,6 +476,8 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\t\t\t\t key, value);\n \t} else if (!strcmp(subkey, \".vcs\")) {\n \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n+\t} else if (!strcmp(subkey, \".transportversion\")) {\n+\t\tremote->transport_version = git_config_int(key, value);\n \t}\n \treturn 0;\n }\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262678","messageId":"1433203338-27493-10-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 09/16] remote.h: add get_remote_capabilities, request_capabilities","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:11Z","receivedAt":"2015-06-02T00:02:11Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Instead of calling get_remote_heads as a first command during the\nprotocol exchange, we need to have fine grained control over the\ncapability negotiation in version 2 of the protocol.\n\nIntroduce get_remote_capabilities, which will just listen to\ncapabilities of the remote and request_capabilities which will\ntell the selection of capabilities to the remote.\n\nSigned-off-by: Stefan Beller \n---\n connect.c | 37 +++++++++++++++++++++++++++++++++++++\n remote.h | 3 +++\n 2 files changed, 40 insertions(+)\n\ndiff --git a/connect.c b/connect.c\nindex a2c777e..4ebe1dc 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -105,6 +105,43 @@ static void annotate_refs_with_symref_info(struct ref *ref)\n \tstring_list_clear(&symref, 0);\n }\n \n+void get_remote_capabilities(int in, char *src_buf, size_t src_len)\n+{\n+\tstring_list_clear(&server_capabilities, 1);\n+\tfor (;;) {\n+\t\tint len;\n+\t\tchar *line = packet_buffer;\n+\n+\t\tlen = packet_read(in, &src_buf, &src_len,\n+\t\t\t\t packet_buffer, sizeof(packet_buffer),\n+\t\t\t\t PACKET_READ_GENTLE_ON_EOF |\n+\t\t\t\t PACKET_READ_CHOMP_NEWLINE);\n+\t\tif (len < 0)\n+\t\t\tdie_initial_contact(0);\n+\n+\t\tif (!len)\n+\t\t\tbreak;\n+\n+\t\tstring_list_append(&server_capabilities, line);\n+\t}\n+}\n+\n+int request_capabilities(int out, struct transport_options *options)\n+{\n+\tif (options->multi_ack == 2) packet_write(out, \"multi_ack_detailed\");\n+\tif (options->multi_ack == 1) packet_write(out, \"multi_ack\");\n+\tif (options->no_done) packet_write(out, \"no-done\");\n+\tif (options->use_sideband == 2) packet_write(out, \"side-band-64k\");\n+\tif (options->use_sideband == 1) packet_write(out, \"side-band\");\n+\tif (options->use_thin_pack) packet_write(out, \"thin-pack\");\n+\tif (options->no_progress) packet_write(out, \"no-progress\");\n+\tif (options->include_tag) packet_write(out, \"include-tag\");\n+\tif (options->prefer_ofs_delta) packet_write(out, \"ofs-delta\");\n+\tif (options->agent_supported) packet_write(out, \"agent=%s\",\n+\t\t\t\t\t\t git_user_agent_sanitized());\n+\tpacket_flush(out);\n+}\n+\n /*\n * Read all the refs from the other end\n */\ndiff --git a/remote.h b/remote.h\nindex 3767bed..61619c5 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -165,6 +165,9 @@ extern void get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\t\t struct sha1_array *extra_have,\n \t\t\t struct sha1_array *shallow);\n \n+void get_remote_capabilities(int in, char *src_buf, size_t src_len);\n+int request_capabilities(int out, struct transport_options*);\n+\n int resolve_remote_symref(struct ref *ref, struct ref *list);\n int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);\n \n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262672","messageId":"1433203338-27493-11-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 10/16] transport: connect_setup appends protocol version number","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:12Z","receivedAt":"2015-06-02T00:02:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n\nNotes:\n name it to_free\n\n transport.c | 17 +++++++++++++++--\n 1 file changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 651f0ac..b49fc60 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -496,15 +496,28 @@ static int set_git_option(struct git_transport_options *opts,\n static int connect_setup(struct transport *transport, int for_push, int verbose)\n {\n \tstruct git_transport_data *data = transport->data;\n+\tconst char *remote_program;\n+\tchar *to_free = 0;\n \n \tif (data->conn)\n \t\treturn 0;\n \n+\tremote_program = (for_push ? data->options.receivepack\n+\t\t\t\t : data->options.uploadpack);\n+\n+\tif (transport->smart_options->transport_version >= 2) {\n+\t\tto_free = xmalloc(strlen(remote_program) + 12);\n+\t\tsprintf(to_free, \"%s-%d\", remote_program,\n+\t\t\ttransport->smart_options->transport_version);\n+\t\tremote_program = to_free;\n+\t}\n+\n \tdata->conn = git_connect(data->fd, transport->url,\n-\t\t\t\t for_push ? data->options.receivepack :\n-\t\t\t\t data->options.uploadpack,\n+\t\t\t\t remote_program,\n \t\t\t\t verbose ? CONNECT_VERBOSE : 0);\n \n+\tfree(to_free);\n+\n \treturn 0;\n }\n \n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262669","messageId":"1433203338-27493-12-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 11/16] remote: have preselect_capabilities","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:13Z","receivedAt":"2015-06-02T00:02:13Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n connect.c | 28 ++++++++++++++++++++++++++++\n remote.h | 1 +\n 2 files changed, 29 insertions(+)\n\ndiff --git a/connect.c b/connect.c\nindex 4ebe1dc..752b9a5 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -126,6 +126,34 @@ void get_remote_capabilities(int in, char *src_buf, size_t src_len)\n \t}\n }\n \n+/* just select all options the server advertised. */\n+void preselect_capabilities(struct transport_options *options)\n+{\n+\tif (is_repository_shallow() && !server_supports(\"shallow\"))\n+\t\tdie(\"Server does not support shallow clients\");\n+\n+\tif (server_supports(\"multi_ack\"))\n+\t\toptions->multi_ack = 1;\n+\telse if (server_supports(\"multi_ack_detailed\"))\n+\t\toptions->multi_ack = 2;\n+\n+\tif (server_supports(\"side-band\"))\n+\t\toptions->use_sideband = 1;\n+\telse if (server_supports(\"side-band-64k\"))\n+\t\toptions->use_sideband = 2;\n+\n+\tif (server_supports(\"no-done\"))\n+\t\toptions->no_done = 1;\n+\tif (server_supports(\"thin-pack\"))\n+\t\toptions->use_thin_pack = 1;\n+\tif (server_supports(\"no-progress\"))\n+\t\toptions->no_progress = 1;\n+\tif (server_supports(\"include-tag\"))\n+\t\toptions->include_tag = 1;\n+\tif (server_supports(\"ofs-delta\"))\n+\t\toptions->prefer_ofs_delta = 1;\n+}\n+\n int request_capabilities(int out, struct transport_options *options)\n {\n \tif (options->multi_ack == 2) packet_write(out, \"multi_ack_detailed\");\ndiff --git a/remote.h b/remote.h\nindex 61619c5..264a513 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -166,6 +166,7 @@ extern void get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\t\t struct sha1_array *shallow);\n \n void get_remote_capabilities(int in, char *src_buf, size_t src_len);\n+void preselect_capabilities(struct transport_options *options);\n int request_capabilities(int out, struct transport_options*);\n \n int resolve_remote_symref(struct ref *ref, struct ref *list);\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262679","messageId":"1433203338-27493-13-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 12/16] transport: get_refs_via_connect exchanges capabilities before refs.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:14Z","receivedAt":"2015-06-02T00:02:14Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n\nNotes:\n A minor issue I am unsure about here is the\n line\n \t&& transport->smart_options->transport_version)\n which could be prevented if we always set the transport_version\n in make_remote to be the default remote version.\n \n The advantage of having it always set in make_remote would\n be a cleaner mind model (the version set is always accurate)\n as opposed to now (version may be 0, then the default\n applies as we don't care enough to set a version)\n \n However I think the code may be more ugly if we were to\n always set the version in make_remote as then we would need\n to move the DEFAULT_TRANSPORT_VERSION define into remote.h\n or somewhere else (transport.h is not included in remote.c,\n I guess that's on purpose?)\n\n transport.c | 28 ++++++++++++++++++++++++----\n transport.h | 6 ++++++\n 2 files changed, 30 insertions(+), 4 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex b49fc60..ba40677 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -523,14 +523,33 @@ static int connect_setup(struct transport *transport, int for_push, int verbose)\n \n static struct ref *get_refs_via_connect(struct transport *transport, int for_push)\n {\n+\tstruct transport_options options;\n \tstruct git_transport_data *data = transport->data;\n \tstruct ref *refs;\n+\tint version = DEFAULT_TRANSPORT_VERSION;\n \n+\tif (transport->smart_options\n+\t && transport->smart_options->transport_version)\n+\t\tversion = transport->smart_options->transport_version;\n \tconnect_setup(transport, for_push, 0);\n-\tget_remote_heads(data->fd[0], NULL, 0, &refs,\n-\t\t\t for_push ? REF_NORMAL : 0,\n-\t\t\t &data->extra_have,\n-\t\t\t &data->shallow);\n+\tswitch (version) {\n+\tcase 2: /* first talk about capabilities, then get the heads */\n+\t\tget_remote_capabilities(data->fd[0], NULL, 0);\n+\t\tpreselect_capabilities(&options);\n+\t\tif (transport->select_capabilities)\n+\t\t\ttransport->select_capabilities(&options);\n+\t\trequest_capabilities(data->fd[1], &options);\n+\t\t/* fall through */\n+\tcase 1:\n+\t\tget_remote_heads(data->fd[0], NULL, 0, &refs,\n+\t\t\t\t for_push ? REF_NORMAL : 0,\n+\t\t\t\t &data->extra_have,\n+\t\t\t\t &data->shallow);\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"BUG: Transport version %d not supported\", version);\n+\t\tbreak;\n+\t}\n \tdata->got_remote_heads = 1;\n \n \treturn refs;\n@@ -987,6 +1006,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tstruct git_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n \t\tret->set_option = NULL;\n+\t\tret->select_capabilities = NULL;\n \t\tret->get_refs_list = get_refs_via_connect;\n \t\tret->fetch = fetch_refs_via_pack;\n \t\tret->push_refs = git_transport_push;\ndiff --git a/transport.h b/transport.h\nindex 6095d7a..3e63efc 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -74,6 +74,12 @@ struct transport {\n \tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n \n \t/**\n+\t * A callback to select protocol options. Must be set if\n+\t * the caller wants to change transport options.\n+\t */\n+\tvoid (*select_capabilities)(struct transport_options *);\n+\n+\t/**\n \t * Push the objects and refs. Send the necessary objects, and\n \t * then, for any refs where peer_ref is set and\n \t * peer_ref->new_sha1 is different from old_sha1, tell the\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262673","messageId":"1433203338-27493-14-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 13/16] fetch-pack: use the configured transport protocol","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:15Z","receivedAt":"2015-06-02T00:02:15Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n builtin/fetch-pack.c | 22 ++++++++++-\n fetch-pack.c | 109 +++++++++++++++++++++++++++------------------------\n fetch-pack.h | 1 +\n 3 files changed, 80 insertions(+), 52 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 4a6b340..be5c43e 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -48,6 +48,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tstruct child_process *conn;\n \tstruct fetch_pack_args args;\n \tstruct sha1_array shallow = SHA1_ARRAY_INIT;\n+\tstruct transport_options select_options;\n \n \tpacket_trace_identity(\"fetch-pack\");\n \n@@ -127,6 +128,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\targs.update_shallow = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!skip_prefix(arg, \"--transport-version\", &arg)) {\n+\t\t\targs.version = strtol(arg, NULL, 0);\n+\t\t\tcontinue;\n+\t\t}\n \t\tusage(fetch_pack_usage);\n \t}\n \n@@ -175,7 +180,22 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\tif (!conn)\n \t\t\treturn args.diag_url ? 0 : 1;\n \t}\n-\tget_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);\n+\tif (args.version == 0)\n+\t\targs.version = DEFAULT_TRANSPORT_VERSION;\n+\n+\tswitch (args.version) {\n+\tcase 2:\n+\n+\t\tget_remote_capabilities(fd[0], NULL, 0);\n+\t\tselect_capabilities(args, &select_options);\n+\t\trequest_capabilities(fd[1], &select_options);\n+\t\t/* fall through */\n+\tcase 1:\n+\t\tget_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"Transport version %d not supported\", args.version);\n+\t}\n \n \tref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,\n \t\t\t &shallow, pack_lockfile_ptr);\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 48526aa..b617128 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -23,7 +23,6 @@ static int prefer_ofs_delta = 1;\n static int no_done;\n static int fetch_fsck_objects = -1;\n static int transfer_fsck_objects = -1;\n-static int agent_supported;\n static struct lock_file shallow_lock;\n static const char *alternate_shallow_file;\n \n@@ -43,7 +42,8 @@ static int marked;\n #define MAX_IN_VAIN 256\n \n static struct prio_queue rev_list = { compare_commits_by_commit_date };\n-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n+static int non_common_revs;\n+static struct transport_options select_options;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -257,7 +257,7 @@ static int find_common(struct fetch_pack_args *args,\n \tstruct strbuf req_buf = STRBUF_INIT;\n \tsize_t state_len = 0;\n \n-\tif (args->stateless_rpc && multi_ack == 1)\n+\tif (args->stateless_rpc && select_options.multi_ack == 1)\n \t\tdie(\"--stateless-rpc requires multi_ack_detailed\");\n \tif (marked)\n \t\tfor_each_ref(clear_marks, NULL);\n@@ -290,17 +290,17 @@ static int find_common(struct fetch_pack_args *args,\n \t\tremote_hex = sha1_to_hex(remote);\n \t\tif (!fetching) {\n \t\t\tstruct strbuf c = STRBUF_INIT;\n-\t\t\tif (multi_ack == 2) strbuf_addstr(&c, \" multi_ack_detailed\");\n-\t\t\tif (multi_ack == 1) strbuf_addstr(&c, \" multi_ack\");\n-\t\t\tif (no_done) strbuf_addstr(&c, \" no-done\");\n-\t\t\tif (use_sideband == 2) strbuf_addstr(&c, \" side-band-64k\");\n-\t\t\tif (use_sideband == 1) strbuf_addstr(&c, \" side-band\");\n-\t\t\tif (args->use_thin_pack) strbuf_addstr(&c, \" thin-pack\");\n-\t\t\tif (args->no_progress) strbuf_addstr(&c, \" no-progress\");\n-\t\t\tif (args->include_tag) strbuf_addstr(&c, \" include-tag\");\n-\t\t\tif (prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n-\t\t\tif (agent_supported) strbuf_addf(&c, \" agent=%s\",\n-\t\t\t\t\t\t\t git_user_agent_sanitized());\n+\t\t\tif (select_options.multi_ack == 2) strbuf_addstr(&c, \" multi_ack_detailed\");\n+\t\t\tif (select_options.multi_ack == 1) strbuf_addstr(&c, \" multi_ack\");\n+\t\t\tif (select_options.no_done) strbuf_addstr(&c, \" no-done\");\n+\t\t\tif (select_options.use_sideband == 2) strbuf_addstr(&c, \" side-band-64k\");\n+\t\t\tif (select_options.use_sideband == 1) strbuf_addstr(&c, \" side-band\");\n+\t\t\tif (select_options.use_thin_pack) strbuf_addstr(&c, \" thin-pack\");\n+\t\t\tif (select_options.no_progress) strbuf_addstr(&c, \" no-progress\");\n+\t\t\tif (select_options.include_tag) strbuf_addstr(&c, \" include-tag\");\n+\t\t\tif (select_options.prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n+\t\t\tif (select_options.agent_supported) strbuf_addf(&c, \" agent=%s\",\n+\t\t\t\t\t\t\t\t\t git_user_agent_sanitized());\n \t\t\tpacket_buf_write(&req_buf, \"want %s%s\\n\", remote_hex, c.buf);\n \t\t\tstrbuf_release(&c);\n \t\t} else\n@@ -391,7 +391,7 @@ static int find_common(struct fetch_pack_args *args,\n \t\t\t\tswitch (ack) {\n \t\t\t\tcase ACK:\n \t\t\t\t\tflushes = 0;\n-\t\t\t\t\tmulti_ack = 0;\n+\t\t\t\t\tselect_options.multi_ack = 0;\n \t\t\t\t\tretval = 0;\n \t\t\t\t\tgoto done;\n \t\t\t\tcase ACK_common:\n@@ -440,14 +440,14 @@ done:\n \tif (args->verbose)\n \t\tfprintf(stderr, \"done\\n\");\n \tif (retval != 0) {\n-\t\tmulti_ack = 0;\n+\t\tselect_options.multi_ack = 0;\n \t\tflushes++;\n \t}\n \tstrbuf_release(&req_buf);\n \n \tif (!got_ready || !no_done)\n \t\tconsume_shallow_list(args, fd[0]);\n-\twhile (flushes || multi_ack) {\n+\twhile (flushes || select_options.multi_ack) {\n \t\tint ack = get_ack(fd[0], result_sha1);\n \t\tif (ack) {\n \t\t\tif (args->verbose)\n@@ -455,7 +455,7 @@ done:\n \t\t\t\t\tsha1_to_hex(result_sha1));\n \t\t\tif (ack == ACK)\n \t\t\t\treturn 0;\n-\t\t\tmulti_ack = 1;\n+\t\t\tselect_options.multi_ack = 1;\n \t\t\tcontinue;\n \t\t}\n \t\tflushes--;\n@@ -542,7 +542,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif (allow_tip_sha1_in_want) {\n+\tif (select_options.allow_tip_sha1_in_want) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -671,7 +671,7 @@ static int get_pack(struct fetch_pack_args *args,\n \tint ret;\n \n \tmemset(&demux, 0, sizeof(demux));\n-\tif (use_sideband) {\n+\tif (select_options.use_sideband) {\n \t\t/* xd[] is talking with upload-pack; subprocess reads from\n \t\t * xd[0], spits out band#2 to stderr, and feeds us band#1\n \t\t * through demux->out.\n@@ -752,7 +752,7 @@ static int get_pack(struct fetch_pack_args *args,\n \t\tclose(cmd.out);\n \t}\n \n-\tif (!use_sideband)\n+\tif (!select_options.use_sideband)\n \t\t/* Closed by start_command() */\n \t\txd[0] = -1;\n \n@@ -763,84 +763,91 @@ static int get_pack(struct fetch_pack_args *args,\n \t\t\tret == 0;\n \telse\n \t\tdie(\"%s failed\", cmd_name);\n-\tif (use_sideband && finish_async(&demux))\n+\tif (select_options.use_sideband && finish_async(&demux))\n \t\tdie(\"error in sideband demultiplexer\");\n \treturn 0;\n }\n \n-static int cmp_ref_by_name(const void *a_, const void *b_)\n-{\n-\tconst struct ref *a = *((const struct ref **)a_);\n-\tconst struct ref *b = *((const struct ref **)b_);\n-\treturn strcmp(a->name, b->name);\n-}\n-\n-static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n-\t\t\t\t int fd[2],\n-\t\t\t\t const struct ref *orig_ref,\n-\t\t\t\t struct ref **sought, int nr_sought,\n-\t\t\t\t struct shallow_info *si,\n-\t\t\t\t char **pack_lockfile)\n+void select_capabilities(struct fetch_pack_args *args, struct transport_options *options)\n {\n-\tstruct ref *ref = copy_ref_list(orig_ref);\n-\tunsigned char sha1[20];\n \tconst char *agent_feature;\n \tint agent_len;\n \n-\tsort_ref_list(&ref, ref_compare_name);\n-\tqsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);\n-\n \tif (is_repository_shallow() && !server_supports(\"shallow\"))\n \t\tdie(\"Server does not support shallow clients\");\n \tif (server_supports(\"multi_ack_detailed\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports multi_ack_detailed\\n\");\n-\t\tmulti_ack = 2;\n+\t\toptions->multi_ack = 2;\n \t\tif (server_supports(\"no-done\")) {\n \t\t\tif (args->verbose)\n \t\t\t\tfprintf(stderr, \"Server supports no-done\\n\");\n \t\t\tif (args->stateless_rpc)\n-\t\t\t\tno_done = 1;\n+\t\t\t\toptions->no_done = 1;\n \t\t}\n \t}\n \telse if (server_supports(\"multi_ack\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports multi_ack\\n\");\n-\t\tmulti_ack = 1;\n+\t\toptions->multi_ack = 1;\n \t}\n \tif (server_supports(\"side-band-64k\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports side-band-64k\\n\");\n-\t\tuse_sideband = 2;\n+\t\toptions->use_sideband = 2;\n \t}\n \telse if (server_supports(\"side-band\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports side-band\\n\");\n-\t\tuse_sideband = 1;\n+\t\toptions->use_sideband = 1;\n \t}\n \tif (server_supports(\"allow-tip-sha1-in-want\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n-\t\tallow_tip_sha1_in_want = 1;\n+\t\toptions->allow_tip_sha1_in_want = 1;\n \t}\n \tif (!server_supports(\"thin-pack\"))\n-\t\targs->use_thin_pack = 0;\n+\t\toptions->use_thin_pack = 0;\n \tif (!server_supports(\"no-progress\"))\n-\t\targs->no_progress = 0;\n+\t\toptions->no_progress = 0;\n \tif (!server_supports(\"include-tag\"))\n-\t\targs->include_tag = 0;\n+\t\toptions->include_tag = 0;\n \tif (server_supports(\"ofs-delta\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports ofs-delta\\n\");\n+\t\toptions->prefer_ofs_delta = 1;\n \t} else\n-\t\tprefer_ofs_delta = 0;\n+\t\toptions->prefer_ofs_delta = 0;\n \n \tif ((agent_feature = server_feature_value(\"agent\", &agent_len))) {\n-\t\tagent_supported = 1;\n+\t\toptions->agent_supported = 1;\n \t\tif (args->verbose && agent_len)\n \t\t\tfprintf(stderr, \"Server version is %.*s\\n\",\n \t\t\t\tagent_len, agent_feature);\n \t}\n+}\n+\n+static int cmp_ref_by_name(const void *a_, const void *b_)\n+{\n+\tconst struct ref *a = *((const struct ref **)a_);\n+\tconst struct ref *b = *((const struct ref **)b_);\n+\treturn strcmp(a->name, b->name);\n+}\n+\n+static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n+\t\t\t\t int fd[2],\n+\t\t\t\t const struct ref *orig_ref,\n+\t\t\t\t struct ref **sought, int nr_sought,\n+\t\t\t\t struct shallow_info *si,\n+\t\t\t\t char **pack_lockfile)\n+{\n+\tstruct ref *ref = copy_ref_list(orig_ref);\n+\tunsigned char sha1[20];\n+\n+\tsort_ref_list(&ref, ref_compare_name);\n+\tqsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);\n+\n+\tselect_capabilities(args, &select_options);\n \n \tif (everything_local(args, &ref, sought, nr_sought)) {\n \t\tpacket_flush(fd[1]);\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex bb7fd76..b48b4f5 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -10,6 +10,7 @@ struct fetch_pack_args {\n \tconst char *uploadpack;\n \tint unpacklimit;\n \tint depth;\n+\tint version;\n \tunsigned quiet:1;\n \tunsigned keep_pack:1;\n \tunsigned lock_pack:1;\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262676","messageId":"1433203338-27493-15-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 14/16] t5544: add a test case for the new protocol","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:16Z","receivedAt":"2015-06-02T00:02:16Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n t/t5544-fetch-2.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n create mode 100755 t/t5544-fetch-2.sh\n\ndiff --git a/t/t5544-fetch-2.sh b/t/t5544-fetch-2.sh\nnew file mode 100755\nindex 0000000..beee46c\n--- /dev/null\n+++ b/t/t5544-fetch-2.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2015 Stefan Beller\n+#\n+\n+test_description='Testing version 2 of the fetch protocol'\n+\n+. ./test-lib.sh\n+\n+mk_repo_pair () {\n+\trm -rf client server &&\n+\ttest_create_repo client &&\n+\ttest_create_repo server &&\n+\t(\n+\t\tcd server &&\n+\t\tgit config receive.denyCurrentBranch warn\n+\t) &&\n+\t(\n+\t\tcd client &&\n+\t\tgit remote add origin ../server\n+\t\tgit config remote.origin.transportversion 2\n+\t)\n+}\n+\n+test_expect_success 'setup' '\n+\tmk_repo_pair &&\n+\t(\n+\t\tcd server &&\n+\t\ttest_commit one\n+\t) &&\n+\t(\n+\t\tcd client &&\n+\t\tgit fetch origin master\n+\t)\n+'\n+\n+# More to come here, similar to t5515 having a sub directory full of expected\n+# data going over the wire.\n+\n+test_done\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262665","messageId":"1433203338-27493-16-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 15/16] Documentation/technical/pack-protocol: Mention http as possible protocol","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:17Z","receivedAt":"2015-06-02T00:02:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n\nThis may go unrelated to this series as well.\n\n Documentation/technical/pack-protocol.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt\nindex fc09c63..4064fc7 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -1,11 +1,11 @@\n Packfile transfer protocols\n ===========================\n \n-Git supports transferring data in packfiles over the ssh://, git:// and\n+Git supports transferring data in packfiles over the ssh://, git://, http:// and\n file:// transports. There exist two sets of protocols, one for pushing\n data from a client to a server and another for fetching data from a\n-server to a client. All three transports (ssh, git, file) use the same\n-protocol to transfer data.\n+server to a client. The three transports (ssh, git, file) use the same\n+protocol to transfer data. http is documented in http-protocol.txt.\n \n The processes invoked in the canonical Git implementation are 'upload-pack'\n on the server side and 'fetch-pack' on the client side for fetching data;\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262675","messageId":"1433203338-27493-17-git-send-email-sbeller@google.com","threadId":"39489","inReplyTo":"1433203338-27493-1-git-send-email-sbeller@google.com","subject":"[RFCv2 16/16] Document protocol version 2","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T00:02:18Z","receivedAt":"2015-06-02T00:02:18Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n Documentation/technical/pack-protocol.txt | 79 +++++++++++++++++++++--\n Documentation/technical/protocol-capabilities.txt | 15 -----\n 2 files changed, 74 insertions(+), 20 deletions(-)\n\ndiff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt\nindex 4064fc7..88bb30a 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -14,6 +14,12 @@ data. The protocol functions to have a server tell a client what is\n currently on the server, then for the two to negotiate the smallest amount\n of data to send in order to fully update one or the other.\n \n+\"upload-pack-2\" and \"receive-pack-2\" are the next generation of\n+\"upload-pack\" and \"receive-pack\" respectively. The first two are\n+referred as \"version 2\" in this document and pack-capabilities.txt\n+while the last two are \"version 1\". Unless stated otherwise, \"version 1\"\n+is implied.\n+\n Transports\n ----------\n There are three transports over which the packfile protocol is\n@@ -42,7 +48,8 @@ hostname parameter, terminated by a NUL byte.\n \n --\n git-proto-request = request-command SP pathname NUL [ host-parameter NUL ]\n- request-command = \"git-upload-pack\" / \"git-receive-pack\" /\n+ request-command = \"git-upload-pack\" / \"git-upload-pack-2\" /\n+\t\t \"git-receive-pack\" / \"git-receive-pack-2\" /\n \t\t \"git-upload-archive\" ; case sensitive\n pathname = *( %x01-ff ) ; exclude NUL\n host-parameter = \"host=\" hostname [ \":\" port ]\n@@ -124,6 +131,49 @@ has, the first can 'fetch' from the second. This operation determines\n what data the server has that the client does not then streams that\n data down to the client in packfile format.\n \n+Capability discovery (v2)\n+-------------------------\n+\n+In version 1, capability discovery is part of reference discovery and\n+covered in the reference discovery section.\n+\n+In version 2, when the client initially connects, the server\n+immediately sends its capabilities to the client followed by a flush.\n+Then the client must send the list of server capabilities it wants to\n+use to the server.\n+\n+ S: 00XXlang\\n\n+ S: 00XXthin-pack\\n\n+ S: 00XXofs-delta\\n\n+ S: 00XXagent:agent=git/2:3.4.5+custom-739-gb850f98\\n\n+ S: 0000\n+\n+ C: 00XXthin-pack\\n\n+ C: 00XXofs-delta\\n\n+ C: 00XXlang=en\\n\n+ C: 00XXagent:agent=git/custom_string\\n\n+ C: 0000\n+\n+----\n+ capability-list = *(capability) flush-pkt\n+ capability = PKT-LINE(keyvaluepair LF)\n+ keyvaluepair = key [\"=\" value]\n+ key = 1*(LC_ALPHA / DIGIT / \"-\" / \"_\")\n+ LC_ALPHA = %x61-7A\n+ value = any octet\n+\n+----\n+\n+The client MUST ignore any data on pkt-lines with unknown keys.\n+\n+The client MUST NOT ask for capabilities the server did not say it\n+supports. The server MUST diagnose and abort if capabilities it does\n+not understand was requested. The server MUST NOT ignore capabilities\n+that client requested and server advertised. As a consequence of these\n+rules, server MUST NOT advertise capabilities it does not understand.\n+\n+See protocol-capabilities.txt for a list of allowed server and client\n+capabilities and descriptions.\n \n Reference Discovery\n -------------------\n@@ -154,10 +204,14 @@ If HEAD is a valid ref, HEAD MUST appear as the first advertised\n ref. If HEAD is not a valid ref, HEAD MUST NOT appear in the\n advertisement list at all, but other refs may still appear.\n \n-The stream MUST include capability declarations behind a NUL on the\n-first ref. The peeled value of a ref (that is \"ref^{}\") MUST be\n-immediately after the ref itself, if presented. A conforming server\n-MUST peel the ref if it's an annotated tag.\n+In version 1 the stream MUST include capability declarations behind\n+a NUL on the first ref. The peeled value of a ref (that is \"ref^{}\")\n+MUST be immediately after the ref itself, if presented. A conforming\n+server MUST peel the ref if it's an annotated tag.\n+\n+In version 2 the capabilities are already negotiated, so the first ref\n+MUST NOT be followed by any capability advertisement, but it should be\n+treated as any other refs advertising line.\n \n ----\n advertised-refs = (no-refs / list-of-refs)\n@@ -185,6 +239,21 @@ MUST peel the ref if it's an annotated tag.\n Server and client MUST use lowercase for obj-id, both MUST treat obj-id\n as case-insensitive.\n \n+On the very first line of the initial server response of either\n+receive-pack and upload-pack the first reference is followed by\n+a NUL byte and then a list of space delimited server capabilities.\n+These allow the server to declare what it can and cannot support\n+to the client.\n+\n+Client will then send a space separated list of capabilities it wants\n+to be in effect. The client MUST NOT ask for capabilities the server\n+did not say it supports.\n+\n+Server MUST diagnose and abort if capabilities it does not understand\n+was sent. Server MUST NOT ignore capabilities that client requested\n+and server advertised. As a consequence of these rules, server MUST\n+NOT advertise capabilities it does not understand.\n+\n See protocol-capabilities.txt for a list of allowed server capabilities\n and descriptions.\n \ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex 4f8a7bf..a6241d8 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -3,21 +3,6 @@ Git Protocol Capabilities\n \n Servers SHOULD support all capabilities defined in this document.\n \n-On the very first line of the initial server response of either\n-receive-pack and upload-pack the first reference is followed by\n-a NUL byte and then a list of space delimited server capabilities.\n-These allow the server to declare what it can and cannot support\n-to the client.\n-\n-Client will then send a space separated list of capabilities it wants\n-to be in effect. The client MUST NOT ask for capabilities the server\n-did not say it supports.\n-\n-Server MUST diagnose and abort if capabilities it does not understand\n-was sent. Server MUST NOT ignore capabilities that client requested\n-and server advertised. As a consequence of these rules, server MUST\n-NOT advertise capabilities it does not understand.\n-\n The 'atomic', 'report-status', 'delete-refs', 'quiet', and 'push-cert'\n capabilities are sent and recognized by the receive-pack (push to server)\n process.\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"262692","messageId":"CACsJy8BEEvgP_YzVnbPaLL9QBs48sd2fn6CeicXD_9U92=9UnA@mail.gmail.com","threadId":"39489","inReplyTo":"1433203338-27493-2-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 01/16] stringlist: add from_space_separated_string","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-02T09:42:04Z","receivedAt":"2015-06-02T09:42:04Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n> diff --git a/string-list.h b/string-list.h\n> index d3809a1..88c18e9 100644\n> --- a/string-list.h\n> +++ b/string-list.h\n> @@ -19,6 +19,7 @@ struct string_list {\n> #define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }\n>\n> void string_list_init(struct string_list *list, int strdup_strings);\n> +void from_space_separated_string(struct string_list *list, char *line);\n\nThe name feels out of place. All functions in here have \"string_list\"\nsomewhere in their names. The implementation looks very close to\nstring_list_split() but that name's already taken.. Maybe\nstring_list_split_by_space()?\n-- \nDuy\n"},{"id":"262695","messageId":"CACsJy8CYKi_MTV+FUSFmTSMR3zu-Ks5pe_b4=HVn-msur=D4yA@mail.gmail.com","threadId":"39489","inReplyTo":"1433203338-27493-14-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 13/16] fetch-pack: use the configured transport protocol","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-02T09:55:02Z","receivedAt":"2015-06-02T09:55:02Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n> @@ -127,6 +128,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n> args.update_shallow = 1;\n> continue;\n> }\n> + if (!skip_prefix(arg, \"--transport-version\", &arg)) {\n\nI think this line should be \"if (skip_prefix(arg,\n\"--transport-version=\", &arg)) {\".\n-- \nDuy\n"},{"id":"262696","messageId":"CACsJy8Dped78Db0Pb455-dxha4aaQnSWDN_TwRpe9miVmwHcjg@mail.gmail.com","threadId":"39489","inReplyTo":"1433203338-27493-11-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 10/16] transport: connect_setup appends protocol version number","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-02T09:58:14Z","receivedAt":"2015-06-02T09:58:14Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n> Signed-off-by: Stefan Beller \n> ---\n>\n> Notes:\n> name it to_free\n>\n> transport.c | 17 +++++++++++++++--\n> 1 file changed, 15 insertions(+), 2 deletions(-)\n>\n> diff --git a/transport.c b/transport.c\n> index 651f0ac..b49fc60 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -496,15 +496,28 @@ static int set_git_option(struct git_transport_options *opts,\n> static int connect_setup(struct transport *transport, int for_push, int verbose)\n> {\n> struct git_transport_data *data = transport->data;\n> + const char *remote_program;\n> + char *to_free = 0;\n>\n> if (data->conn)\n> return 0;\n>\n> + remote_program = (for_push ? data->options.receivepack\n> + : data->options.uploadpack);\n> +\n> + if (transport->smart_options->transport_version >= 2) {\n> + to_free = xmalloc(strlen(remote_program) + 12);\n> + sprintf(to_free, \"%s-%d\", remote_program,\n> + transport->smart_options->transport_version);\n> + remote_program = to_free;\n> + }\n> +\n\nIt looks to me that the caller should pass \"upload-pack-2\" here in\ndata->options.uploadpack already. We should not need to manipulate the\nuploadpack's program name. Not sure how complicated it would be\nthough.\n\n> data->conn = git_connect(data->fd, transport->url,\n> - for_push ? data->options.receivepack :\n> - data->options.uploadpack,\n> + remote_program,\n> verbose ? CONNECT_VERBOSE : 0);\n>\n> + free(to_free);\n> +\n> return 0;\n> }\n>\n> --\n> 2.4.1.345.gab207b6.dirty\n>\n\n\n\n-- \nDuy\n"},{"id":"262697","messageId":"CACsJy8AadHof8bxeD3jfZ-MKSBLXGdw23SeJLgp7sCy+N7n_GA@mail.gmail.com","threadId":"39489","inReplyTo":"1433203338-27493-14-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 13/16] fetch-pack: use the configured transport protocol","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-02T10:02:45Z","receivedAt":"2015-06-02T10:02:45Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n> builtin/fetch-pack.c | 22 ++++++++++-\n> fetch-pack.c | 109 +++++++++++++++++++++++++++------------------------\n> fetch-pack.h | 1 +\n> 3 files changed, 80 insertions(+), 52 deletions(-)\n\nAnd the companion changes in transport-helper.c should be in this\npatch as well to support smart http. I don't think there is any\nproblem with how you store the \"version\" (or \"transport_version\", you\nshould probably stick to one name) though.\n-- \nDuy\n"},{"id":"262707","messageId":"20150602113238.GA15140@LK-Perkele-VII","threadId":"39489","inReplyTo":"CACsJy8AadHof8bxeD3jfZ-MKSBLXGdw23SeJLgp7sCy+N7n_GA@mail.gmail.com","subject":"Re: [RFCv2 13/16] fetch-pack: use the configured transport protocol","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2015-06-02T11:32:38Z","receivedAt":"2015-06-02T11:32:38Z","isPatch":false,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Jun 02, 2015 at 05:02:45PM +0700, Duy Nguyen wrote:\n> On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n> > builtin/fetch-pack.c | 22 ++++++++++-\n> > fetch-pack.c | 109 +++++++++++++++++++++++++++------------------------\n> > fetch-pack.h | 1 +\n> > 3 files changed, 80 insertions(+), 52 deletions(-)\n> \n> And the companion changes in transport-helper.c should be in this\n> patch as well to support smart http. I don't think there is any\n> problem with how you store the \"version\" (or \"transport_version\", you\n> should probably stick to one name) though.\n\nLooking at transport-helper.c, process_connect() looks to need patching,\nit handles smart transport establishment via remote helpers.\n\nLooking at the routine it calls, both the name and exec look to need\npatching.\n\nI think that if process_connect() succeeds, then connect_setup() will\nhit the \"if (data->conn) return 0;\" case and exit early.\n\ntransport-helper.c doesn't look to have anything smart-http specific.\n\n\n-Ilari\n"},{"id":"262735","messageId":"CAPig+cTsBU2t=NJEe16N2CMyYrWgbP-NvoKxNQ9QdPiokXj+hw@mail.gmail.com","threadId":"39489","inReplyTo":"CACsJy8BEEvgP_YzVnbPaLL9QBs48sd2fn6CeicXD_9U92=9UnA@mail.gmail.com","subject":"Re: [RFCv2 01/16] stringlist: add from_space_separated_string","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-02T15:10:49Z","receivedAt":"2015-06-02T15:10:49Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 2, 2015 at 5:42 AM, Duy Nguyen wrote:\n> On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n>> diff --git a/string-list.h b/string-list.h\n>> index d3809a1..88c18e9 100644\n>> --- a/string-list.h\n>> +++ b/string-list.h\n>> @@ -19,6 +19,7 @@ struct string_list {\n>> #define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }\n>>\n>> void string_list_init(struct string_list *list, int strdup_strings);\n>> +void from_space_separated_string(struct string_list *list, char *line);\n>\n> The name feels out of place. All functions in here have \"string_list\"\n> somewhere in their names. The implementation looks very close to\n> string_list_split() but that name's already taken.. Maybe\n> string_list_split_by_space()?\n\nIndeed. If you really want to go the specialized route, splitting only\non whitespace, then Duy's suggestion makes sense. Alternately,\nstring_list_split_ws() might be easily understood while still\nremaining somewhat terse.\n\nHowever, why make this so specialized? A more generalized function\ncould be more widely useful. For instance, you could introduce a\nfunction very similar to string_list_split() to which you supply\nmultiple delimiter characters (as a 'const char *') rather than the\nsingle delimiter character accepted by string_list_split(). The\nfunction could be named string_list_split_any() or\nstring_list_tokenize().\n\nAlso, it's ugly and inconvenient to require the incoming string be\nnon-const, and feels as if you're letting the interface be dictated by\nan implementation detail (underlying use of strtok_r).\n"},{"id":"262754","messageId":"CAGZ79katiDViBzC8sMJaDqaekc4+KSyTnfdYD-uni9K72iAVpg@mail.gmail.com","threadId":"39489","inReplyTo":"CAPig+cTsBU2t=NJEe16N2CMyYrWgbP-NvoKxNQ9QdPiokXj+hw@mail.gmail.com","subject":"Re: [RFCv2 01/16] stringlist: add from_space_separated_string","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T17:54:08Z","receivedAt":"2015-06-02T17:54:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 8:10 AM, Eric Sunshine wrote:\n> On Tue, Jun 2, 2015 at 5:42 AM, Duy Nguyen wrote:\n>> On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n>>> diff --git a/string-list.h b/string-list.h\n>>> index d3809a1..88c18e9 100644\n>>> --- a/string-list.h\n>>> +++ b/string-list.h\n>>> @@ -19,6 +19,7 @@ struct string_list {\n>>> #define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }\n>>>\n>>> void string_list_init(struct string_list *list, int strdup_strings);\n>>> +void from_space_separated_string(struct string_list *list, char *line);\n>>\n>> The name feels out of place. All functions in here have \"string_list\"\n>> somewhere in their names. The implementation looks very close to\n>> string_list_split() but that name's already taken.. Maybe\n>> string_list_split_by_space()?\n>\n> Indeed. If you really want to go the specialized route, splitting only\n> on whitespace, then Duy's suggestion makes sense. Alternately,\n> string_list_split_ws() might be easily understood while still\n> remaining somewhat terse.\n>\n> However, why make this so specialized? A more generalized function\n> could be more widely useful. For instance, you could introduce a\n> function very similar to string_list_split() to which you supply\n> multiple delimiter characters (as a 'const char *') rather than the\n> single delimiter character accepted by string_list_split(). The\n> function could be named string_list_split_any() or\n> string_list_tokenize().\n>\n> Also, it's ugly and inconvenient to require the incoming string be\n> non-const, and feels as if you're letting the interface be dictated by\n> an implementation detail (underlying use of strtok_r).\n\nI see. I think I can even use string_list_split here, and drop this patch.\n\nThanks for pointing out the flaws!\n"},{"id":"262755","messageId":"CAGZ79kYg0W7biSXV1VH8NKcG=YNpotvbB7GD5-JAg8hEq0LOuw@mail.gmail.com","threadId":"39489","inReplyTo":"CACsJy8Dped78Db0Pb455-dxha4aaQnSWDN_TwRpe9miVmwHcjg@mail.gmail.com","subject":"Re: [RFCv2 10/16] transport: connect_setup appends protocol version number","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T18:04:52Z","receivedAt":"2015-06-02T18:04:52Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 2:58 AM, Duy Nguyen wrote:\n> On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller wrote:\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Notes:\n>> name it to_free\n>>\n>> transport.c | 17 +++++++++++++++--\n>> 1 file changed, 15 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/transport.c b/transport.c\n>> index 651f0ac..b49fc60 100644\n>> --- a/transport.c\n>> +++ b/transport.c\n>> @@ -496,15 +496,28 @@ static int set_git_option(struct git_transport_options *opts,\n>> static int connect_setup(struct transport *transport, int for_push, int verbose)\n>> {\n>> struct git_transport_data *data = transport->data;\n>> + const char *remote_program;\n>> + char *to_free = 0;\n>>\n>> if (data->conn)\n>> return 0;\n>>\n>> + remote_program = (for_push ? data->options.receivepack\n>> + : data->options.uploadpack);\n>> +\n>> + if (transport->smart_options->transport_version >= 2) {\n>> + to_free = xmalloc(strlen(remote_program) + 12);\n>> + sprintf(to_free, \"%s-%d\", remote_program,\n>> + transport->smart_options->transport_version);\n>> + remote_program = to_free;\n>> + }\n>> +\n>\n> It looks to me that the caller should pass \"upload-pack-2\" here in\n> data->options.uploadpack already. We should not need to manipulate the\n> uploadpack's program name. Not sure how complicated it would be\n> though.\n>\n\nI tried that before as it seemed to be the better approach to me. But\nthere are multiple\noccasions where you can overwrite the \"upload-pack\" string. It can be\na repository\noption or globally configured or coming from a command line argument.\n\nAnd keeping track of all these places and both passing around the\nversion number as\nwell as the binary name seemed cumbersome to me when seeing my implementation.\nThis way we only have a very small change and you can tell the version\nnumber from\nthe name, which is an advantage (the version is fixed and will not be\nnegotiable, so you\nneed some way to tell the protocol version and the name seems to be\nthe obvious choice).\n\nSo if there are no strong arguments for doing it the other way, I'd\nlike to keep it this way.\n"},{"id":"262763","messageId":"xmqqk2vm2bh3.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-4-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 03/16] connect: rewrite feature parsing to work on string_list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T18:48:08Z","receivedAt":"2015-06-02T18:48:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n\nThe change in 2/16 to extract something into a helper was sort-of\nguessable without any comment (i.e. perhaps it will gain new callers\nin later patches), but a change like this needs to hint why this is\na good thing to do, I would think.\n\nI think you'll be rerolling this anyway due to droppage of 1/16, so\nhopefully we'll see the next version a bit better explained than\nthis round ;-)\n\nThanks.\n\n> builtin/receive-pack.c | 13 +++++----\n> connect.c | 79 ++++++++++++++++++++++----------------------------\n> connect.h | 2 +-\n> upload-pack.c | 11 +++++--\n> 4 files changed, 51 insertions(+), 54 deletions(-)\n"},{"id":"262766","messageId":"xmqqfv6a2ayx.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-5-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T18:59:02Z","receivedAt":"2015-06-02T18:59:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Subject: [RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack\n\nNit; s/I/i/, to match others in the series, I think.\n\n> In upload-pack-2 we send each capability in its own packet buffer.\n>\n> Signed-off-by: Stefan Beller \n> ---\n>\n> Notes:\n> Moved the capabilities into a struct containing all the capabilities,\n> and then we selectively cancel out unwanted capabilities.\n\n> diff --git a/upload-pack-2.c b/upload-pack-2.c\n> new file mode 120000\n> index 0000000..e30a871\n> --- /dev/null\n> +++ b/upload-pack-2.c\n> @@ -0,0 +1 @@\n> +upload-pack.c\n> \\ No newline at end of file\n\nYuck.\n\nCan't we do an equivalent without this symbolic link, i.e. a new\nMakefile rule to compile upload-pack.c in two different ways to two\ndifferent object files?\n\nThe way this patch is organized makes it unclear which part is what\nwas added for v2 and which part is shared with v1 (and changes can\nbe possible breakage to the existing code), leading to a patch that\nis hard to review.\n\n> diff --git a/upload-pack.c b/upload-pack.c\n> index 2493964..7477ca3 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -716,33 +716,69 @@ static void format_symref_info(struct strbuf *buf, struct string_list *symref)\n> \t\tstrbuf_addf(buf, \" symref=%s:%s\", item->string, (char *)item->util);\n> }\n> \n> +static int advertise_capabilities = 1;\n> +const char *all_capabilities[] = {\n> +\t\"multi_ack\",\n> +\t\"thin-pack\",\n> +\t\"side-band\",\n> +\t\"side-band-64k\",\n> +\t\"ofs-delta\",\n> +\t\"shallow\",\n> +\t\"no-progress\",\n> +\t\"include-tag\",\n> +\t\"multi_ack_detailed\",\n> +\t\"allow-tip-sha1-in-want\",\n> +\t\"no-done\",\n> +};\n> +\n> +static void send_capabilities(void)\n\nThis looks like send-capabilities-v2. I am OK to share code between\nv1 and v2 by having two implementations in the same file and some\n(or major part of) helper functions or overall structure code shared\nbetween the two, but if you are taking that route, a version specific\nhelper like this needs to be made clear which one is which.\n\n> +{\n> +\tint i;\n> +\tfor (i = 0; i < ARRAY_SIZE(all_capabilities); i++) {\n> +\t\tconst char *cap = all_capabilities[i];\n> +\t\tif (!strcmp(cap, \"allow-tip-sha1-in-want\") && !allow_tip_sha1_in_want)\n> +\t\t\tcontinue;\n> +\t\tif (!strcmp(cap, \"no-done\") && !stateless_rpc)\n> +\t\t\tcontinue;\n> +\t\tpacket_write(1,\"%s\", cap);\n\ns/1,/1, /;\n\n> static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\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> +\n\nAnd is this one only for v1?\n\n> \tconst char *refname_nons = strip_namespace(refname);\n> \tunsigned char peeled[20];\n> \n> \tif (mark_our_ref(refname, sha1))\n> \t\treturn 0;\n> \n> -\tif (capabilities) {\n> -\t\tstruct strbuf symref_info = STRBUF_INIT;\n> -\n> -\t\tformat_symref_info(&symref_info, cb_data);\n> -\t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n> -\t\t\t sha1_to_hex(sha1), refname_nons,\n> -\t\t\t 0, capabilities,\n> -\t\t\t allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n> -\t\t\t stateless_rpc ? \" no-done\" : \"\",\n> -\t\t\t symref_info.buf,\n> -\t\t\t git_user_agent_sanitized());\n> -\t\tstrbuf_release(&symref_info);\n> +\tif (advertise_capabilities) {\n> +\t\tint i;\n> +\t\tstruct strbuf capabilities = STRBUF_INIT;\n> +\n> +\t\tfor (i = 0; i < ARRAY_SIZE(all_capabilities); i++) {\n> +\t\t\tconst char *cap = all_capabilities[i];\n> +\t\t\tif (!strcmp(cap, \"allow-tip-sha1-in-want\") && !allow_tip_sha1_in_want)\n> +\t\t\t\tcontinue;\n> +\t\t\tif (!strcmp(cap, \"no-done\") && !stateless_rpc)\n> +\t\t\t\tcontinue;\n> +\t\t\tstrbuf_addf(&capabilities, \" %s\", cap);\n> +\t\t}\n> +\n> +\t\tformat_symref_info(&capabilities, cb_data);\n> +\t\tstrbuf_addf(&capabilities, \" agent=%s\", git_user_agent_sanitized());\n> +\n> +\t\tpacket_write(1, \"%s %s%c%s\\n\", sha1_to_hex(sha1), refname_nons,\n> +\t\t\t 0, capabilities.buf);\n> +\t\tstrbuf_release(&capabilities);\n> +\t\tadvertise_capabilities = 0;\n\nThe change itself may be going in the right direction, but I'd\nsuggest doing this in two steps:\n\n * refactor existing v1 without adding anything v2 specific\n (e.g. send-capabilities above). A new file-scope global\n all_capabilities[] array, use of it in this new implementation of\n send_ref(), and introduction of the separate\n advertise_capabilities bool, are good examples of refactoring of\n v1.\n\n * then on top of that solid foundation, add v2 specific stuff.\n"},{"id":"262782","messageId":"xmqqlhg124ji.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-6-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 05/16] remote.h: Change get_remote_heads return to void","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:17:53Z","receivedAt":"2015-06-02T21:17:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> No function uses the return value of get_remote_heads, so we don't want\n> to confuse readers by it.\n>\n> Signed-off-by: Stefan Beller \n> ---\n\nThis is somewhat a sad change, as the returned value is designed to\nbe useful if caller wants to continue appending to the list.\n\nNow such a caller has to tangle the list (the variable it gave the\nfunction as the fourth argument) itself to find its tail.\n\nDoes it really \"confuse\" readers enough that it hurts to have a\nreturn value?\n\n> connect.c | 10 ++++------\n> remote.h | 8 ++++----\n> 2 files changed, 8 insertions(+), 10 deletions(-)\n>\n> diff --git a/connect.c b/connect.c\n> index 4295ba1..a2c777e 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -108,10 +108,10 @@ static void annotate_refs_with_symref_info(struct ref *ref)\n> /*\n> * Read all the refs from the other end\n> */\n> -struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n> -\t\t\t struct ref **list, unsigned int flags,\n> -\t\t\t struct sha1_array *extra_have,\n> -\t\t\t struct sha1_array *shallow_points)\n> +void get_remote_heads(int in, char *src_buf, size_t src_len,\n> +\t\t struct ref **list, unsigned int flags,\n> +\t\t struct sha1_array *extra_have,\n> +\t\t struct sha1_array *shallow_points)\n> {\n> \tstruct ref **orig_list = list;\n> \tint got_at_least_one_head = 0;\n> @@ -172,8 +172,6 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n> \t}\n> \n> \tannotate_refs_with_symref_info(*orig_list);\n> -\n> -\treturn list;\n> }\n> \n> static const char *parse_feature_value(struct string_list *feature_list, const char *feature, int *lenp)\n> diff --git a/remote.h b/remote.h\n> index 02d66ce..d5242b0 100644\n> --- a/remote.h\n> +++ b/remote.h\n> @@ -144,10 +144,10 @@ int check_ref_type(const struct ref *ref, int flags);\n> void free_refs(struct ref *ref);\n> \n> struct sha1_array;\n> -extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n> -\t\t\t\t struct ref **list, unsigned int flags,\n> -\t\t\t\t struct sha1_array *extra_have,\n> -\t\t\t\t struct sha1_array *shallow);\n> +extern void get_remote_heads(int in, char *src_buf, size_t src_len,\n> +\t\t\t struct ref **list, unsigned int flags,\n> +\t\t\t struct sha1_array *extra_have,\n> +\t\t\t struct sha1_array *shallow);\n> \n> int resolve_remote_symref(struct ref *ref, struct ref *list);\n> int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);\n"},{"id":"262783","messageId":"xmqqh9qp24hx.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-7-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 06/16] remote.h: add new struct for options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:18:50Z","receivedAt":"2015-06-02T21:18:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n\nWhy?\n\n> remote.h | 14 ++++++++++++++\n> 1 file changed, 14 insertions(+)\n>\n> diff --git a/remote.h b/remote.h\n> index d5242b0..16cacfe 100644\n> --- a/remote.h\n> +++ b/remote.h\n> @@ -56,6 +56,20 @@ struct remote {\n> \tchar *http_proxy;\n> };\n> \n> +/* todo: discuss if this should be merged with\n> + * git_transport_options in transport.c */\n> +struct transport_options {\n> +\tunsigned multi_ack : 2;\n> +\tunsigned no_done : 1;\n> +\tunsigned use_thin_pack : 1;\n> +\tunsigned no_progress : 1;\n> +\tunsigned include_tag : 1;\n> +\tunsigned prefer_ofs_delta : 1;\n> +\tunsigned agent_supported : 1;\n> +\tunsigned allow_tip_sha1_in_want : 1;\n> +\tunsigned use_sideband;\n> +};\n> +\n> struct remote *remote_get(const char *name);\n> struct remote *pushremote_get(const char *name);\n> int remote_is_configured(const char *name);\n"},{"id":"262784","messageId":"xmqqd21d248d.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-10-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 09/16] remote.h: add get_remote_capabilities, request_capabilities","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:24:34Z","receivedAt":"2015-06-02T21:24:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Instead of calling get_remote_heads as a first command during the\n> protocol exchange, we need to have fine grained control over the\n> capability negotiation in version 2 of the protocol.\n>\n> Introduce get_remote_capabilities, which will just listen to\n> capabilities of the remote and request_capabilities which will\n> tell the selection of capabilities to the remote.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> connect.c | 37 +++++++++++++++++++++++++++++++++++++\n> remote.h | 3 +++\n> 2 files changed, 40 insertions(+)\n>\n> diff --git a/connect.c b/connect.c\n> index a2c777e..4ebe1dc 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -105,6 +105,43 @@ static void annotate_refs_with_symref_info(struct ref *ref)\n> \tstring_list_clear(&symref, 0);\n> }\n> \n> +void get_remote_capabilities(int in, char *src_buf, size_t src_len)\n\nWe need to clarify that this is for v2 only in its name, no?\n\n> +{\n> +\tstring_list_clear(&server_capabilities, 1);\n> +\tfor (;;) {\n> +\t\tint len;\n> +\t\tchar *line = packet_buffer;\n> +\n> +\t\tlen = packet_read(in, &src_buf, &src_len,\n> +\t\t\t\t packet_buffer, sizeof(packet_buffer),\n> +\t\t\t\t PACKET_READ_GENTLE_ON_EOF |\n> +\t\t\t\t PACKET_READ_CHOMP_NEWLINE);\n> +\t\tif (len < 0)\n> +\t\t\tdie_initial_contact(0);\n> +\n> +\t\tif (!len)\n> +\t\t\tbreak;\n\nOK, so we get sequence of capabilities, one per packet, until\npacket_flush(). I wonder if we want to hint that with:\n\n\t\tif (!len)\n\t\t\tbreak; /* flush */\n\nor something like that.\n\n> +\t\tstring_list_append(&server_capabilities, line);\n> +\t}\n> +}\n\n> +int request_capabilities(int out, struct transport_options *options)\n> +{\n> +\tif (options->multi_ack == 2) packet_write(out, \"multi_ack_detailed\");\n\nSpell these just like you would do any other functions, i.e.\n\n\tif (options->multi_ack == 2)\n \tpacket_write(out, \"multi_ack_detailed\");\n\n\nI think this step is sensible, and anticipate that a new method that\ncorresponds to this will be introduced to the transport layer.\n"},{"id":"262785","messageId":"CAGZ79kbuwT_Wd099fys7Dz8y3ckC7pXk-qV4xe=1w4rXhMwRQg@mail.gmail.com","threadId":"39489","inReplyTo":"xmqqlhg124ji.fsf@gitster.dls.corp.google.com","subject":"Re: [RFCv2 05/16] remote.h: Change get_remote_heads return to void","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T21:25:03Z","receivedAt":"2015-06-02T21:25:03Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 2:17 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> No function uses the return value of get_remote_heads, so we don't want\n>> to confuse readers by it.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>\n> This is somewhat a sad change, as the returned value is designed to\n> be useful if caller wants to continue appending to the list.\n\nBut there are no callers since like 2005. ;)\n(I did not exactly track it down, but even the last caller went away\nrather fast)\n\n>\n> Does it really \"confuse\" readers enough that it hurts to have a\n> return value?\n>\n\nProbably no.\nBut I think this is just carrying around cruft we could avoid?\n\n>\n> Now such a caller has to tangle the list (the variable it gave the\n> function as the fourth argument) itself to find its tail.\n\nSo you're saying if someone in the future really wants to append\nto that list, they don't find out to just return it again but rather\ndo an O(n) operation?\n"},{"id":"262786","messageId":"xmqq8uc123n5.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-11-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 10/16] transport: connect_setup appends protocol version number","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:37:18Z","receivedAt":"2015-06-02T21:37:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n>\n> Notes:\n> name it to_free\n>\n> transport.c | 17 +++++++++++++++--\n> 1 file changed, 15 insertions(+), 2 deletions(-)\n>\n> diff --git a/transport.c b/transport.c\n> index 651f0ac..b49fc60 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -496,15 +496,28 @@ static int set_git_option(struct git_transport_options *opts,\n> static int connect_setup(struct transport *transport, int for_push, int verbose)\n> {\n> \tstruct git_transport_data *data = transport->data;\n> +\tconst char *remote_program;\n> +\tchar *to_free = 0;\n\n\tchar *to_free = NULL;\n\n> +\tremote_program = (for_push ? data->options.receivepack\n> +\t\t\t\t : data->options.uploadpack);\n> +\n> +\tif (transport->smart_options->transport_version >= 2) {\n> +\t\tto_free = xmalloc(strlen(remote_program) + 12);\n> +\t\tsprintf(to_free, \"%s-%d\", remote_program,\n> +\t\t\ttransport->smart_options->transport_version);\n> +\t\tremote_program = to_free;\n> +\t}\n\nHmph, so everybody else thinks it is interacting with 'upload-pack',\nand this is the only function that knows it is actually talking with\n'upload-pack-2'?\n\nI am wondering why there isn't a separate helper function that\nmunges data->options.{uploadpack,receivepack} fields based on\nthe value of transport_version that is called _before_ this function\nis called.\n\nAlso, how does this interact with the name of the program the end\nuser can specify via \"fetch --upload-pack=\" option?\n\n> \tdata->conn = git_connect(data->fd, transport->url,\n> -\t\t\t\t for_push ? data->options.receivepack :\n> -\t\t\t\t data->options.uploadpack,\n> +\t\t\t\t remote_program,\n> \t\t\t\t verbose ? CONNECT_VERBOSE : 0);\n> \n> +\tfree(to_free);\n> +\n> \treturn 0;\n> }\n"},{"id":"262787","messageId":"CAGZ79kZ5q1bbHjVL3W6gT7QceQdza9+Lquu6vzcjO6vKeu7fSQ@mail.gmail.com","threadId":"39489","inReplyTo":"xmqqh9qp24hx.fsf@gitster.dls.corp.google.com","subject":"Re: [RFCv2 06/16] remote.h: add new struct for options","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T21:40:13Z","receivedAt":"2015-06-02T21:40:13Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 2:18 PM, Junio C Hamano wrote:\n> Why?\n>\n\nTo have all options required for selecting the capabilities\ntogether in one struct.\n\nCurrently there are independent variables used in a few places for this\n(fetchpack.c: lines 296 - 309, which is where I also got the formatting from)\n\nAs soon as I realized I need to touch many places I thought about introducing\nthis struct. But I guess I'll drop this as well as the patches\nbuilding on top of this\nbecause it's actually only fetch-pack.c using it and it would have communicated\nwith the transport layer with this struct.\n\nMaybe it is better to let the caller handle each option anyway, so that the\ntransport layer just invokes a callback at the caller (fetchpack or later on\nsendpack) which immediately decides if it knows the capability and needs to\nact on it.\n"},{"id":"262788","messageId":"xmqq4mmp23go.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"CAGZ79kbuwT_Wd099fys7Dz8y3ckC7pXk-qV4xe=1w4rXhMwRQg@mail.gmail.com","subject":"Re: [RFCv2 05/16] remote.h: Change get_remote_heads return to void","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:41:11Z","receivedAt":"2015-06-02T21:41:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Jun 2, 2015 at 2:17 PM, Junio C Hamano wrote:\n>> Stefan Beller writes:\n>>\n>>> No function uses the return value of get_remote_heads, so we don't want\n>>> to confuse readers by it.\n>>>\n>>> Signed-off-by: Stefan Beller \n>>> ---\n>>\n>> This is somewhat a sad change, as the returned value is designed to\n>> be useful if caller wants to continue appending to the list.\n>\n> But there are no callers since like 2005. ;)\n\nThat is not a good excuse when it comes to the API design to deal\nwith linked lists in a bog-standard way, though. Many callers of\ncommit_list_insert() may ignore its return values, but even when the\nlast caller stopped caring it, that does not necessarily mean the\nfeature can be removed.\n"},{"id":"262789","messageId":"xmqqzj4hzsyx.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"CAGZ79kZ5q1bbHjVL3W6gT7QceQdza9+Lquu6vzcjO6vKeu7fSQ@mail.gmail.com","subject":"Re: [RFCv2 06/16] remote.h: add new struct for options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:43:50Z","receivedAt":"2015-06-02T21:43:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Jun 2, 2015 at 2:18 PM, Junio C Hamano wrote:\n>> Why?\n>>\n>\n> To have all options required for selecting the capabilities\n> together in one struct.\n\nNo need to explain in the e-mail, as that won't be kept in our\nhistory, unless you write it in your reroll. Just keep them in mind\nand try not to forget when you reroll ;-).\n\nThanks.\n"},{"id":"262790","messageId":"xmqqvbf5zsvl.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-12-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 11/16] remote: have preselect_capabilities","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:45:50Z","receivedAt":"2015-06-02T21:45:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n\nWhy?\n\nWhen adding a new thing that nobody uses yet, please explain what it\nis used for and how it would help the callers in what way to help\nreviewers.\n\n> connect.c | 28 ++++++++++++++++++++++++++++\n> remote.h | 1 +\n> 2 files changed, 29 insertions(+)\n>\n> diff --git a/connect.c b/connect.c\n> index 4ebe1dc..752b9a5 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -126,6 +126,34 @@ void get_remote_capabilities(int in, char *src_buf, size_t src_len)\n> \t}\n> }\n> \n> +/* just select all options the server advertised. */\n> +void preselect_capabilities(struct transport_options *options)\n> +{\n> +\tif (is_repository_shallow() && !server_supports(\"shallow\"))\n> +\t\tdie(\"Server does not support shallow clients\");\n> +\n> +\tif (server_supports(\"multi_ack\"))\n> +\t\toptions->multi_ack = 1;\n> +\telse if (server_supports(\"multi_ack_detailed\"))\n> +\t\toptions->multi_ack = 2;\n> +\n> +\tif (server_supports(\"side-band\"))\n> +\t\toptions->use_sideband = 1;\n> +\telse if (server_supports(\"side-band-64k\"))\n> +\t\toptions->use_sideband = 2;\n> +\n> +\tif (server_supports(\"no-done\"))\n> +\t\toptions->no_done = 1;\n> +\tif (server_supports(\"thin-pack\"))\n> +\t\toptions->use_thin_pack = 1;\n> +\tif (server_supports(\"no-progress\"))\n> +\t\toptions->no_progress = 1;\n> +\tif (server_supports(\"include-tag\"))\n> +\t\toptions->include_tag = 1;\n> +\tif (server_supports(\"ofs-delta\"))\n> +\t\toptions->prefer_ofs_delta = 1;\n> +}\n> +\n> int request_capabilities(int out, struct transport_options *options)\n> {\n> \tif (options->multi_ack == 2) packet_write(out, \"multi_ack_detailed\");\n> diff --git a/remote.h b/remote.h\n> index 61619c5..264a513 100644\n> --- a/remote.h\n> +++ b/remote.h\n> @@ -166,6 +166,7 @@ extern void get_remote_heads(int in, char *src_buf, size_t src_len,\n> \t\t\t struct sha1_array *shallow);\n> \n> void get_remote_capabilities(int in, char *src_buf, size_t src_len);\n> +void preselect_capabilities(struct transport_options *options);\n> int request_capabilities(int out, struct transport_options*);\n> \n> int resolve_remote_symref(struct ref *ref, struct ref *list);\n"},{"id":"262792","messageId":"xmqqr3ptzsfj.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-13-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 12/16] transport: get_refs_via_connect exchanges capabilities before refs.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:55:28Z","receivedAt":"2015-06-02T21:55:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n>\n> Notes:\n> A minor issue I am unsure about here is the\n> line\n> \t&& transport->smart_options->transport_version)\n> which could be prevented if we always set the transport_version\n> in make_remote to be the default remote version.\n> \n> The advantage of having it always set in make_remote would\n> be a cleaner mind model (the version set is always accurate)\n> as opposed to now (version may be 0, then the default\n> applies as we don't care enough to set a version)\n> \n> However I think the code may be more ugly if we were to\n> always set the version in make_remote as then we would need\n> to move the DEFAULT_TRANSPORT_VERSION define into remote.h\n> or somewhere else (transport.h is not included in remote.c,\n> I guess that's on purpose?)\n\nInteresting. After reading 9/16, I was somehow expecting to see\nthat a new method get_capability() would be added to the transport\nlayer, and a function get_capability_via_connect() that calls\nget_remote_capabilities() would be its implementation for the\n\"connect\" transport.\n\nThe capability thing however is limited to the Git-aware aka smart\ntransports and it is unlikely that other transports would benefit\nfrom such an organization, so I think the way this step integrates\nthe new get_remote_capabilities() to the system would be not just\nsufficient but is more appropriate.\n\nIf you do not like the switching based on version in this function,\nhowever, it is probably an option to add the new method and define\nconnect_v1 and connect_v2 as two separate transports. The latter\nwould have get_capability() method, while the former (and all the\nother transports) does not define it. And the overall transport\nlayer would call get_capability() method when defined, and then\nget_refs() method next, or something like that.\n\n> transport.c | 28 ++++++++++++++++++++++++----\n> transport.h | 6 ++++++\n> 2 files changed, 30 insertions(+), 4 deletions(-)\n>\n> diff --git a/transport.c b/transport.c\n> index b49fc60..ba40677 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -523,14 +523,33 @@ static int connect_setup(struct transport *transport, int for_push, int verbose)\n> \n> static struct ref *get_refs_via_connect(struct transport *transport, int for_push)\n> {\n> +\tstruct transport_options options;\n> \tstruct git_transport_data *data = transport->data;\n> \tstruct ref *refs;\n> +\tint version = DEFAULT_TRANSPORT_VERSION;\n> \n> +\tif (transport->smart_options\n> +\t && transport->smart_options->transport_version)\n> +\t\tversion = transport->smart_options->transport_version;\n> \tconnect_setup(transport, for_push, 0);\n> -\tget_remote_heads(data->fd[0], NULL, 0, &refs,\n> -\t\t\t for_push ? REF_NORMAL : 0,\n> -\t\t\t &data->extra_have,\n> -\t\t\t &data->shallow);\n> +\tswitch (version) {\n> +\tcase 2: /* first talk about capabilities, then get the heads */\n> +\t\tget_remote_capabilities(data->fd[0], NULL, 0);\n> +\t\tpreselect_capabilities(&options);\n> +\t\tif (transport->select_capabilities)\n> +\t\t\ttransport->select_capabilities(&options);\n> +\t\trequest_capabilities(data->fd[1], &options);\n> +\t\t/* fall through */\n> +\tcase 1:\n> +\t\tget_remote_heads(data->fd[0], NULL, 0, &refs,\n> +\t\t\t\t for_push ? REF_NORMAL : 0,\n> +\t\t\t\t &data->extra_have,\n> +\t\t\t\t &data->shallow);\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tdie(\"BUG: Transport version %d not supported\", version);\n> +\t\tbreak;\n> +\t}\n> \tdata->got_remote_heads = 1;\n> \n> \treturn refs;\n> @@ -987,6 +1006,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n> \t\tstruct git_transport_data *data = xcalloc(1, sizeof(*data));\n> \t\tret->data = data;\n> \t\tret->set_option = NULL;\n> +\t\tret->select_capabilities = NULL;\n> \t\tret->get_refs_list = get_refs_via_connect;\n> \t\tret->fetch = fetch_refs_via_pack;\n> \t\tret->push_refs = git_transport_push;\n> diff --git a/transport.h b/transport.h\n> index 6095d7a..3e63efc 100644\n> --- a/transport.h\n> +++ b/transport.h\n> @@ -74,6 +74,12 @@ struct transport {\n> \tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n> \n> \t/**\n> +\t * A callback to select protocol options. Must be set if\n> +\t * the caller wants to change transport options.\n> +\t */\n> +\tvoid (*select_capabilities)(struct transport_options *);\n> +\n> +\t/**\n> \t * Push the objects and refs. Send the necessary objects, and\n> \t * then, for any refs where peer_ref is set and\n> \t * peer_ref->new_sha1 is different from old_sha1, tell the\n"},{"id":"262794","messageId":"xmqqmw0hzsc6.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"1433203338-27493-16-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 15/16] Documentation/technical/pack-protocol: Mention http as possible protocol","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T21:57:29Z","receivedAt":"2015-06-02T21:57:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n>\n> This may go unrelated to this series as well.\n\nYeah, this can come before this series as a good independent\nclean-up.\n\n>\n> Documentation/technical/pack-protocol.txt | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/technical/pack-protocol.txt\n> b/Documentation/technical/pack-protocol.txt\n> index fc09c63..4064fc7 100644\n> --- a/Documentation/technical/pack-protocol.txt\n> +++ b/Documentation/technical/pack-protocol.txt\n> @@ -1,11 +1,11 @@\n> Packfile transfer protocols\n> ===========================\n> \n> -Git supports transferring data in packfiles over the ssh://, git:// and\n> +Git supports transferring data in packfiles over the ssh://, git://, http:// and\n> file:// transports. There exist two sets of protocols, one for pushing\n> data from a client to a server and another for fetching data from a\n> -server to a client. All three transports (ssh, git, file) use the same\n> -protocol to transfer data.\n> +server to a client. The three transports (ssh, git, file) use the same\n> +protocol to transfer data. http is documented in http-protocol.txt.\n> \n> The processes invoked in the canonical Git implementation are 'upload-pack'\n> on the server side and 'fetch-pack' on the client side for fetching data;\n"},{"id":"262795","messageId":"xmqqiob5zs6w.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"xmqqmw0hzsc6.fsf@gitster.dls.corp.google.com","subject":"Re: [RFCv2 15/16] Documentation/technical/pack-protocol: Mention http as possible protocol","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T22:00:39Z","receivedAt":"2015-06-02T22:00:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Stefan Beller writes:\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> This may go unrelated to this series as well.\n>\n> Yeah, this can come before this series as a good independent\n> clean-up.\n\nI'll pick this up and queue (with s/: Mention/: mention/) separately\nfor 'next'.\n\nThanks.\n"},{"id":"262796","messageId":"CAGZ79kbnX_kyuvj73PGcO7OBOj7CfdouARrqNWEkCnUfdN=DqQ@mail.gmail.com","threadId":"39489","inReplyTo":"xmqq8uc123n5.fsf@gitster.dls.corp.google.com","subject":"Re: [RFCv2 10/16] transport: connect_setup appends protocol version number","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T22:09:23Z","receivedAt":"2015-06-02T22:09:23Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 2:37 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Notes:\n>> name it to_free\n>>\n>> transport.c | 17 +++++++++++++++--\n>> 1 file changed, 15 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/transport.c b/transport.c\n>> index 651f0ac..b49fc60 100644\n>> --- a/transport.c\n>> +++ b/transport.c\n>> @@ -496,15 +496,28 @@ static int set_git_option(struct git_transport_options *opts,\n>> static int connect_setup(struct transport *transport, int for_push, int verbose)\n>> {\n>> struct git_transport_data *data = transport->data;\n>> + const char *remote_program;\n>> + char *to_free = 0;\n>\n> char *to_free = NULL;\n>\n>> + remote_program = (for_push ? data->options.receivepack\n>> + : data->options.uploadpack);\n>> +\n>> + if (transport->smart_options->transport_version >= 2) {\n>> + to_free = xmalloc(strlen(remote_program) + 12);\n>> + sprintf(to_free, \"%s-%d\", remote_program,\n>> + transport->smart_options->transport_version);\n>> + remote_program = to_free;\n>> + }\n>\n> Hmph, so everybody else thinks it is interacting with 'upload-pack',\n> and this is the only function that knows it is actually talking with\n> 'upload-pack-2'?\n\nYes.\n\n>\n> I am wondering why there isn't a separate helper function that\n> munges data->options.{uploadpack,receivepack} fields based on\n> the value of transport_version that is called _before_ this function\n> is called.\n\nThat makes sense.\n\n>\n> Also, how does this interact with the name of the program the end\n> user can specify via \"fetch --upload-pack=\" option?\n\nYou'd specify --upload-pack=foo-frotz and --transport-version=2\nand it would look for foo-frotz-2 instead.\n\nThe problem IMHO is we have quite a few places where the\nupload-pack binary path can be configured. Either as a command line\noption or as a repository configuration.\n\nAnd the way we're currently architecting the next protocol, the version\nis encoded in the file name, which makes sense (an old binary will not\naccept a new protocol), so what should happen when\n\n* there is a repository configuration \"upload-pack-custom\" for upload-pack\n for historic reasons. When just switching to a new version, you would need\n to add a \"upload-pack-custom-2\" binary on the server side anyway\n\n* additionally to the configured value you want to play around with the new\n protocol, so would you rather just say \"--transport-version=2\" or also need\n to have some sort of \"--upload-pack=upload-pack-custom-another-path\"\n involved? It's easy to forget the second option I believe.\n\n* the user specifies\n\"--upload-pack=custom-upload-pack-which-talks-version1\" and\n \"--transport-version=2\" together. This will fail, but at which stage do we\n want to fail?\n\nAll these questions lead me to think it's maybe better to make the rest of Git\nunaware of the added \"-${version}\" string and pretend we would be talking to\nupload-pack instead.\n"},{"id":"262798","messageId":"CAGZ79kaTusesFuMZ92797CGw_pbTp8GQ4-nhJ_ciVTXobbLepQ@mail.gmail.com","threadId":"39489","inReplyTo":"xmqqr3ptzsfj.fsf@gitster.dls.corp.google.com","subject":"Re: [RFCv2 12/16] transport: get_refs_via_connect exchanges capabilities before refs.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T22:19:44Z","receivedAt":"2015-06-02T22:19:44Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 2:55 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Notes:\n>> A minor issue I am unsure about here is the\n>> line\n>> && transport->smart_options->transport_version)\n>> which could be prevented if we always set the transport_version\n>> in make_remote to be the default remote version.\n>>\n>> The advantage of having it always set in make_remote would\n>> be a cleaner mind model (the version set is always accurate)\n>> as opposed to now (version may be 0, then the default\n>> applies as we don't care enough to set a version)\n>>\n>> However I think the code may be more ugly if we were to\n>> always set the version in make_remote as then we would need\n>> to move the DEFAULT_TRANSPORT_VERSION define into remote.h\n>> or somewhere else (transport.h is not included in remote.c,\n>> I guess that's on purpose?)\n>\n> Interesting. After reading 9/16, I was somehow expecting to see\n> that a new method get_capability() would be added to the transport\n> layer, and a function get_capability_via_connect() that calls\n> get_remote_capabilities() would be its implementation for the\n> \"connect\" transport.\n\nThis suggests that we collect the capabilities at first and then handover\na (possibly huge) list to the upper calling layer with all our capabilities.\n\nPeff pointed out last Friday (I re-read the email on Monday unfortunately),\nthat we don't want to have buffers of capabilities at all for future extension.\n\nSo the design we need to actually have here is to have a callback given to the\ntransport layer, which calls this callback for each capability received.\nThen the upper layer must decide for each capability if it knows it and wants\nto store it or drop it or just set a flag for it.\n\n>\n> The capability thing however is limited to the Git-aware aka smart\n> transports and it is unlikely that other transports would benefit\n> from such an organization, so I think the way this step integrates\n> the new get_remote_capabilities() to the system would be not just\n> sufficient but is more appropriate.\n>\n> If you do not like the switching based on version in this function,\n> however, it is probably an option to add the new method and define\n> connect_v1 and connect_v2 as two separate transports. The latter\n> would have get_capability() method, while the former (and all the\n> other transports) does not define it. And the overall transport\n> layer would call get_capability() method when defined, and then\n> get_refs() method next, or something like that.\n\nThe way you quoted my previous email, it seems to me as if you specially\nwant to address my concerns from the notes.\nHowever my actual concern was only focused on the one line if we do\n\n+ if (transport->smart_options\n+ && transport->smart_options->transport_version) // <-- this line\n+ version = transport->smart_options->transport_version;\n\nor rather want to have it a bit cleaner with\n\n+ if (transport->smart_options)\n+ version = transport->smart_options->transport_version;\n\nThis second approach assumes transport->smart_options->transport_version;\nto be not NULL, i.e. when setting up the smart options we must make the\ndecision which protocol version to use. At this point in time this would be\njust initializing the transport_version correctly in remote.h. But that's more\nlines than just this one.\n\n\n>\n>> transport.c | 28 ++++++++++++++++++++++++----\n>> transport.h | 6 ++++++\n>> 2 files changed, 30 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/transport.c b/transport.c\n>> index b49fc60..ba40677 100644\n>> --- a/transport.c\n>> +++ b/transport.c\n>> @@ -523,14 +523,33 @@ static int connect_setup(struct transport *transport, int for_push, int verbose)\n>>\n>> static struct ref *get_refs_via_connect(struct transport *transport, int for_push)\n>> {\n>> + struct transport_options options;\n>> struct git_transport_data *data = transport->data;\n>> struct ref *refs;\n>> + int version = DEFAULT_TRANSPORT_VERSION;\n>>\n>> + if (transport->smart_options\n>> + && transport->smart_options->transport_version)\n>> + version = transport->smart_options->transport_version;\n>> connect_setup(transport, for_push, 0);\n>> - get_remote_heads(data->fd[0], NULL, 0, &refs,\n>> - for_push ? REF_NORMAL : 0,\n>> - &data->extra_have,\n>> - &data->shallow);\n>> + switch (version) {\n>> + case 2: /* first talk about capabilities, then get the heads */\n>> + get_remote_capabilities(data->fd[0], NULL, 0);\n>> + preselect_capabilities(&options);\n>> + if (transport->select_capabilities)\n>> + transport->select_capabilities(&options);\n>> + request_capabilities(data->fd[1], &options);\n>> + /* fall through */\n>> + case 1:\n>> + get_remote_heads(data->fd[0], NULL, 0, &refs,\n>> + for_push ? REF_NORMAL : 0,\n>> + &data->extra_have,\n>> + &data->shallow);\n>> + break;\n>> + default:\n>> + die(\"BUG: Transport version %d not supported\", version);\n>> + break;\n>> + }\n>> data->got_remote_heads = 1;\n>>\n>> return refs;\n>> @@ -987,6 +1006,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n>> struct git_transport_data *data = xcalloc(1, sizeof(*data));\n>> ret->data = data;\n>> ret->set_option = NULL;\n>> + ret->select_capabilities = NULL;\n>> ret->get_refs_list = get_refs_via_connect;\n>> ret->fetch = fetch_refs_via_pack;\n>> ret->push_refs = git_transport_push;\n>> diff --git a/transport.h b/transport.h\n>> index 6095d7a..3e63efc 100644\n>> --- a/transport.h\n>> +++ b/transport.h\n>> @@ -74,6 +74,12 @@ struct transport {\n>> int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n>>\n>> /**\n>> + * A callback to select protocol options. Must be set if\n>> + * the caller wants to change transport options.\n>> + */\n>> + void (*select_capabilities)(struct transport_options *);\n>> +\n>> + /**\n>> * Push the objects and refs. Send the necessary objects, and\n>> * then, for any refs where peer_ref is set and\n>> * peer_ref->new_sha1 is different from old_sha1, tell the\n"},{"id":"262799","messageId":"xmqqa8whzqxr.fsf@gitster.dls.corp.google.com","threadId":"39489","inReplyTo":"CAGZ79kbnX_kyuvj73PGcO7OBOj7CfdouARrqNWEkCnUfdN=DqQ@mail.gmail.com","subject":"Re: [RFCv2 10/16] transport: connect_setup appends protocol version number","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T22:27:44Z","receivedAt":"2015-06-02T22:27:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> Hmph, so everybody else thinks it is interacting with 'upload-pack',\n>> and this is the only function that knows it is actually talking with\n>> 'upload-pack-2'?\n>\n> Yes.\n>>\n>> I am wondering why there isn't a separate helper function that\n>> munges data->options.{uploadpack,receivepack} fields based on\n>> the value of transport_version that is called _before_ this function\n>> is called.\n>\n> That makes sense.\n\nHmph, but then everybody would know that it is now interacting with\nupload-pack-2; I think it probably is a better way to go.\n\nIn any case, all codepaths other than what actually runs exec()\nshould not be basing their decision on the program names---that is\nwhat you added transport_version field for, and they should look\nat that field if they want to switch behaviour between protocol\nversions anyway, so I think we can live with either way.\n\n>> Also, how does this interact with the name of the program the end\n>> user can specify via \"fetch --upload-pack=\" option?\n>\n> You'd specify --upload-pack=foo-frotz and --transport-version=2\n> and it would look for foo-frotz-2 instead.\n\nHmm, that's an unfortunate interaction. If you wrote a new protocol\ndriver that talks v2, it may be natural to say\n\n\tgit fetch --upload-pack=a.out --transport-version=2\n\nwhen you want to test it, and we do not want to invoke a.out-2 in\nthat case.\n\n> The problem IMHO is we have quite a few places where the\n> upload-pack binary path can be configured. Either as a command line\n> option or as a repository configuration.\n\nAt least shouldn't you be able to tell if we are using compiled-in\ndefault or user specified value (whether configuration/command line)?\n\nDoes the code that initialise data->options.{upload,receive}pack\nfields with the compiled-in default values know what protocol\nversion is going to be used at that point? I think that is where\nthe appending of \"-2\" should be done; in other words, I think\naddition of \"-2\" should be done _ONLY_ for compiled-in default\nvalues, and it should be done not just for exec() but should be\nvisible to everybody, including those who are debugging and inspect\ndata->options.uploadpack field. What we spawn must match what is\nstored there.\n"},{"id":"262800","messageId":"CAGZ79kbe+1okiUL5bGcfykyn7MmhALF+3ANY9k-ycadk0RNAuw@mail.gmail.com","threadId":"39489","inReplyTo":"xmqqfv6a2ayx.fsf@gitster.dls.corp.google.com","subject":"Re: [RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-02T23:08:12Z","receivedAt":"2015-06-02T23:08:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 11:59 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Subject: [RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack\n>\n> Nit; s/I/i/, to match others in the series, I think.\n>\n>> In upload-pack-2 we send each capability in its own packet buffer.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Notes:\n>> Moved the capabilities into a struct containing all the capabilities,\n>> and then we selectively cancel out unwanted capabilities.\n>\n>> diff --git a/upload-pack-2.c b/upload-pack-2.c\n>> new file mode 120000\n>> index 0000000..e30a871\n>> --- /dev/null\n>> +++ b/upload-pack-2.c\n>> @@ -0,0 +1 @@\n>> +upload-pack.c\n>> \\ No newline at end of file\n>\n> Yuck.\n>\n> Can't we do an equivalent without this symbolic link, i.e. a new\n> Makefile rule to compile upload-pack.c in two different ways to two\n> different object files?\n\nOk I changed that and it works now (only one upload-pack.c file no\nupload-pack-2.c and no corresponding object either.)\n\nHowever we don't want to have the version used in upload pack depending\non the file name at run time, which is why I am reverting to this state\nand depending on the file name at compile time. Instead of a symlink we\ncould use an option passed into the compiler as well, but I am not sure if\nthat is as easy to add to the Makefile as this way.\n\n>\n> The way this patch is organized makes it unclear which part is what\n> was added for v2 and which part is shared with v1 (and changes can\n> be possible breakage to the existing code), leading to a patch that\n> is hard to review.\n\nok :(\n\nChanged in a reroll.\n"},{"id":"262803","messageId":"CAPig+cTEK5FEBZVqRg1SJEWG77r3-7TZ0hgpuUFfD5FW0dDbKg@mail.gmail.com","threadId":"39489","inReplyTo":"1433203338-27493-15-git-send-email-sbeller@google.com","subject":"Re: [RFCv2 14/16] t5544: add a test case for the new protocol","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-03T00:16:16Z","receivedAt":"2015-06-03T00:16:16Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 1, 2015 at 8:02 PM, Stefan Beller wrote:\n> Signed-off-by: Stefan Beller \n> ---\n> diff --git a/t/t5544-fetch-2.sh b/t/t5544-fetch-2.sh\n> new file mode 100755\n> index 0000000..beee46c\n> --- /dev/null\n> +++ b/t/t5544-fetch-2.sh\n> @@ -0,0 +1,40 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2015 Stefan Beller\n> +#\n> +\n> +test_description='Testing version 2 of the fetch protocol'\n> +\n> +. ./test-lib.sh\n> +\n> +mk_repo_pair () {\n> + rm -rf client server &&\n> + test_create_repo client &&\n> + test_create_repo server &&\n> + (\n> + cd server &&\n> + git config receive.denyCurrentBranch warn\n> + ) &&\n> + (\n> + cd client &&\n> + git remote add origin ../server\n\nBroken &&-chain (still[1]).\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/270014\n\n> + git config remote.origin.transportversion 2\n> + )\n> +}\n> +\n> +test_expect_success 'setup' '\n> + mk_repo_pair &&\n> + (\n> + cd server &&\n> + test_commit one\n> + ) &&\n> + (\n> + cd client &&\n> + git fetch origin master\n> + )\n> +'\n> +\n> +# More to come here, similar to t5515 having a sub directory full of expected\n> +# data going over the wire.\n> +\n> +test_done\n> --\n> 2.4.1.345.gab207b6.dirty\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"}]}