{"thread":{"id":"21878","subject":"[RFC PATCH v3 0/8] Remote helpers smart transport extensions","startedAt":"2009-12-06T16:28:43Z","lastAt":"2009-12-08T06:29:37Z","messageCount":25,"participants":["Ilari Liusvaara","Junio C Hamano","Nanako Shiraishi","Erik Faye-Lund","Nicolas Pitre","Shawn O. Pearce","Jeff King"],"isPatch":true,"patchVersion":3,"patchTotal":8},"messages":[{"id":"129335","messageId":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":null,"subject":"[RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:43Z","receivedAt":"2009-12-06T16:28:43Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Changes from v2 to v3:\n\n- Rename \"FALLBACK\" -> \"fallback\"\n- Get rid of magic \"layer6 ready\" return\n- Initialize paths when called as foo::bar://\n- Remove dead code as NULL remote can't happen\n- Move the stream dupping from unknown protocols patch to taking over\n  transports patch (that one needs it)\n- Reorder the series so that debugging and mandatory cap patches are\n  first.\n- Rename virtual_connected to got_remote_heads (that's what it is)\n- Coding style fix ups\n- Move transport connect code from smart transports to archive patch.\n\nIlari Liusvaara (8):\n  Add remote helper debug mode\n  Support mandatory capabilities\n  Pass unknown protocols to external protocol handlers\n  Refactor git transport options parsing\n  Support taking over transports\n  Support remote helpers implementing smart transports\n  Support remote archive from external protocol helpers\n  Remove special casing of http, https and ftp\n\n .gitignore                           |    4 +\n Documentation/git-remote-helpers.txt |   30 ++++-\n Makefile                             |   24 +++-\n builtin-archive.c                    |   17 ++-\n transport-helper.c                   |  265 +++++++++++++++++++++++++++++-----\n transport.c                          |  231 +++++++++++++++++++++++-------\n transport.h                          |   28 ++++\n 7 files changed, 506 insertions(+), 93 deletions(-)\n"},{"id":"129336","messageId":"1260116931-16549-2-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 1/8] Add remote helper debug mode","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:44Z","receivedAt":"2009-12-06T16:28:44Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Remote helpers deadlock easily, so support debug mode which shows the\ninteraction steps.\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n transport-helper.c |   90 +++++++++++++++++++++++++++++++++++++--------------\n 1 files changed, 65 insertions(+), 25 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 11f3d7e..914bed0 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -8,6 +8,8 @@\n #include \"quote.h\"\n #include \"remote.h\"\n \n+static int debug = 0;\n+\n struct helper_data\n {\n \tconst char *name;\n@@ -22,6 +24,45 @@ struct helper_data\n \tint refspec_nr;\n };\n \n+static void sendline(struct helper_data *helper, struct strbuf *buffer)\n+{\n+\tif (debug)\n+\t\tfprintf(stderr, \"Debug: Remote helper: -> %s\", buffer->buf);\n+\tif (write_in_full(helper->helper->in, buffer->buf, buffer->len)\n+\t\t!= buffer->len)\n+\t\tdie_errno(\"Full write to remote helper failed\");\n+}\n+\n+static int recvline(struct helper_data *helper, struct strbuf *buffer)\n+{\n+\tstrbuf_reset(buffer);\n+\tif (debug)\n+\t\tfprintf(stderr, \"Debug: Remote helper: Waiting...\\n\");\n+\tif (strbuf_getline(buffer, helper->out, '\\n') == EOF) {\n+\t\tif (debug)\n+\t\t\tfprintf(stderr, \"Debug: Remote helper quit.\\n\");\n+\t\texit(128);\n+\t}\n+\n+\tif (debug)\n+\t\tfprintf(stderr, \"Debug: Remote helper: <- %s\\n\", buffer->buf);\n+\treturn 0;\n+}\n+\n+static void xchgline(struct helper_data *helper, struct strbuf *buffer)\n+{\n+\tsendline(helper, buffer);\n+\trecvline(helper, buffer);\n+}\n+\n+static void write_constant(int fd, const char *str)\n+{\n+\tif (debug)\n+\t\tfprintf(stderr, \"Debug: Remote helper: -> %s\", str);\n+\tif (write_in_full(fd, str, strlen(str)) != strlen(str))\n+\t\tdie_errno(\"Full write to remote helper failed\");\n+}\n+\n static struct child_process *get_helper(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n@@ -48,15 +89,15 @@ static struct child_process *get_helper(struct transport *transport)\n \t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\n \tdata->helper = helper;\n \n-\twrite_str_in_full(helper->in, \"capabilities\\n\");\n+\twrite_constant(helper->in, \"capabilities\\n\");\n \n \tdata->out = xfdopen(helper->out, \"r\");\n \twhile (1) {\n-\t\tif (strbuf_getline(&buf, data->out, '\\n') == EOF)\n-\t\t\texit(128); /* child died, message supplied already */\n+\t\trecvline(data, &buf);\n \n \t\tif (!*buf.buf)\n \t\t\tbreak;\n+\t\tif (debug) fprintf(stderr, \"Debug: Got cap %s\\n\", buf.buf);\n \t\tif (!strcmp(buf.buf, \"fetch\"))\n \t\t\tdata->fetch = 1;\n \t\tif (!strcmp(buf.buf, \"option\"))\n@@ -82,14 +123,19 @@ static struct child_process *get_helper(struct transport *transport)\n \t\tfree(refspecs);\n \t}\n \tstrbuf_release(&buf);\n+\tif (debug) fprintf(stderr, \"Debug: Capabilities complete.\\n\");\n \treturn data->helper;\n }\n \n static int disconnect_helper(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n \tif (data->helper) {\n-\t\twrite_str_in_full(data->helper->in, \"\\n\");\n+\t\tif (debug) fprintf(stderr, \"Debug: Disconnecting.\\n\");\n+\t\tstrbuf_addf(&buf, \"\\n\");\n+\t\tsendline(data, &buf);\n \t\tclose(data->helper->in);\n \t\tfclose(data->out);\n \t\tfinish_command(data->helper);\n@@ -117,10 +163,11 @@ static int set_helper_option(struct transport *transport,\n \t\t\t  const char *name, const char *value)\n {\n \tstruct helper_data *data = transport->data;\n-\tstruct child_process *helper = get_helper(transport);\n \tstruct strbuf buf = STRBUF_INIT;\n \tint i, ret, is_bool = 0;\n \n+\tget_helper(transport);\n+\n \tif (!data->option)\n \t\treturn 1;\n \n@@ -143,12 +190,7 @@ static int set_helper_option(struct transport *transport,\n \t\tquote_c_style(value, &buf, NULL, 0);\n \tstrbuf_addch(&buf, '\\n');\n \n-\tif (write_in_full(helper->in, buf.buf, buf.len) != buf.len)\n-\t\tdie_errno(\"cannot send option to %s\", data->name);\n-\n-\tstrbuf_reset(&buf);\n-\tif (strbuf_getline(&buf, data->out, '\\n') == EOF)\n-\t\texit(128); /* child died, message supplied already */\n+\txchgline(data, &buf);\n \n \tif (!strcmp(buf.buf, \"ok\"))\n \t\tret = 0;\n@@ -208,13 +250,10 @@ static int fetch_with_fetch(struct transport *transport,\n \t}\n \n \tstrbuf_addch(&buf, '\\n');\n-\tif (write_in_full(data->helper->in, buf.buf, buf.len) != buf.len)\n-\t\tdie_errno(\"cannot send fetch to %s\", data->name);\n+\tsendline(data, &buf);\n \n \twhile (1) {\n-\t\tstrbuf_reset(&buf);\n-\t\tif (strbuf_getline(&buf, data->out, '\\n') == EOF)\n-\t\t\texit(128); /* child died, message supplied already */\n+\t\trecvline(data, &buf);\n \n \t\tif (!prefixcmp(buf.buf, \"lock \")) {\n \t\t\tconst char *name = buf.buf + 5;\n@@ -249,12 +288,13 @@ static int fetch_with_import(struct transport *transport,\n \t\t\t     int nr_heads, struct ref **to_fetch)\n {\n \tstruct child_process fastimport;\n-\tstruct child_process *helper = get_helper(transport);\n \tstruct helper_data *data = transport->data;\n \tint i;\n \tstruct ref *posn;\n \tstruct strbuf buf = STRBUF_INIT;\n \n+\tget_helper(transport);\n+\n \tif (get_importer(transport, &fastimport))\n \t\tdie(\"Couldn't run fast-import\");\n \n@@ -264,7 +304,7 @@ static int fetch_with_import(struct transport *transport,\n \t\t\tcontinue;\n \n \t\tstrbuf_addf(&buf, \"import %s\\n\", posn->name);\n-\t\twrite_in_full(helper->in, buf.buf, buf.len);\n+\t\tsendline(data, &buf);\n \t\tstrbuf_reset(&buf);\n \t}\n \tdisconnect_helper(transport);\n@@ -369,17 +409,14 @@ static int push_refs(struct transport *transport,\n \t}\n \n \tstrbuf_addch(&buf, '\\n');\n-\tif (write_in_full(helper->in, buf.buf, buf.len) != buf.len)\n-\t\texit(128);\n+\tsendline(data, &buf);\n \n \tref = remote_refs;\n \twhile (1) {\n \t\tchar *refname, *msg;\n \t\tint status;\n \n-\t\tstrbuf_reset(&buf);\n-\t\tif (strbuf_getline(&buf, data->out, '\\n') == EOF)\n-\t\t\texit(128); /* child died, message supplied already */\n+\t\trecvline(data, &buf);\n \t\tif (!buf.len)\n \t\t\tbreak;\n \n@@ -471,8 +508,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \n \twhile (1) {\n \t\tchar *eov, *eon;\n-\t\tif (strbuf_getline(&buf, data->out, '\\n') == EOF)\n-\t\t\texit(128); /* child died, message supplied already */\n+\t\trecvline(data, &buf);\n \n \t\tif (!*buf.buf)\n \t\t\tbreak;\n@@ -497,6 +533,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \t\t}\n \t\ttail = &((*tail)->next);\n \t}\n+\tif (debug) fprintf(stderr, \"Debug: Read ref listing.\\n\");\n \tstrbuf_release(&buf);\n \n \tfor (posn = ret; posn; posn = posn->next)\n@@ -510,6 +547,9 @@ int transport_helper_init(struct transport *transport, const char *name)\n \tstruct helper_data *data = xcalloc(sizeof(*data), 1);\n \tdata->name = name;\n \n+\tif (getenv(\"GIT_TRANSPORT_HELPER_DEBUG\"))\n+\t\tdebug = 1;\n+\n \ttransport->data = data;\n \ttransport->set_option = set_helper_option;\n \ttransport->get_refs_list = get_refs_list;\n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129337","messageId":"1260116931-16549-3-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 2/8] Support mandatory capabilities","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:45Z","receivedAt":"2009-12-06T16:28:45Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Add support for marking capability as mandatory for hosting git version\nto understand. This is useful for helpers which require various types\nof assistance from main git binary.\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n Documentation/git-remote-helpers.txt |    5 ++++-\n transport-helper.c                   |   26 ++++++++++++++++++++------\n 2 files changed, 24 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 5cfdc0c..20a05fe 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -25,7 +25,10 @@ Commands are given by the caller on the helper's standard input, one per line.\n \n 'capabilities'::\n \tLists the capabilities of the helper, one per line, ending\n-\twith a blank line.\n+\twith a blank line. Each capability may be preceeded with '*'.\n+\tThis marks them mandatory for git version using the remote\n+\thelper to understand (unknown mandatory capability is fatal\n+\terror).\n \n 'list'::\n \tLists the refs, one per line, in the format \"<value> <name>\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 914bed0..f5c585d 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -93,24 +93,38 @@ static struct child_process *get_helper(struct transport *transport)\n \n \tdata->out = xfdopen(helper->out, \"r\");\n \twhile (1) {\n+\t\tconst char* capname;\n+\t\tint mandatory = 0;\n \t\trecvline(data, &buf);\n \n \t\tif (!*buf.buf)\n \t\t\tbreak;\n-\t\tif (debug) fprintf(stderr, \"Debug: Got cap %s\\n\", buf.buf);\n-\t\tif (!strcmp(buf.buf, \"fetch\"))\n+\n+\t\tif (*buf.buf == '*') {\n+\t\t\tcapname = buf.buf + 1;\n+\t\t\tmandatory = 1;\n+\t\t} else\n+\t\t\tcapname = buf.buf;\n+\n+\t\tif (debug) fprintf(stderr, \"Debug: Got cap %s\\n\", capname);\n+\t\tif (!strcmp(capname, \"fetch\"))\n \t\t\tdata->fetch = 1;\n-\t\tif (!strcmp(buf.buf, \"option\"))\n+\t\telse if (!strcmp(capname, \"option\"))\n \t\t\tdata->option = 1;\n-\t\tif (!strcmp(buf.buf, \"push\"))\n+\t\telse if (!strcmp(capname, \"push\"))\n \t\t\tdata->push = 1;\n-\t\tif (!strcmp(buf.buf, \"import\"))\n+\t\telse if (!strcmp(capname, \"import\"))\n \t\t\tdata->import = 1;\n-\t\tif (!data->refspecs && !prefixcmp(buf.buf, \"refspec \")) {\n+\t\telse if (!data->refspecs && !prefixcmp(capname, \"refspec \")) {\n \t\t\tALLOC_GROW(refspecs,\n \t\t\t\t   refspec_nr + 1,\n \t\t\t\t   refspec_alloc);\n \t\t\trefspecs[refspec_nr++] = strdup(buf.buf + strlen(\"refspec \"));\n+\t\t} else if (mandatory) {\n+\t\t\tfflush(stderr);\n+\t\t\tdie(\"Unknown madatory capability %s. This remote \"\n+\t\t\t    \"helper probably needs newer version of Git.\\n\",\n+\t\t\t    capname);\n \t\t}\n \t}\n \tif (refspecs) {\n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129340","messageId":"1260116931-16549-4-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 3/8] Pass unknown protocols to external protocol handlers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:46Z","receivedAt":"2009-12-06T16:28:46Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Change URL handling to allow external protocol handlers to implement\nnew protocols without the '::' syntax if helper name does not conflict\nwith any built-in protocol.\n\nfoo:// now invokes git-remote-foo with foo:// URL.\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n transport-helper.c |   22 ++++++++++++-\n transport.c        |   87 +++++++++++++++++++++++++++++++++++++++++++---------\n 2 files changed, 93 insertions(+), 16 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex f5c585d..95aaa02 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -63,6 +63,26 @@ static void write_constant(int fd, const char *str)\n \t\tdie_errno(\"Full write to remote helper failed\");\n }\n \n+const char* remove_ext_force(const char* url)\n+{\n+\tconst char* url2 = url;\n+\tconst char* first_colon = NULL;\n+\n+\tif (!url)\n+\t\treturn NULL;\n+\n+\twhile (*url2 && !first_colon)\n+\t\tif (*url2 == ':')\n+\t\t\tfirst_colon = url2;\n+\t\telse\n+\t\t\turl2++;\n+\n+\tif (first_colon && first_colon[1] == ':')\n+\t\treturn first_colon + 2;\n+\telse\n+\t\treturn url;\n+}\n+\n static struct child_process *get_helper(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n@@ -83,7 +103,7 @@ static struct child_process *get_helper(struct transport *transport)\n \tstrbuf_addf(&buf, \"remote-%s\", data->name);\n \thelper->argv[0] = strbuf_detach(&buf, NULL);\n \thelper->argv[1] = transport->remote->name;\n-\thelper->argv[2] = transport->url;\n+\thelper->argv[2] = remove_ext_force(transport->url);\n \thelper->git_cmd = 1;\n \tif (start_command(helper))\n \t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\ndiff --git a/transport.c b/transport.c\nindex 3eea836..eadd553 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -780,6 +780,55 @@ static int is_file(const char *url)\n \treturn S_ISREG(buf.st_mode);\n }\n \n+static const char* strchrc(const char* str, int c)\n+{\n+\twhile (*str)\n+\t\tif (*str == c)\n+\t\t\treturn str;\n+\t\telse\n+\t\t\tstr++;\n+\treturn NULL;\n+}\n+\n+static int is_url(const char* url)\n+{\n+\tif (!url)\n+\t\treturn 0;\n+\n+\tconst char* url2 = url;\n+\tconst char* first_slash = strchrc(url, '/');\n+\n+\t/* Input with no slash at all or slash first can't be URL. */\n+\tif (!first_slash || first_slash == url)\n+\t\treturn 0;\n+\t/* Character before must be : and next must be /. */\n+\tif (first_slash[-1] != ':' || first_slash[1] != '/')\n+\t\treturn 0;\n+\t/* There must be something before the :// */\n+\tif (first_slash == url + 1)\n+\t\treturn 0;\n+\t/* Check all characters up to first slash. Only alpha, num and\n+\t   : are allowed. : must be followed by : or / */\n+\turl2 = url;\n+\twhile (url2 < first_slash) {\n+\t\tif (*url2 != ':' && !isalnum((unsigned char)*url2))\n+\t\t\treturn 0;\n+\t\tif (*url2 == ':' && url2[1] != ':' && url2[1] != '/')\n+\t\t\treturn 0;\n+\t\tif (*url2 == ':')\n+\t\t\turl2++;\t\t/* Skip second : */\n+\t\turl2++;\n+\t}\n+\n+\t/* Valid enough. */\n+\treturn 1;\n+}\n+\n+static int external_specification_len(const char* url)\n+{\n+\treturn strchrc(url, ':') - url;\n+}\n+\n struct transport *transport_get(struct remote *remote, const char *url)\n {\n \tstruct transport *ret = xcalloc(1, sizeof(*ret));\n@@ -805,30 +854,23 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \n \tif (remote && remote->foreign_vcs) {\n \t\ttransport_helper_init(ret, remote->foreign_vcs);\n-\t\treturn ret;\n-\t}\n-\n-\tif (!prefixcmp(url, \"rsync:\")) {\n+\t} else if (!prefixcmp(url, \"rsync:\")) {\n \t\tret->get_refs_list = get_refs_via_rsync;\n \t\tret->fetch = fetch_objs_via_rsync;\n \t\tret->push = rsync_transport_push;\n-\n-\t} else if (!prefixcmp(url, \"http://\")\n-\t        || !prefixcmp(url, \"https://\")\n-\t        || !prefixcmp(url, \"ftp://\")) {\n-\t\ttransport_helper_init(ret, \"curl\");\n-#ifdef NO_CURL\n-\t\terror(\"git was compiled without libcurl support.\");\n-#endif\n-\n \t} else if (is_local(url) && is_file(url)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n \t\tret->get_refs_list = get_refs_from_bundle;\n \t\tret->fetch = fetch_refs_from_bundle;\n \t\tret->disconnect = close_bundle;\n-\n-\t} else {\n+\t} else if (!is_url(url)\n+\t\t|| !prefixcmp(url, \"file://\")\n+\t\t|| !prefixcmp(url, \"git://\")\n+\t\t|| !prefixcmp(url, \"ssh://\")\n+\t\t|| !prefixcmp(url, \"git+ssh://\")\n+\t\t|| !prefixcmp(url, \"ssh+git://\")) {\n+\t\t/* These are builtin smart transports. */\n \t\tstruct git_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n \t\tret->set_option = set_git_option;\n@@ -845,6 +887,21 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tdata->receivepack = \"git-receive-pack\";\n \t\tif (remote->receivepack)\n \t\t\tdata->receivepack = remote->receivepack;\n+\t} else if (!prefixcmp(url, \"http://\")\n+\t\t|| !prefixcmp(url, \"https://\")\n+\t\t|| !prefixcmp(url, \"ftp://\")) {\n+\t\t/* These three are just plain special. */\n+\t\ttransport_helper_init(ret, \"curl\");\n+#ifdef NO_CURL\n+\t\terror(\"git was compiled without libcurl support.\");\n+#endif\n+\t} else {\n+\t\t/* Unknown protocol in URL. Pass to external handler. */\n+\t\tint len = external_specification_len(url);\n+\t\tchar* handler = xmalloc(len + 1);\n+\t\thandler[len] = 0;\n+\t\tstrncpy(handler, url, len);\n+\t\ttransport_helper_init(ret, handler);\n \t}\n \n \treturn ret;\n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129338","messageId":"1260116931-16549-5-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 4/8] Refactor git transport options parsing","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:47Z","receivedAt":"2009-12-06T16:28:47Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Refactor the transport options parsing so that protocols that aren't\ndirectly smart transports (file://, git://, ssh:// & co) can record\nthe smart transport options for the case if it turns that transport\ncan actually be smart.\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n transport.c |   78 +++++++++++++++++++++++++++++++++++-----------------------\n transport.h |   13 ++++++++++\n 2 files changed, 60 insertions(+), 31 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex eadd553..deaf983 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -395,41 +395,35 @@ static int close_bundle(struct transport *transport)\n }\n \n struct git_transport_data {\n-\tunsigned thin : 1;\n-\tunsigned keep : 1;\n-\tunsigned followtags : 1;\n-\tint depth;\n+\tstruct git_transport_options options;\n \tstruct child_process *conn;\n \tint fd[2];\n-\tconst char *uploadpack;\n-\tconst char *receivepack;\n \tstruct extra_have_objects extra_have;\n };\n \n-static int set_git_option(struct transport *connection,\n+static int set_git_option(struct git_transport_options *opts,\n \t\t\t  const char *name, const char *value)\n {\n-\tstruct git_transport_data *data = connection->data;\n \tif (!strcmp(name, TRANS_OPT_UPLOADPACK)) {\n-\t\tdata->uploadpack = value;\n+\t\topts->uploadpack = value;\n \t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_RECEIVEPACK)) {\n-\t\tdata->receivepack = value;\n+\t\topts->receivepack = value;\n \t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_THIN)) {\n-\t\tdata->thin = !!value;\n+\t\topts->thin = !!value;\n \t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_FOLLOWTAGS)) {\n-\t\tdata->followtags = !!value;\n+\t\topts->followtags = !!value;\n \t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_KEEP)) {\n-\t\tdata->keep = !!value;\n+\t\topts->keep = !!value;\n \t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_DEPTH)) {\n \t\tif (!value)\n-\t\t\tdata->depth = 0;\n+\t\t\topts->depth = 0;\n \t\telse\n-\t\t\tdata->depth = atoi(value);\n+\t\t\topts->depth = atoi(value);\n \t\treturn 0;\n \t}\n \treturn 1;\n@@ -439,7 +433,8 @@ static int connect_setup(struct transport *transport, int for_push, int verbose)\n {\n \tstruct git_transport_data *data = transport->data;\n \tdata->conn = git_connect(data->fd, transport->url,\n-\t\t\t\t for_push ? data->receivepack : data->uploadpack,\n+\t\t\t\t for_push ? data->options.receivepack :\n+\t\t\t\t data->options.uploadpack,\n \t\t\t\t verbose ? CONNECT_VERBOSE : 0);\n \treturn 0;\n }\n@@ -469,15 +464,15 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tstruct ref *refs_tmp = NULL;\n \n \tmemset(&args, 0, sizeof(args));\n-\targs.uploadpack = data->uploadpack;\n-\targs.keep_pack = data->keep;\n+\targs.uploadpack = data->options.uploadpack;\n+\targs.keep_pack = data->options.keep;\n \targs.lock_pack = 1;\n-\targs.use_thin_pack = data->thin;\n-\targs.include_tag = data->followtags;\n+\targs.use_thin_pack = data->options.thin;\n+\targs.include_tag = data->options.followtags;\n \targs.verbose = (transport->verbose > 0);\n \targs.quiet = (transport->verbose < 0);\n \targs.no_progress = args.quiet || (!transport->progress && !isatty(1));\n-\targs.depth = data->depth;\n+\targs.depth = data->options.depth;\n \n \tfor (i = 0; i < nr_heads; i++)\n \t\torigh[i] = heads[i] = xstrdup(to_fetch[i]->name);\n@@ -734,7 +729,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \tmemset(&args, 0, sizeof(args));\n \targs.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);\n \targs.force_update = !!(flags & TRANSPORT_PUSH_FORCE);\n-\targs.use_thin_pack = data->thin;\n+\targs.use_thin_pack = data->options.thin;\n \targs.verbose = !!(flags & TRANSPORT_PUSH_VERBOSE);\n \targs.quiet = !!(flags & TRANSPORT_PUSH_QUIET);\n \targs.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);\n@@ -858,12 +853,14 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->get_refs_list = get_refs_via_rsync;\n \t\tret->fetch = fetch_objs_via_rsync;\n \t\tret->push = rsync_transport_push;\n+\t\tret->smart_options = NULL;\n \t} else if (is_local(url) && is_file(url)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n \t\tret->get_refs_list = get_refs_from_bundle;\n \t\tret->fetch = fetch_refs_from_bundle;\n \t\tret->disconnect = close_bundle;\n+\t\tret->smart_options = NULL;\n \t} else if (!is_url(url)\n \t\t|| !prefixcmp(url, \"file://\")\n \t\t|| !prefixcmp(url, \"git://\")\n@@ -873,20 +870,14 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\t/* These are builtin smart transports. */\n \t\tstruct git_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n-\t\tret->set_option = set_git_option;\n+\t\tret->set_option = 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 \t\tret->disconnect = disconnect_git;\n+\t\tret->smart_options = &(data->options);\n \n-\t\tdata->thin = 1;\n \t\tdata->conn = NULL;\n-\t\tdata->uploadpack = \"git-upload-pack\";\n-\t\tif (remote->uploadpack)\n-\t\t\tdata->uploadpack = remote->uploadpack;\n-\t\tdata->receivepack = \"git-receive-pack\";\n-\t\tif (remote->receivepack)\n-\t\t\tdata->receivepack = remote->receivepack;\n \t} else if (!prefixcmp(url, \"http://\")\n \t\t|| !prefixcmp(url, \"https://\")\n \t\t|| !prefixcmp(url, \"ftp://\")) {\n@@ -904,14 +895,39 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\ttransport_helper_init(ret, handler);\n \t}\n \n+\tif (ret->smart_options) {\n+\t\tret->smart_options->thin = 1;\n+\t\tret->smart_options->uploadpack = \"git-upload-pack\";\n+\t\tif (remote->uploadpack)\n+\t\t\tret->smart_options->uploadpack = remote->uploadpack;\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}\n+\n \treturn ret;\n }\n \n int transport_set_option(struct transport *transport,\n \t\t\t const char *name, const char *value)\n {\n+\tint git_reports = 1, protocol_reports = 1;\n+\n+\tif (transport->smart_options)\n+\t\tgit_reports = set_git_option(transport->smart_options,\n+\t\t\t\t\t     name, value);\n+\n \tif (transport->set_option)\n-\t\treturn transport->set_option(transport, name, value);\n+\t\tprotocol_reports = transport->set_option(transport, name,\n+\t\t\t\t\t\t\tvalue);\n+\n+\t/* If either report is 0, report 0 (success). */\n+\tif (!git_reports || !protocol_reports)\n+\t\treturn 0;\n+\t/* If either reports -1 (invalid value), report -1. */\n+\tif ((git_reports == -1) || (protocol_reports == -1))\n+\t\treturn -1;\n+\t/* Otherwise if both report unknown, report unknown. */\n \treturn 1;\n }\n \ndiff --git a/transport.h b/transport.h\nindex 9e74406..d0192f9 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -4,6 +4,15 @@\n #include \"cache.h\"\n #include \"remote.h\"\n \n+struct git_transport_options {\n+\tunsigned thin : 1;\n+\tunsigned keep : 1;\n+\tunsigned followtags : 1;\n+\tint depth;\n+\tconst char *uploadpack;\n+\tconst char *receivepack;\n+};\n+\n struct transport {\n \tstruct remote *remote;\n \tconst char *url;\n@@ -65,6 +74,10 @@ struct transport {\n \tsigned verbose : 3;\n \t/* Force progress even if the output is not a tty */\n \tunsigned progress : 1;\n+\t/* If transport is at least potentially smart, this points to\n+\t   git_transport_options structure to use in case transport actually\n+\t   turns out to be smart. */\n+\tstruct git_transport_options* smart_options;\n };\n \n #define TRANSPORT_PUSH_ALL 1\n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129342","messageId":"1260116931-16549-6-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 5/8] Support taking over transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:48Z","receivedAt":"2009-12-06T16:28:48Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Add support for taking over transports that turn out to be smart.\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n transport-helper.c |   28 ++++++++++++++++++++++++-\n transport.c        |   57 +++++++++++++++++++++++++++++++++++++++++++++++----\n transport.h        |   10 +++++++++\n 3 files changed, 89 insertions(+), 6 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 95aaa02..3d99fe1 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -22,6 +22,7 @@ struct helper_data\n \t/* These go from remote name (as in \"list\") to private name */\n \tstruct refspec *refspecs;\n \tint refspec_nr;\n+\tstruct git_transport_options gitoptions;\n };\n \n static void sendline(struct helper_data *helper, struct strbuf *buffer)\n@@ -63,6 +64,15 @@ static void write_constant(int fd, const char *str)\n \t\tdie_errno(\"Full write to remote helper failed\");\n }\n \n+static struct child_process* helper_disown(struct transport *transport)\n+{\n+\tstruct helper_data *data = transport->data;\n+\tstruct child_process *child = data->helper;\n+\tfclose(data->out);\n+\tfree(data);\n+\treturn child;\n+}\n+\n const char* remove_ext_force(const char* url)\n {\n \tconst char* url2 = url;\n@@ -91,6 +101,7 @@ static struct child_process *get_helper(struct transport *transport)\n \tconst char **refspecs = NULL;\n \tint refspec_nr = 0;\n \tint refspec_alloc = 0;\n+\tint duped;\n \n \tif (data->helper)\n \t\treturn data->helper;\n@@ -109,9 +120,21 @@ static struct child_process *get_helper(struct transport *transport)\n \t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\n \tdata->helper = helper;\n \n+\t/* Open the output as FILE* so strbuf_getline() can be used.\n+\t   Do this with duped fd because fclose() will close the fd,\n+\t   and stuff like disowning will require the fd to remain.\n+\n+\t   Set the stream to unbuffered because some reads are critical\n+\t   in sense that any overreading will cause deadlocks.\n+\t*/\n+        duped = dup(helper->out);\n+\tif (duped < 0)\n+\t\tdie_errno(\"Can't dup helper output fd\");\n+\tdata->out = xfdopen(duped, \"r\");\n+\tsetvbuf(data->out, NULL, _IONBF, 0);\n+\n \twrite_constant(helper->in, \"capabilities\\n\");\n \n-\tdata->out = xfdopen(helper->out, \"r\");\n \twhile (1) {\n \t\tconst char* capname;\n \t\tint mandatory = 0;\n@@ -171,6 +194,7 @@ static int disconnect_helper(struct transport *transport)\n \t\tstrbuf_addf(&buf, \"\\n\");\n \t\tsendline(data, &buf);\n \t\tclose(data->helper->in);\n+\t\tclose(data->helper->out);\n \t\tfclose(data->out);\n \t\tfinish_command(data->helper);\n \t\tfree((char *)data->helper->argv[0]);\n@@ -590,5 +614,7 @@ int transport_helper_init(struct transport *transport, const char *name)\n \ttransport->fetch = fetch;\n \ttransport->push_refs = push_refs;\n \ttransport->disconnect = release_helper;\n+\ttransport->disown = helper_disown;\n+\ttransport->smart_options = &(data->gitoptions);\n \treturn 0;\n }\ndiff --git a/transport.c b/transport.c\nindex deaf983..e9802e3 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -398,6 +398,7 @@ struct git_transport_data {\n \tstruct git_transport_options options;\n \tstruct child_process *conn;\n \tint fd[2];\n+\tunsigned got_remote_heads : 1;\n \tstruct extra_have_objects extra_have;\n };\n \n@@ -432,10 +433,15 @@ 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+\n+\tif (data->conn)\n+\t\treturn 0;\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 verbose ? CONNECT_VERBOSE : 0);\n+\n \treturn 0;\n }\n \n@@ -447,6 +453,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus\n \tconnect_setup(transport, for_push, 0);\n \tget_remote_heads(data->fd[0], &refs, 0, NULL,\n \t\t\t for_push ? REF_NORMAL : 0, &data->extra_have);\n+\tdata->got_remote_heads = 1;\n \n \treturn refs;\n }\n@@ -477,9 +484,10 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tfor (i = 0; i < nr_heads; i++)\n \t\torigh[i] = heads[i] = xstrdup(to_fetch[i]->name);\n \n-\tif (!data->conn) {\n+\tif (!data->got_remote_heads) {\n \t\tconnect_setup(transport, 0, 0);\n \t\tget_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);\n+\t\tdata->got_remote_heads = 1;\n \t}\n \n \trefs = fetch_pack(&args, data->fd, data->conn,\n@@ -490,6 +498,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tif (finish_connect(data->conn))\n \t\trefs = NULL;\n \tdata->conn = NULL;\n+\tdata->got_remote_heads = 0;\n \n \tfree_refs(refs_tmp);\n \n@@ -718,12 +727,13 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \tstruct send_pack_args args;\n \tint ret;\n \n-\tif (!data->conn) {\n+\tif (!data->got_remote_heads) {\n \t\tstruct ref *tmp_refs;\n \t\tconnect_setup(transport, 1, 0);\n \n \t\tget_remote_heads(data->fd[0], &tmp_refs, 0, NULL, REF_NORMAL,\n \t\t\t\t NULL);\n+\t\tdata->got_remote_heads = 1;\n \t}\n \n \tmemset(&args, 0, sizeof(args));\n@@ -741,6 +751,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \tclose(data->fd[0]);\n \tret |= finish_connect(data->conn);\n \tdata->conn = NULL;\n+\tdata->got_remote_heads = 0;\n \n \treturn ret;\n }\n@@ -749,7 +760,8 @@ static int disconnect_git(struct transport *transport)\n {\n \tstruct git_transport_data *data = transport->data;\n \tif (data->conn) {\n-\t\tpacket_flush(data->fd[1]);\n+\t\tif (data->got_remote_heads)\n+\t\t\tpacket_flush(data->fd[1]);\n \t\tclose(data->fd[0]);\n \t\tclose(data->fd[1]);\n \t\tfinish_connect(data->conn);\n@@ -759,6 +771,35 @@ static int disconnect_git(struct transport *transport)\n \treturn 0;\n }\n \n+void transport_take_over(struct transport *transport)\n+{\n+\tstruct git_transport_data *data;\n+\n+\tif (!transport->disown)\n+\t\tdie(\"Bug detected: Taking over transport requires non-NULL \"\n+\t\t    \"disown method.\");\n+\tif (!transport->smart_options)\n+\t\tdie(\"Bug detected: Taking over transport requires non-NULL \"\n+\t\t    \"smart_options field.\");\n+\n+\tdata = xcalloc(1, sizeof(*data));\n+\tdata->options = *transport->smart_options;\n+\tdata->conn = transport->disown(transport);\n+\tdata->fd[0] = data->conn->out;\n+\tdata->fd[1] = data->conn->in;\n+\tdata->got_remote_heads = 0;\n+\ttransport->data = data;\n+\n+\ttransport->set_option = NULL;\n+\ttransport->get_refs_list = get_refs_via_connect;\n+\ttransport->fetch = fetch_refs_via_pack;\n+\ttransport->push = NULL;\n+\ttransport->push_refs = git_transport_push;\n+\ttransport->disconnect = disconnect_git;\n+\ttransport->smart_options = &(data->options);\n+\ttransport->disown = NULL;\n+}\n+\n static int is_local(const char *url)\n {\n \tconst char *colon = strchr(url, ':');\n@@ -854,6 +895,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->fetch = fetch_objs_via_rsync;\n \t\tret->push = rsync_transport_push;\n \t\tret->smart_options = NULL;\n+\t\tret->disown = NULL;\n \t} else if (is_local(url) && is_file(url)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n@@ -861,6 +903,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->fetch = fetch_refs_from_bundle;\n \t\tret->disconnect = close_bundle;\n \t\tret->smart_options = NULL;\n+\t\tret->disown = NULL;\n \t} else if (!is_url(url)\n \t\t|| !prefixcmp(url, \"file://\")\n \t\t|| !prefixcmp(url, \"git://\")\n@@ -876,8 +919,10 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->push_refs = git_transport_push;\n \t\tret->disconnect = disconnect_git;\n \t\tret->smart_options = &(data->options);\n+\t\tret->disown = NULL;\n \n \t\tdata->conn = NULL;\n+\t\tdata->got_remote_heads = 0;\n \t} else if (!prefixcmp(url, \"http://\")\n \t\t|| !prefixcmp(url, \"https://\")\n \t\t|| !prefixcmp(url, \"ftp://\")) {\n@@ -938,9 +983,9 @@ int transport_push(struct transport *transport,\n \t*nonfastforward = 0;\n \tverify_remote_names(refspec_nr, refspec);\n \n-\tif (transport->push)\n+\tif (transport->push) {\n \t\treturn transport->push(transport, refspec_nr, refspec, flags);\n-\tif (transport->push_refs) {\n+\t} else if (transport->push_refs) {\n \t\tstruct ref *remote_refs =\n \t\t\ttransport->get_refs_list(transport, 1);\n \t\tstruct ref *local_refs = get_local_heads();\n@@ -984,6 +1029,7 @@ const struct ref *transport_get_remote_refs(struct transport *transport)\n {\n \tif (!transport->remote_refs)\n \t\ttransport->remote_refs = transport->get_refs_list(transport, 0);\n+\n \treturn transport->remote_refs;\n }\n \n@@ -1018,6 +1064,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)\n \t}\n \n \trc = transport->fetch(transport, nr_heads, heads);\n+\n \tfree(heads);\n \treturn rc;\n }\ndiff --git a/transport.h b/transport.h\nindex d0192f9..499962d 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -65,6 +65,15 @@ struct transport {\n \tint (*push_refs)(struct transport *transport, struct ref *refs, int flags);\n \tint (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);\n \n+\t/**\n+\t * Disown the transport helper. Releases all resources used\n+\t * by field pointed by member data, except that the child\n+\t * process is not released but returned and whatever is pointed\n+\t * by smart transport options structure is not freed (but the\n+\t * smart transport options structure itself is).\n+\t **/\n+\tstruct child_process* (*disown)(struct transport* connection);\n+\n \t/** get_refs_list(), fetch(), and push_refs() can keep\n \t * resources (such as a connection) reserved for futher\n \t * use. disconnect() releases these resources.\n@@ -128,6 +137,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs);\n void transport_unlock_pack(struct transport *transport);\n int transport_disconnect(struct transport *transport);\n char *transport_anonymize_url(const char *url);\n+void transport_take_over(struct transport *transport);\n \n /* Transport methods defined outside transport.c */\n int transport_helper_init(struct transport *transport, const char *name);\n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129339","messageId":"1260116931-16549-7-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 6/8] Support remote helpers implementing smart transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:49Z","receivedAt":"2009-12-06T16:28:49Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n Documentation/git-remote-helpers.txt |   25 ++++++++++-\n transport-helper.c                   |   84 +++++++++++++++++++++++++++++++++-\n 2 files changed, 106 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 20a05fe..b957813 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -93,6 +93,20 @@ Supported if the helper has the \"push\" capability.\n +\n Supported if the helper has the \"import\" capability.\n \n+'connect' <service>::\n+\tConnects to given service. Stdin and stdout of helper are\n+\tconnected to specified service (git prefix is included in service\n+\tname so e.g. fetching uses 'git-upload-pack' as service) on\n+\tremote side. Valid replies to this command are empty line\n+\t(connection established), 'fallback' (no smart transport support,\n+\tfall back to dumb transports) and just exiting with error message\n+\tprinted (can't connect, don't bother trying to fall back). After\n+\tline feed terminating the positive (empty) response, the output\n+\tof service starts. After the connection ends, the remote\n+\thelper exits.\n++\n+Supported if the helper has the \"connect\" capability.\n+\n If a fatal error occurs, the program writes the error message to\n stderr and exits. The caller should expect that a suitable error\n message has been printed if the child closes the connection without\n@@ -126,6 +140,9 @@ CAPABILITIES\n \tall, it must cover all refs reported by the list command; if\n \tit is not used, it is effectively \"*:*\"\n \n+'connect'::\n+\tThis helper supports the 'connect' command.\n+\n REF LIST ATTRIBUTES\n -------------------\n \n@@ -168,9 +185,15 @@ OPTIONS\n \tbut don't actually change any repository data.\tFor most\n \thelpers this only applies to the 'push', if supported.\n \n+'option servpath <c-style-quoted-path>'::\n+\tSet service path (--upload-pack, --receive-pack etc.) for\n+\tnext connect. Remote helper MAY support this option. Remote\n+\thelper MUST NOT rely on this option being set before\n+\tconnect request occurs.\n+\n Documentation\n -------------\n-Documentation by Daniel Barkalow.\n+Documentation by Daniel Barkalow and Ilari Liusvaara\n \n GIT\n ---\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 3d99fe1..e0254bc 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -18,7 +18,8 @@ struct helper_data\n \tunsigned fetch : 1,\n \t\timport : 1,\n \t\toption : 1,\n-\t\tpush : 1;\n+\t\tpush : 1,\n+\t\tconnect : 1;\n \t/* These go from remote name (as in \"list\") to private name */\n \tstruct refspec *refspecs;\n \tint refspec_nr;\n@@ -163,7 +164,10 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\t\t   refspec_nr + 1,\n \t\t\t\t   refspec_alloc);\n \t\t\trefspecs[refspec_nr++] = strdup(buf.buf + strlen(\"refspec \"));\n-\t\t} else if (mandatory) {\n+\t\t}\n+\t\telse if (!strcmp(capname, \"connect\"))\n+\t\t\tdata->connect = 1;\n+\t\telse if (mandatory) {\n \t\t\tfflush(stderr);\n \t\t\tdie(\"Unknown madatory capability %s. This remote \"\n \t\t\t    \"helper probably needs newer version of Git.\\n\",\n@@ -386,12 +390,78 @@ static int fetch_with_import(struct transport *transport,\n \treturn 0;\n }\n \n+static int _process_connect(struct transport *transport,\n+\t\t\t\t      const char *name, const char *exec)\n+{\n+\tstruct helper_data *data = transport->data;\n+\tstruct strbuf cmdbuf = STRBUF_INIT;\n+\tstruct child_process *helper;\n+\tint r;\n+\n+\thelper = get_helper(transport);\n+\n+\t/* Handle --upload-pack and friends. This is fire and forget...\n+\t   just warn if it fails. */\n+\tif (strcmp(name, exec)) {\n+\t\tr = set_helper_option(transport, \"servpath\", exec);\n+\t\tif (r > 0)\n+\t\t\tfprintf(stderr, \"Warning: Setting remote service path \"\n+\t\t\t\t\"not supported by protocol.\\n\");\n+\t\telse if (r < 0)\n+\t\t\tfprintf(stderr, \"Warning: Invalid remote service \"\n+\t\t\t\t\"path.\\n\");\n+\t}\n+\n+\tif (data->connect)\n+\t\tstrbuf_addf(&cmdbuf, \"connect %s\\n\", name);\n+\telse\n+\t\treturn 0;\n+\n+\txchgline(data, &cmdbuf);\n+\tif (!strcmp(cmdbuf.buf, \"\")) {\n+\t\tif (debug)\n+\t\t\tfprintf(stderr, \"Debug: Smart transport connection \"\n+\t\t\t\t\"ready.\\n\");\n+\t\treturn 1;\n+\t} else if (!strcmp(cmdbuf.buf, \"fallback\")) {\n+\t\tif (debug)\n+\t\t\tfprintf(stderr, \"Debug: Falling back to dumb \"\n+\t\t\t\t\"transport.\\n\");\n+\t\treturn 0;\n+\t} else\n+\t\tdie(\"Unknown response to connect: %s\",\n+\t\t\tcmdbuf.buf);\n+\n+\treturn 0;\t/* Shouldn't be here. */\n+}\n+\n+static int process_connect(struct transport* transport,\n+\t\t\t\t     int for_push)\n+{\n+\tstruct helper_data *data = transport->data;\n+\tconst char *name;\n+\tconst char *exec;\n+\n+\tname = for_push ? \"git-receive-pack\" : \"git-upload-pack\";\n+\tif (for_push)\n+\t\texec = data->gitoptions.receivepack;\n+\telse\n+\t\texec = data->gitoptions.uploadpack;\n+\n+\treturn _process_connect(transport, name, exec);\n+}\n+\n static int fetch(struct transport *transport,\n \t\t int nr_heads, struct ref **to_fetch)\n {\n \tstruct helper_data *data = transport->data;\n \tint i, count;\n \n+\tif (process_connect(transport, 0)) {\n+\t\ttransport_take_over(transport);\n+\t\treturn transport->fetch(transport, nr_heads, to_fetch);\n+\t}\n+\n \tcount = 0;\n \tfor (i = 0; i < nr_heads; i++)\n \t\tif (!(to_fetch[i]->status & REF_STATUS_UPTODATE))\n@@ -419,6 +489,11 @@ static int push_refs(struct transport *transport,\n \tstruct child_process *helper;\n \tstruct ref *ref;\n \n+\tif (process_connect(transport, 1)) {\n+\t\ttransport_take_over(transport);\n+\t\treturn transport->push_refs(transport, remote_refs, flags);\n+\t}\n+\n \tif (!remote_refs)\n \t\treturn 0;\n \n@@ -559,6 +634,11 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \n \thelper = get_helper(transport);\n \n+\tif (process_connect(transport, for_push)) {\n+\t\ttransport_take_over(transport);\n+\t\treturn transport->get_refs_list(transport, for_push);\n+\t}\n+\n \tif (data->push && for_push)\n \t\twrite_str_in_full(helper->in, \"list for-push\\n\");\n \telse\n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129341","messageId":"1260116931-16549-8-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 7/8] Support remote archive from external protocol helpers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:50Z","receivedAt":"2009-12-06T16:28:50Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Helpers which support invoke/connect also should support remote archive\nsnapshot (or at least there's only one way to attempt it). So support\nremote snapshotting for protocol helpers.\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n builtin-archive.c  |   17 ++++++++++-------\n transport-helper.c |   19 +++++++++++++++++++\n transport.c        |   21 +++++++++++++++++++++\n transport.h        |    5 +++++\n 4 files changed, 55 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-archive.c b/builtin-archive.c\nindex 12351e9..d34b3fd 100644\n--- a/builtin-archive.c\n+++ b/builtin-archive.c\n@@ -5,6 +5,7 @@\n #include \"cache.h\"\n #include \"builtin.h\"\n #include \"archive.h\"\n+#include \"transport.h\"\n #include \"parse-options.h\"\n #include \"pkt-line.h\"\n #include \"sideband.h\"\n@@ -25,12 +26,16 @@ static void create_output_file(const char *output_file)\n static int run_remote_archiver(int argc, const char **argv,\n \t\t\t       const char *remote, const char *exec)\n {\n-\tchar *url, buf[LARGE_PACKET_MAX];\n+\tchar buf[LARGE_PACKET_MAX];\n \tint fd[2], i, len, rv;\n-\tstruct child_process *conn;\n+\tstruct transport *transport;\n+\tstruct remote *_remote;\n \n-\turl = xstrdup(remote);\n-\tconn = git_connect(fd, url, exec, 0);\n+\t_remote = remote_get(remote);\n+\tif (!_remote->url[0])\n+\t\tdie(\"git archive: Remote with no URL\");\n+\ttransport = transport_get(_remote, _remote->url[0]);\n+\ttransport_connect(transport, \"git-upload-archive\", exec, fd);\n \n \tfor (i = 1; i < argc; i++)\n \t\tpacket_write(fd[1], \"argument %s\\n\", argv[i]);\n@@ -53,9 +58,7 @@ static int run_remote_archiver(int argc, const char **argv,\n \n \t/* Now, start reading from fd[0] and spit it out to stdout */\n \trv = recv_sideband(\"archive\", fd[0], 1);\n-\tclose(fd[0]);\n-\tclose(fd[1]);\n-\trv |= finish_connect(conn);\n+\trv |= transport_disconnect(transport);\n \n \treturn !!rv;\n }\ndiff --git a/transport-helper.c b/transport-helper.c\nindex e0254bc..a49be55 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -451,6 +451,24 @@ static int process_connect(struct transport* transport,\n \treturn _process_connect(transport, name, exec);\n }\n \n+static int connect_helper(struct transport *transport, const char *name,\n+\t\t   const char *exec, int fd[2])\n+{\n+\tstruct helper_data *data = transport->data;\n+\n+\t/* Get_helper so connect is inited. */\n+\tget_helper(transport);\n+\tif (!data->connect)\n+\t\tdie(\"Operation not supported by protocol.\");\n+\n+\tif (!_process_connect(transport, name, exec))\n+\t\tdie(\"Can't connect to subservice %s.\", name);\n+\n+\tfd[0] = data->helper->out;\n+\tfd[1] = data->helper->in;\n+\treturn 0;\n+}\n+\n static int fetch(struct transport *transport,\n \t\t int nr_heads, struct ref **to_fetch)\n {\n@@ -694,6 +712,7 @@ int transport_helper_init(struct transport *transport, const char *name)\n \ttransport->fetch = fetch;\n \ttransport->push_refs = push_refs;\n \ttransport->disconnect = release_helper;\n+\ttransport->connect = connect_helper;\n \ttransport->disown = helper_disown;\n \ttransport->smart_options = &(data->gitoptions);\n \treturn 0;\ndiff --git a/transport.c b/transport.c\nindex e9802e3..b418889 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -756,6 +756,17 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \treturn ret;\n }\n \n+static int connect_git(struct transport *transport, const char* name,\n+\t\t       const char* executable, int fd[2])\n+{\n+\tstruct git_transport_data *data = transport->data;\n+\tdata->conn = git_connect(data->fd, transport->url,\n+\t\t\t\t executable, 0);\n+\tfd[0] = data->fd[0];\n+\tfd[1] = data->fd[1];\n+\treturn 0;\n+}\n+\n static int disconnect_git(struct transport *transport)\n {\n \tstruct git_transport_data *data = transport->data;\n@@ -917,6 +928,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\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+\t\tret->connect = connect_git;\n \t\tret->disconnect = disconnect_git;\n \t\tret->smart_options = &(data->options);\n \t\tret->disown = NULL;\n@@ -1078,6 +1090,15 @@ void transport_unlock_pack(struct transport *transport)\n \t}\n }\n \n+int transport_connect(struct transport *transport, const char *name,\n+\t\t      const char* exec, int fd[2])\n+{\n+\tif (transport->connect)\n+\t\treturn transport->connect(transport, name, exec, fd);\n+\telse\n+\t\tdie(\"Operation not supported by protocol\");\n+}\n+\n int transport_disconnect(struct transport *transport)\n {\n \tint ret = 0;\ndiff --git a/transport.h b/transport.h\nindex 499962d..d702e31 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -64,6 +64,8 @@ struct transport {\n \t **/\n \tint (*push_refs)(struct transport *transport, struct ref *refs, int flags);\n \tint (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);\n+\tint (*connect)(struct transport *connection, const char* name,\n+\t\t       const char* executable, int fd[2]);\n \n \t/**\n \t * Disown the transport helper. Releases all resources used\n@@ -139,6 +141,9 @@ int transport_disconnect(struct transport *transport);\n char *transport_anonymize_url(const char *url);\n void transport_take_over(struct transport *transport);\n \n+int transport_connect(struct transport *transport, const char *name,\n+\t\t      const char* exec, int fd[2]);\n+\n /* Transport methods defined outside transport.c */\n int transport_helper_init(struct transport *transport, const char *name);\n \n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129343","messageId":"1260116931-16549-9-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v3 8/8] Remove special casing of http, https and ftp","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-06T16:28:51Z","receivedAt":"2009-12-06T16:28:51Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"HTTP, HTTPS and FTP are no longer special to transport code. Also\nadd support for FTPS (curl supports it so it is easy).\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\n .gitignore  |    4 ++++\n Makefile    |   24 ++++++++++++++++++++++--\n transport.c |    8 --------\n 3 files changed, 26 insertions(+), 10 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 7cc54b4..7c79f97 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -107,6 +107,10 @@\n /git-relink\n /git-remote\n /git-remote-curl\n+/git-remote-http\n+/git-remote-https\n+/git-remote-ftp\n+/git-remote-ftps\n /git-repack\n /git-replace\n /git-repo-config\ndiff --git a/Makefile b/Makefile\nindex 42744a4..1332225 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -424,6 +424,13 @@ BUILT_INS += git-stage$X\n BUILT_INS += git-status$X\n BUILT_INS += git-whatchanged$X\n \n+#ifdef NO_CURL\n+REMOTE_CURL_NAMES =\n+#else\n+# Yes, this is missing git-remote-http intentionally!\n+REMOTE_CURL_NAMES = git-remote-https git-remote-ftp git-remote-ftps\n+#endif\n+\n # what 'all' will build and 'install' will install in gitexecdir,\n # excluding programs for built-in commands\n ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)\n@@ -1097,7 +1104,7 @@ else\n \telse\n \t\tCURL_LIBCURL = -lcurl\n \tendif\n-\tPROGRAMS += git-remote-curl$X git-http-fetch$X\n+\tPROGRAMS += git-remote-http$X git-remote-https$X git-remote-ftp$X git-remote-ftps$X git-http-fetch$X\n \tcurl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)\n \tifeq \"$(curl_check)\" \"070908\"\n \t\tifndef NO_EXPAT\n@@ -1676,7 +1683,13 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n \n-git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)\n+$(REMOTE_CURL_NAMES): git-remote-http$X\n+\t$(QUIET_LNCP)$(RM) $@ && \\\n+\tln $< $@ 2>/dev/null || \\\n+\tln -s $< $@ 2>/dev/null || \\\n+\tcp $< $@\n+\n+git-remote-http$X: remote-curl.o http.o http-walker.o $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n \n@@ -1853,6 +1866,7 @@ endif\n ifneq (,$X)\n \t$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' -ef '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p$X' || $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)\n endif\n+\n \tbindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \\\n \texecdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \\\n \t{ test \"$$bindir/\" = \"$$execdir/\" || \\\n@@ -1866,6 +1880,12 @@ endif\n \t\tln -s \"git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n \t\tcp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; \\\n \t  done; } && \\\n+\t{ for p in $(REMOTE_CURL_NAMES); do \\\n+\t\t$(RM) \"$$execdir/$$p\" && \\\n+\t\tln \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n+\t\tln -s \"git-remote-http$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n+\t\tcp \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" || exit; \\\n+\t  done; } && \\\n \t./check_bindir \"z$$bindir\" \"z$$execdir\" \"$$bindir/git-add$X\"\n \n install-doc:\ndiff --git a/transport.c b/transport.c\nindex b418889..c6684ac 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -935,14 +935,6 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \n \t\tdata->conn = NULL;\n \t\tdata->got_remote_heads = 0;\n-\t} else if (!prefixcmp(url, \"http://\")\n-\t\t|| !prefixcmp(url, \"https://\")\n-\t\t|| !prefixcmp(url, \"ftp://\")) {\n-\t\t/* These three are just plain special. */\n-\t\ttransport_helper_init(ret, \"curl\");\n-#ifdef NO_CURL\n-\t\terror(\"git was compiled without libcurl support.\");\n-#endif\n \t} else {\n \t\t/* Unknown protocol in URL. Pass to external handler. */\n \t\tint len = external_specification_len(url);\n-- \n1.6.6.rc1.300.gfbc27\n"},{"id":"129387","messageId":"7v7hsz9qxj.fsf@alter.siamese.dyndns.org","threadId":"21878","inReplyTo":"1260116931-16549-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-07T07:36:08Z","receivedAt":"2009-12-07T07:36:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\nwhile doing so.  Please sanity check the result.\n\nThanks.\n"},{"id":"129419","messageId":"20091207210608.6117@nanako3.lavabit.com","threadId":"21878","inReplyTo":"7v7hsz9qxj.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-07T12:06:08Z","receivedAt":"2009-12-07T12:06:08Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com>\n\n> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\n> while doing so.  Please sanity check the result.\n\nI see that you changed many \"char* variable\" to \"char *variable\", but\nwhat is the reason for these changes?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"129421","messageId":"40aa078e0912070457i16394913v5ff041061ec9eeaf@mail.gmail.com","threadId":"21878","inReplyTo":"20091207210608.6117@nanako3.lavabit.com","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-12-07T12:57:37Z","receivedAt":"2009-12-07T12:57:37Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Dec 7, 2009 at 1:06 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:\n> Quoting Junio C Hamano <gitster@pobox.com>\n>\n>> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\n>> while doing so.  Please sanity check the result.\n>\n> I see that you changed many \"char* variable\" to \"char *variable\", but\n> what is the reason for these changes?\n>\n\nDocumentation/CodingGuidelines:\n\n - When declaring pointers, the star sides with the variable\n   name, i.e. \"char *string\", not \"char* string\" or\n   \"char * string\".  This makes it easier to understand code\n   like \"char *string, c;\".\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"129427","messageId":"alpine.LFD.2.00.0912071043200.31174@xanadu.home","threadId":"21878","inReplyTo":"20091207210608.6117@nanako3.lavabit.com","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-12-07T15:44:41Z","receivedAt":"2009-12-07T15:44:41Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 7 Dec 2009, Nanako Shiraishi wrote:\n\n> Quoting Junio C Hamano <gitster@pobox.com>\n> \n> > I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\n> > while doing so.  Please sanity check the result.\n> \n> I see that you changed many \"char* variable\" to \"char *variable\", but\n> what is the reason for these changes?\n\nThe * is an attribute of the variable and not of the type.  This makes \nfor clearer code.\n\n\nNicolas\n"},{"id":"129430","messageId":"20091207163338.GA27516@Knoppix","threadId":"21878","inReplyTo":"7v7hsz9qxj.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-07T16:33:38Z","receivedAt":"2009-12-07T16:33:38Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sun, Dec 06, 2009 at 11:36:08PM -0800, Junio C Hamano wrote:\n> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\n> while doing so.  Please sanity check the result.\n\nThe conflict resolution seems sane. \n\nSorry about leaving lots of codingstyle stuff unfixed.\n\n-Ilari\n"},{"id":"129433","messageId":"20091207174947.GF17173@spearce.org","threadId":"21878","inReplyTo":"1260116931-16549-6-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v3 5/8] Support taking over transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-07T17:49:47Z","receivedAt":"2009-12-07T17:49:47Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:\n> @@ -109,9 +120,21 @@ static struct child_process *get_helper(struct transport *transport)\n>  \t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\n>  \tdata->helper = helper;\n>  \n> +\t/* Open the output as FILE* so strbuf_getline() can be used.\n> +\t   Do this with duped fd because fclose() will close the fd,\n> +\t   and stuff like disowning will require the fd to remain.\n> +\n> +\t   Set the stream to unbuffered because some reads are critical\n> +\t   in sense that any overreading will cause deadlocks.\n> +\t*/\n> +        duped = dup(helper->out);\n\nFormatting error here, the line is indented wrong.\n\n> +\tif (duped < 0)\n> +\t\tdie_errno(\"Can't dup helper output fd\");\n> +\tdata->out = xfdopen(duped, \"r\");\n> +\tsetvbuf(data->out, NULL, _IONBF, 0);\n\nI wonder if this is really a good idea.  Most helpers actually\nuse a lot of text based IO to communicate.  Disabling buffering\nfor those helpers to avoid overreading the advertisement from a\nconnect is a problem.\n\nMaybe we could leave buffering on, but use a handshake protocol\nwith the helper during connect:\n\n  (1) > \"connect git-upload-pack\\n\"\n  (2) < \"\\n\"\n  (3) > \"begin\\n\"\n\nDuring 2 we are still buffered, but the only content on the pipe\nshould be the single blank line, so we pull that in and the FILE*\nbuffer should be empty.\n\nAfter writing message 2 the remote helper blocks reading for the\n\"begin\\n\" message 3.  This gives the transport-helper.c code time\nto switch off the FILE* and start using raw IO off the pipe before\nany data starts coming down the line.\n\nIt does mean that the helper may need to run unbuffered IO, but\nif the helper is only a smart helper supporting connect then this\nisn't really a problem.  It can run buffered IO until connect is\nreceived, switch to unbuffered, and use unbuffered for the single\n\"begin\\n\" message it has to consume before it starts writing output\nor reading input.\n\n-- \nShawn.\n"},{"id":"129434","messageId":"20091207181148.GG17173@spearce.org","threadId":"21878","inReplyTo":"1260116931-16549-7-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v3 6/8] Support remote helpers implementing smart transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-07T18:11:48Z","receivedAt":"2009-12-07T18:11:48Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 3d99fe1..e0254bc 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n>  static int fetch(struct transport *transport,\n>  \t\t int nr_heads, struct ref **to_fetch)\n>  {\n>  \tstruct helper_data *data = transport->data;\n>  \tint i, count;\n>  \n> +\tif (process_connect(transport, 0)) {\n> +\t\ttransport_take_over(transport);\n> +\t\treturn transport->fetch(transport, nr_heads, to_fetch);\n> +\t}\n\nWe should already be connected because of the prior call into\nget_refs_list().  If I read your code correctly we'd try to open\na new connection right here, which makes no sense.  But its also\npossible for us to be in a different transport, so we do code with\nthe assumption that we didn't get invoked through get_refs_list()\nfirst and therefore need to open the connection ourselves.\n\nAlso, given the above invocation pattern, I see no reason why you\nneed the disown virtual function on struct transport*.  Just pass\nthe #@!**! struct child* into transport_take_over() from the 3\ncall sites here and get rid of that unnecessary indirection.\n\n-- \nShawn.\n"},{"id":"129435","messageId":"20091207181227.GH17173@spearce.org","threadId":"21878","inReplyTo":"1260116931-16549-8-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v3 7/8] Support remote archive from external protocol helpers","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-07T18:12:27Z","receivedAt":"2009-12-07T18:12:27Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:\n> Helpers which support invoke/connect also should support remote archive\n\nThere is no such thing as invoke anymore.\n\n-- \nShawn.\n"},{"id":"129441","messageId":"7vmy1u35yy.fsf@alter.siamese.dyndns.org","threadId":"21878","inReplyTo":"20091207163338.GA27516@Knoppix","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-07T20:05:25Z","receivedAt":"2009-12-07T20:05:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:\n\n> On Sun, Dec 06, 2009 at 11:36:08PM -0800, Junio C Hamano wrote:\n>> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\n>> while doing so.  Please sanity check the result.\n>\n> The conflict resolution seems sane. \n>\n> Sorry about leaving lots of codingstyle stuff unfixed.\n\nNo problem at all, and thanks.\n"},{"id":"129442","messageId":"7vein635vn.fsf@alter.siamese.dyndns.org","threadId":"21878","inReplyTo":"20091207210608.6117@nanako3.lavabit.com","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-07T20:07:24Z","receivedAt":"2009-12-07T20:07:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> Quoting Junio C Hamano <gitster@pobox.com>\n>\n>> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\n>> while doing so.  Please sanity check the result.\n>\n> I see that you changed many \"char* variable\" to \"char *variable\", but\n> what is the reason for these changes?\n\nNico already gave you correct and more concise version; this more verbose\nexplanation is primarily for you who said a few times that you are not\nfluent in C.  Others can skip this message without missing anything.\n\nI haven't asked people why they choose to write like this:\n\n\tchar* string;\n\nbeyond \"that is how we were taught and what we are used to\".\n\nBut if I have to guess, it is because it makes it appear as if you are\nbeing consistent between basic types like \"int\" and pointer types like\n\"char *\".  E.g. these both look like \"<type> <variable>;\"\n\n\tint     variable;\n        char*   variable;\n\nThe example in CodingGuidelines gives a clear illustration that this\nhowever is not the way how C language works.  IOW, the syntax for\ndeclaration is not \"<type> <var1>, <var2>,...;\".  E.g.\n\n\tchar*\tvar1, var2;\n\ndeclares var1, which is a pointer to a char, and var2, which is a char.\nThat is what Nico means by '* is an attribute of the variable and not of\nthe type'.\n\nThe only sane way to read declaration in C is to read from inside to\noutside, starting from a variable name.  When you read \"char **argv\", for\nexample, you treat it as \"char (*(*argv))\", and read it from the variable\nname:\n\n    - It declares argv; what's the type of it I wonder...\n\n    - It is a pointer, because the asterisk in front of it tells us that\n      we can dereference it. Now, what's the type of the result of\n      dereferencing argv, i.e. \"*argv\", I wonder...\n\n    - That \"*argv\" is a pointer, because the asterisk in front of it again\n      tells us that we can dereference it. Now, what's the type of\n      dereferencing that \"*argv\", i.e. \"**argv\", I wonder...\n\n    - Ah, it is a \"char\", that is what this declaration says.\n\nAnd the style we use (which is the same as Linux kernel style) naturally\nmatches the way how you read this declaration.\n"},{"id":"129445","messageId":"20091207203524.GA29629@Knoppix","threadId":"21878","inReplyTo":"20091207181148.GG17173@spearce.org","subject":"Re: [RFC PATCH v3 6/8] Support remote helpers implementing smart transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-07T20:35:24Z","receivedAt":"2009-12-07T20:35:24Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Mon, Dec 07, 2009 at 10:11:48AM -0800, Shawn O. Pearce wrote:\n> \n> We should already be connected because of the prior call into\n> get_refs_list().  If I read your code correctly we'd try to open\n> a new connection right here, which makes no sense. \n\nThe have prior connection case can't happen since take_over_transport()\noverwrites the method pointers.\n\n> But its also\n> possible for us to be in a different transport, so we do code with\n> the assumption that we didn't get invoked through get_refs_list()\n> first and therefore need to open the connection ourselves.\n\nRight. The reason why the code is there is in case somebody invokes\nfetch() first.\n\nThe same things apply to push function too.\n\n> Also, given the above invocation pattern, I see no reason why you\n> need the disown virtual function on struct transport*.  Just pass\n> the #@!**! struct child* into transport_take_over() from the 3\n> call sites here and get rid of that unnecessary indirection.\n\nFixed.\n\n-Ilari\n"},{"id":"129447","messageId":"20091207203711.GB29629@Knoppix","threadId":"21878","inReplyTo":"20091207181227.GH17173@spearce.org","subject":"Re: [RFC PATCH v3 7/8] Support remote archive from external protocol helpers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-07T20:37:11Z","receivedAt":"2009-12-07T20:37:11Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Mon, Dec 07, 2009 at 10:12:27AM -0800, Shawn O. Pearce wrote:\n> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:\n\n> There is no such thing as invoke anymore.\n\nFixed.\n\n-Ilari\n"},{"id":"129453","messageId":"20091207211907.GA3916@Knoppix","threadId":"21878","inReplyTo":"20091207174947.GF17173@spearce.org","subject":"Re: [RFC PATCH v3 5/8] Support taking over transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-07T21:19:08Z","receivedAt":"2009-12-07T21:19:08Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Mon, Dec 07, 2009 at 09:49:47AM -0800, Shawn O. Pearce wrote:\n> \n> > +\tif (duped < 0)\n> > +\t\tdie_errno(\"Can't dup helper output fd\");\n> > +\tdata->out = xfdopen(duped, \"r\");\n> > +\tsetvbuf(data->out, NULL, _IONBF, 0);\n> \n> I wonder if this is really a good idea.  Most helpers actually\n> use a lot of text based IO to communicate.  Disabling buffering\n> for those helpers to avoid overreading the advertisement from a\n> connect is a problem.\n\nI dropped the buffering change. \n \n> Maybe we could leave buffering on, but use a handshake protocol\n> with the helper during connect:\n> \n>   (1) > \"connect git-upload-pack\\n\"\n>   (2) < \"\\n\"\n>   (3) > \"begin\\n\"\n> \n> During 2 we are still buffered, but the only content on the pipe\n> should be the single blank line, so we pull that in and the FILE*\n> buffer should be empty.\n\nDoesn't work. Stream buffering can only be changed before first\nI/O.\n \nI figured out a solution. Dup the file descriptor second time and\nmake another FILE* for it. Then send the connect and read the\nresponse over the new stream. This avoids overreading and allows\nother I/O to be buffered. After connect attempt, the new streaam\ncan be closed.\n\nThis way most of the I/O can be buffered, only reading responses\nto connect commands can't, and that is at most 1 byte if fallbacks\naren't required. And the thing fits nicely inside _process_connect().\nOh, and no protocol change needed.\n\n-Ilari\n"},{"id":"129455","messageId":"20091208072532.6117@nanako3.lavabit.com","threadId":"21878","inReplyTo":"7vein635vn.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-07T22:25:32Z","receivedAt":"2009-12-07T22:25:32Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com>\n\n> Nanako Shiraishi <nanako3@lavabit.com> writes:\n>\n>> Quoting Junio C Hamano <gitster@pobox.com>\n>>\n>>> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts\n>>> while doing so.  Please sanity check the result.\n>>\n>> I see that you changed many \"char* variable\" to \"char *variable\", but\n>> what is the reason for these changes?\n>\n> Nico already gave you correct and more concise version; this more verbose\n> explanation is primarily for you who said a few times that you are not\n> fluent in C.  Others can skip this message without missing anything.\n\nThanks everybody for quick answers and thank you, Junio, \nfor a detailed explanation. Now I understand.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"129482","messageId":"20091208055735.GA9951@coredump.intra.peff.net","threadId":"21878","inReplyTo":"7vein635vn.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-12-08T05:57:35Z","receivedAt":"2009-12-08T05:57:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 07, 2009 at 12:07:24PM -0800, Junio C Hamano wrote:\n\n> I haven't asked people why they choose to write like this:\n> \n> \tchar* string;\n> \n> beyond \"that is how we were taught and what we are used to\".\n\nI have seen it in C++ code and recommended many years ago on\ncomp.lang.c++. The argument was something along the lines of:\n\n  1. It's good to keep type information together, especially in C++\n     where you are often doing things like using types as template\n     parameters.\n\n  2. The fact that \"char* foo, bar\" doesn't do what you want isn't\n     relevant if you have a style guideline not to declare two variables\n     on the same line (because it's easier to notice both if they each\n     get their own line, and because in C++ you can declare closer to\n     the point of use).\n\nBut that is me paraphrasing an argument I read on usenet almost 10 years\nago, so I may be entirely misremembering (and please don't flame me; I\nam presenting it for anthropological curiosity, not because I believe we\nshould use that style).\n\n-Peff\n"},{"id":"129489","messageId":"20091208062937.GA11353@coredump.intra.peff.net","threadId":"21878","inReplyTo":"20091208055735.GA9951@coredump.intra.peff.net","subject":"Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-12-08T06:29:37Z","receivedAt":"2009-12-08T06:29:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 08, 2009 at 12:57:35AM -0500, Jeff King wrote:\n\n> On Mon, Dec 07, 2009 at 12:07:24PM -0800, Junio C Hamano wrote:\n> \n> > I haven't asked people why they choose to write like this:\n> > \n> > \tchar* string;\n> > \n> > beyond \"that is how we were taught and what we are used to\".\n> \n> I have seen it in C++ code and recommended many years ago on\n> comp.lang.c++. The argument was something along the lines of:\n\nPerhaps I should have simply used google:\n\n  http://www2.research.att.com/~bs/bs_faq2.html#whitespace\n\n-Peff\n"}]}