{"thread":{"id":"21859","subject":"[RFC PATCH v2 0/8] Remote helpers smart transport extensions","startedAt":"2009-12-04T15:55:58Z","lastAt":"2009-12-05T13:18:14Z","messageCount":20,"participants":["Ilari Liusvaara","Sverre Rabbelier","Shawn O. Pearce"],"isPatch":true,"patchVersion":2,"patchTotal":8},"messages":[{"id":"129185","messageId":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":null,"subject":"[RFC PATCH v2 0/8] Remote helpers smart transport extensions","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:55:58Z","receivedAt":"2009-12-04T15:55:58Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"This series is reroll of previous version of smart transport extensions,\nwith various technical nits addressed and some errors fixed. Also rebased\non top of latest next.\n\nMajor changes:\n- Service names now have 'git-' prefix.\n- Successful response to <connect> is \"\", not \"OK\".\n- Removed <connect> \"ERROR\" response.\n- <connect-r> renamed to <connect>.\n- <invoke-r> removed. Pass service executable as option \"servpath\" instead.\n- HTTP helpers are now hardlinked if possible (copied if not).\n- Revert the changes to way helpers are invoked.[1]\n- Actually test the intermediate states for gross errors.\n\n[1] The current way seems to have all sorts of funky failure cases, but\nI'm not touching it without fixing it properly.\n\nKnown issues (not caused by this code and does not need this code to be\nvisible):\n- Segfaults when closing transports, caused by double-frees\n- Funky error message if trying to use not-present helper.\n\nIlari Liusvaara (8):\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  Add remote helper debug mode\n  Support mandatory capabilities\n\n .gitignore                           |    4 +\n Documentation/git-remote-helpers.txt |   33 ++++-\n Makefile                             |   24 +++-\n builtin-archive.c                    |   17 ++-\n transport-helper.c                   |  258 +++++++++++++++++++++++++++++-----\n transport.c                          |  258 +++++++++++++++++++++++++++------\n transport.h                          |   32 ++++\n 7 files changed, 534 insertions(+), 92 deletions(-)\n"},{"id":"129184","messageId":"1259942168-24869-2-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 0/8] Remote helpers smart transport extensions","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:55:59Z","receivedAt":"2009-12-04T15:55:59Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"This series is reroll of previous version of smart transport extensions,\nwith various technical nits addressed and some errors fixed. Also rebased\non top of latest next.\n\nMajor changes:\n- Service names now have 'git-' prefix.\n- Successful response to <connect> is \"\", not \"OK\".\n- Removed <connect> \"ERROR\" response.\n- <connect-r> renamed to <connect>.\n- <invoke-r> removed. Pass service executable as option \"servpath\" instead.\n- HTTP helpers are now hardlinked if possible (copied if not).\n- Revert the changes to way helpers are invoked.[1]\n- Actually test that all intermediate states compile.\n\n[1] The current way seems to have all sorts of funky failure cases, but\nI'm not touching it without fixing it properly.\n\nKnown issues (not caused by this code and does not need this code to be\nvisible):\n- Segfaults when closing transports, caused by double-frees\n- Funky error message if trying to use not-present helper.\n\nIlari Liusvaara (8):\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  Add remote helper debug mode\n  Support mandatory capabilities\n\n .gitignore                           |    4 +\n Documentation/git-remote-helpers.txt |   33 ++++-\n Makefile                             |   24 +++-\n builtin-archive.c                    |   17 ++-\n transport-helper.c                   |  257 +++++++++++++++++++++++++++++-----\n transport.c                          |  258 +++++++++++++++++++++++++++------\n transport.h                          |   32 ++++\n 7 files changed, 533 insertions(+), 92 deletions(-)\n"},{"id":"129189","messageId":"1259942168-24869-3-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 0/8] Remote helpers smart transport extensions","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:00Z","receivedAt":"2009-12-04T15:56:00Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"This series is reroll of previous version of smart transport extensions,\nwith various technical nits addressed and some errors fixed. Also rebased\non top of latest next.\n\nMajor changes:\n- Service names now have 'git-' prefix.\n- Successful response to <connect> is \"\", not \"OK\".\n- Removed <connect> \"ERROR\" response.\n- <connect-r> renamed to <connect>.\n- <invoke-r> removed. Pass service executable as option \"servpath\" instead.\n- HTTP helpers are now hardlinked if possible (copied if not).\n- Revert the changes to invoking helpers (see known issues)\n\n\nKnown issues (not caused by this code and does not need this code to be\nvisible):\n- Segfaults when closing transports, caused by double-frees\n- Funky error message if trying to use not-present helper.\n\n\nIlari Liusvaara (8):\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  Add remote helper debug mode\n  Support mandatory capabilities\n\n .gitignore                           |    4 +\n Documentation/git-remote-helpers.txt |   36 +++++-\n Makefile                             |   24 +++-\n builtin-archive.c                    |   17 ++-\n transport-helper.c                   |  261 +++++++++++++++++++++++++++++-----\n transport.c                          |  258 +++++++++++++++++++++++++++------\n transport.h                          |   32 ++++\n 7 files changed, 540 insertions(+), 92 deletions(-)\n"},{"id":"129194","messageId":"1259942168-24869-4-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 1/8] Pass unknown protocols to external protocol handlers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:01Z","receivedAt":"2009-12-04T15:56:01Z","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 |   37 ++++++++++++++++++++++-\n transport.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++-------\n 2 files changed, 106 insertions(+), 13 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 11f3d7e..d3aad5c 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -22,6 +22,26 @@ struct helper_data\n \tint refspec_nr;\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@@ -30,6 +50,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@@ -42,15 +63,26 @@ 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]);\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+\tif((duped = dup(helper->out)) < 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_str_in_full(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@@ -91,6 +123,7 @@ static int disconnect_helper(struct transport *transport)\n \tif (data->helper) {\n \t\twrite_str_in_full(data->helper->in, \"\\n\");\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]);\ndiff --git a/transport.c b/transport.c\nindex 3eea836..162d022 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@@ -812,23 +861,19 @@ 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-\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 +890,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.288.g40e67\n"},{"id":"129187","messageId":"1259942168-24869-5-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 2/8] Refactor git transport options parsing","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:02Z","receivedAt":"2009-12-04T15:56:02Z","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 |   12 +++++++++\n 2 files changed, 59 insertions(+), 31 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 162d022..21310c5 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@@ -861,12 +856,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@@ -876,20 +873,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@@ -907,14 +898,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..5949132 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,9 @@ 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 git_transport_options\n+\t   structure to use in case transport actually 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.288.g40e67\n"},{"id":"129186","messageId":"1259942168-24869-6-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 3/8] Support taking over transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:03Z","receivedAt":"2009-12-04T15:56:03Z","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 |   12 +++++++\n transport.c        |   89 +++++++++++++++++++++++++++++++++++++++++++++++----\n transport.h        |   15 +++++++++\n 3 files changed, 109 insertions(+), 7 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex d3aad5c..5d17fb5 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -20,8 +20,18 @@ 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 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@@ -549,5 +559,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 21310c5..7e6ef2b 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -9,6 +9,8 @@\n #include \"dir.h\"\n #include \"refs.h\"\n \n+struct ref special_transport_layer6_ready;\n+\n /* rsync support */\n \n /*\n@@ -398,6 +400,8 @@ struct git_transport_data {\n \tstruct git_transport_options options;\n \tstruct child_process *conn;\n \tint fd[2];\n+\t/* Connection is fully up. */\n+\tunsigned virtual_connected : 1;\n \tstruct extra_have_objects extra_have;\n };\n \n@@ -432,10 +436,21 @@ 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->virtual_connected && data->conn) {\n+\t\t/* Just mark it connected. */\n+\t\tdata->virtual_connected = 1;\n+\t\treturn 0;\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 verbose ? CONNECT_VERBOSE : 0);\n+\n+\tif(data->conn)\n+\t\tdata->virtual_connected = 1;\n+\n \treturn 0;\n }\n \n@@ -477,7 +492,7 @@ 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->virtual_connected) {\n \t\tconnect_setup(transport, 0, 0);\n \t\tget_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);\n \t}\n@@ -490,6 +505,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->virtual_connected = 0;\n \n \tfree_refs(refs_tmp);\n \n@@ -718,7 +734,7 @@ 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->virtual_connected) {\n \t\tstruct ref *tmp_refs;\n \t\tconnect_setup(transport, 1, 0);\n \n@@ -741,6 +757,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->virtual_connected = 0;\n \n \treturn ret;\n }\n@@ -749,7 +766,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->virtual_connected)\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 +777,35 @@ static int disconnect_git(struct transport *transport)\n \treturn 0;\n }\n \n+static void git_take_over_transport(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->virtual_connected = 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@@ -857,6 +904,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@@ -864,6 +912,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@@ -879,8 +928,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->virtual_connected = 0;\n \t} else if (!prefixcmp(url, \"http://\")\n \t\t|| !prefixcmp(url, \"https://\")\n \t\t|| !prefixcmp(url, \"ftp://\")) {\n@@ -938,14 +989,25 @@ int transport_push(struct transport *transport,\n \t\t   int refspec_nr, const char **refspec, int flags,\n \t\t   int *nonfastforward)\n {\n+\tint rc = 0;\n \t*nonfastforward = 0;\n \tverify_remote_names(refspec_nr, refspec);\n \n-\tif (transport->push)\n-\t\treturn transport->push(transport, refspec_nr, refspec, flags);\n-\tif (transport->push_refs) {\n+retry:\n+\tif (transport->push) {\n+\t\trc = transport->push(transport, refspec_nr, refspec, flags);\n+\t\tif(rc == TRANSPORT_LAYER6_READY) {\n+\t\t\tgit_take_over_transport(transport);\n+\t\t\tgoto retry;\n+\t\t}\n+\t\treturn rc;\n+\t} else if (transport->push_refs) {\n \t\tstruct ref *remote_refs =\n \t\t\ttransport->get_refs_list(transport, 1);\n+\t\tif(remote_refs == &special_transport_layer6_ready) {\n+\t\t\tgit_take_over_transport(transport);\n+\t\t\tgoto retry;\n+\t\t}\n \t\tstruct ref *local_refs = get_local_heads();\n \t\tint match_flags = MATCH_REFS_NONE;\n \t\tint verbose = flags & TRANSPORT_PUSH_VERBOSE;\n@@ -985,8 +1047,15 @@ int transport_push(struct transport *transport,\n \n const struct ref *transport_get_remote_refs(struct transport *transport)\n {\n-\tif (!transport->remote_refs)\n+\tif (!transport->remote_refs) {\n+retry:\n \t\ttransport->remote_refs = transport->get_refs_list(transport, 0);\n+\t\tif(transport->remote_refs == &special_transport_layer6_ready) {\n+\t\t\tgit_take_over_transport(transport);\n+\t\t\tgoto retry;\n+\t\t}\n+\t}\n+\n \treturn transport->remote_refs;\n }\n \n@@ -1020,7 +1089,13 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)\n \t\t\theads[nr_heads++] = rm;\n \t}\n \n+retry:\n \trc = transport->fetch(transport, nr_heads, heads);\n+\tif(rc == TRANSPORT_LAYER6_READY) {\n+\t\tgit_take_over_transport(transport);\n+\t\tgoto retry;\n+\t}\n+\n \tfree(heads);\n \treturn rc;\n }\ndiff --git a/transport.h b/transport.h\nindex 5949132..f3ee890 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@@ -79,6 +88,12 @@ struct transport {\n \tstruct git_transport_options* smart_options;\n };\n \n+/* Returned by get_refs_list, fetch or push methods of struct transport: Layer 6 is ready,\n+   take over the transport and retry operation. */\n+#define TRANSPORT_LAYER6_READY -42\n+extern struct ref special_transport_layer6_ready;\n+\n+\n #define TRANSPORT_PUSH_ALL 1\n #define TRANSPORT_PUSH_FORCE 2\n #define TRANSPORT_PUSH_DRY_RUN 4\n-- \n1.6.6.rc1.288.g40e67\n"},{"id":"129195","messageId":"1259942168-24869-7-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 4/8] Support remote helpers implementing smart transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:04Z","receivedAt":"2009-12-04T15:56:04Z","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 |   28 ++++++++++-\n transport-helper.c                   |   96 +++++++++++++++++++++++++++++++++-\n transport.c                          |   21 +++++++\n transport.h                          |    5 ++\n 4 files changed, 147 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 5cfdc0c..91cd9eb 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -90,6 +90,23 @@ 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. Note that to prevent deadlocking, all read data\n+\tshould be immediately flushed to outgoing connection (excepting\n+\tremote initial advertisments, which should be flushed on first\n+\tflush packet (0000 as length) encountered.\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@@ -123,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@@ -165,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 5d17fb5..21aa4ab 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -16,7 +16,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@@ -72,7 +73,10 @@ static struct child_process *get_helper(struct transport *transport)\n \thelper->argv = xcalloc(4, sizeof(*helper->argv));\n \tstrbuf_addf(&buf, \"remote-%s\", data->name);\n \thelper->argv[0] = strbuf_detach(&buf, NULL);\n-\thelper->argv[1] = transport->remote->name;\n+\tif(transport->remote)\n+\t\thelper->argv[1] = transport->remote->name;\n+\telse\n+\t\thelper->argv[1] = \"\";\n \thelper->argv[2] = remove_ext_force(transport->url);\n \thelper->git_cmd = 1;\n \tif (start_command(helper))\n@@ -113,6 +117,8 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\t\t   refspec_alloc);\n \t\t\trefspecs[refspec_nr++] = strdup(buf.buf + strlen(\"refspec \"));\n \t\t}\n+\t\tif (!strcmp(buf.buf, \"connect\"))\n+\t\t\tdata->connect = 1;\n \t}\n \tif (refspecs) {\n \t\tint i;\n@@ -331,12 +337,91 @@ 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(exec && 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+\t} else\n+\t\treturn 0;\n+\n+\twrite_in_full(helper->in, cmdbuf.buf, cmdbuf.len);\n+\tstrbuf_reset(&cmdbuf);\n+\tif (strbuf_getline(&cmdbuf, data->out, '\\n') == EOF)\n+\t\texit(128); /* child died, message supplied already */\n+\tif(!strcmp(cmdbuf.buf, \"\"))\n+\t\treturn 1;\n+\telse if(!strcmp(cmdbuf.buf, \"FALLBACK\"))\n+\t\treturn 0;\n+\telse\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 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 \tstruct helper_data *data = transport->data;\n \tint i, count;\n \n+\tif(process_connect(transport, 0))\n+\t\treturn TRANSPORT_LAYER6_READY;\n+\n \tcount = 0;\n \tfor (i = 0; i < nr_heads; i++)\n \t\tif (!(to_fetch[i]->status & REF_STATUS_UPTODATE))\n@@ -364,6 +449,9 @@ 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\treturn TRANSPORT_LAYER6_READY;\n+\n \tif (!remote_refs)\n \t\treturn 0;\n \n@@ -507,6 +595,9 @@ 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\treturn &special_transport_layer6_ready;\n+\n \tif (data->push && for_push)\n \t\twrite_str_in_full(helper->in, \"list for-push\\n\");\n \telse\n@@ -559,6 +650,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 7e6ef2b..1488cfe 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -762,6 +762,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@@ -926,6 +937,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@@ -1109,6 +1121,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+\t} else\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 f3ee890..c86329a 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, const char* executable,\n+\t\t       int fd[2]);\n \n \t/**\n \t * Disown the transport helper. Releases all resources used\n@@ -143,6 +145,9 @@ void transport_unlock_pack(struct transport *transport);\n int transport_disconnect(struct transport *transport);\n char *transport_anonymize_url(const char *url);\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.288.g40e67\n"},{"id":"129190","messageId":"1259942168-24869-8-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 5/8] Support remote archive from external protocol helpers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:05Z","receivedAt":"2009-12-04T15:56:05Z","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 1 files changed, 10 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 }\n-- \n1.6.6.rc1.288.g40e67\n"},{"id":"129188","messageId":"1259942168-24869-9-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 6/8] Remove special casing of http, https and ftp","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:06Z","receivedAt":"2009-12-04T15:56:06Z","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 1488cfe..872cc30 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -944,14 +944,6 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \n \t\tdata->conn = NULL;\n \t\tdata->virtual_connected = 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.288.g40e67\n"},{"id":"129191","messageId":"1259942168-24869-10-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 7/8] Add remote helper debug mode","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:07Z","receivedAt":"2009-12-04T15:56:07Z","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 |   96 ++++++++++++++++++++++++++++++++++++----------------\n 1 files changed, 67 insertions(+), 29 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 21aa4ab..03920bb 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@@ -24,6 +26,46 @@ struct helper_data\n \tstruct git_transport_options gitoptions;\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+\t}\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* helper_disown(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n@@ -95,14 +137,14 @@ static struct child_process *get_helper(struct transport *transport)\n \tdata->out = xfdopen(duped, \"r\");\n \tsetvbuf(data->out, NULL, _IONBF, 0);\n \n-\twrite_str_in_full(helper->in, \"capabilities\\n\");\n+\twrite_constant(helper->in, \"capabilities\\n\");\n \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@@ -130,14 +172,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\tclose(data->helper->out);\n \t\tfclose(data->out);\n@@ -166,10 +213,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@@ -192,12 +240,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@@ -257,13 +300,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@@ -298,12 +338,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@@ -313,7 +354,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@@ -364,10 +405,7 @@ static int _process_connect(struct transport *transport,\n \t} else\n \t\treturn 0;\n \n-\twrite_in_full(helper->in, cmdbuf.buf, cmdbuf.len);\n-\tstrbuf_reset(&cmdbuf);\n-\tif (strbuf_getline(&cmdbuf, data->out, '\\n') == EOF)\n-\t\texit(128); /* child died, message supplied already */\n+\txchgline(data, &cmdbuf);\n \tif(!strcmp(cmdbuf.buf, \"\"))\n \t\treturn 1;\n \telse if(!strcmp(cmdbuf.buf, \"FALLBACK\"))\n@@ -500,17 +538,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@@ -605,8 +640,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@@ -631,6 +665,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@@ -644,6 +679,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.288.g40e67\n"},{"id":"129192","messageId":"1259942168-24869-11-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21859","inReplyTo":"1259942168-24869-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH v2 8/8] Support mandatory capabilities","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-04T15:56:08Z","receivedAt":"2009-12-04T15:56:08Z","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                   |   29 ++++++++++++++++++++++-------\n 2 files changed, 26 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 91cd9eb..6af6824 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 03920bb..9ddcdc3 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -140,27 +140,42 @@ static struct child_process *get_helper(struct transport *transport)\n \twrite_constant(helper->in, \"capabilities\\n\");\n \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}\n-\t\tif (!strcmp(buf.buf, \"connect\"))\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+\t\t\t    capname);\n+\t\t}\n \t}\n \tif (refspecs) {\n \t\tint i;\n-- \n1.6.6.rc1.288.g40e67\n"},{"id":"129196","messageId":"fabb9a1e0912040801t6a0a10f7k2892e78285894cba@mail.gmail.com","threadId":"21859","inReplyTo":"1259942168-24869-4-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v2 1/8] Pass unknown protocols to external protocol handlers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-04T16:01:12Z","receivedAt":"2009-12-04T16:01:12Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Dec 4, 2009 at 16:56, Ilari Liusvaara\n<ilari.liusvaara@elisanet.fi> wrote:\n> Change URL handling to allow external protocol handlers to implement\n> new protocols without the '::' syntax if helper name does not conflict\n> with any built-in protocol.\n\nHaven't looked at the patch itself, but I like the idea, a lot.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"129197","messageId":"fabb9a1e0912040803q41b7d015ic9fb3014748a3fdc@mail.gmail.com","threadId":"21859","inReplyTo":"1259942168-24869-10-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v2 7/8] Add remote helper debug mode","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-04T16:03:40Z","receivedAt":"2009-12-04T16:03:40Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Dec 4, 2009 at 16:56, Ilari Liusvaara\n<ilari.liusvaara@elisanet.fi> wrote:\n> Remote helpers deadlock easily, so support debug mode which shows the\n> interaction steps.\n\nYou should move this to the beginning of the series, so that it can be\nmerged even if your other patches are not :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"129198","messageId":"fabb9a1e0912040805w609bbf15p1e514716edaba9c0@mail.gmail.com","threadId":"21859","inReplyTo":"1259942168-24869-9-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v2 6/8] Remove special casing of http, https and ftp","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-04T16:05:41Z","receivedAt":"2009-12-04T16:05:41Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Dec 4, 2009 at 16:56, Ilari Liusvaara\n<ilari.liusvaara@elisanet.fi> wrote:\n> HTTP, HTTPS and FTP are no longer special to transport code. Also\n> add support for FTPS (curl supports it so it is easy).\n\nAs I said earlier, I like this idea, again, without having looked at\nthe patch itself so I cannot comment on the implementation :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"129224","messageId":"20091204175545.GC1483@spearce.org","threadId":"21859","inReplyTo":"1259942168-24869-4-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v2 1/8] Pass unknown protocols to external protocol handlers","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-04T17:55:45Z","receivedAt":"2009-12-04T17:55:45Z","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> Change URL handling to allow external protocol handlers to implement\n> new protocols without the '::' syntax if helper name does not conflict\n> with any built-in protocol.\n> \n> foo:// now invokes git-remote-foo with foo:// URL.\n...\n\nUh, great, but...\n\n> @@ -30,6 +50,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...\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> +\tif((duped = dup(helper->out)) < 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\nThis is an entirely unrelated change.  Please split it into its\nown commit so its easier to review, test, blah blah blah.\n\n-- \nShawn.\n"},{"id":"129229","messageId":"20091204182753.GD1483@spearce.org","threadId":"21859","inReplyTo":"1259942168-24869-6-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v2 3/8] Support taking over transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-04T18:27:53Z","receivedAt":"2009-12-04T18:27:53Z","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> Add support for taking over transports that turn out to be smart.\n\nI really don't like this disown strategy and its magic ref return\nvalue from fetch.\n \n> @@ -1020,7 +1089,13 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)\n>  \t\t\theads[nr_heads++] = rm;\n>  \t}\n>  \n> +retry:\n>  \trc = transport->fetch(transport, nr_heads, heads);\n> +\tif(rc == TRANSPORT_LAYER6_READY) {\n> +\t\tgit_take_over_transport(transport);\n> +\t\tgoto retry;\n> +\t}\n\nWhy can't you expose git_take_over_transport as a public function\nand then the transport-helper.c code can instead do:\n\n\t... setup connect with helper ...\n\ttransport_takeover(transport, child);\n\treturn transport->fetch(....);\n\nWould this make the code simpler?\n\n-- \nShawn.\n"},{"id":"129230","messageId":"20091204183713.GE1483@spearce.org","threadId":"21859","inReplyTo":"1259942168-24869-7-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH v2 4/8] Support remote helpers implementing smart transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-04T18:37:13Z","receivedAt":"2009-12-04T18:37:13Z","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>  \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\nWhy not 'fallback' to remain consistent with this protocol and many\nothers in git where we stick to lowercase ASCII?\n\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. Note that to prevent deadlocking, all read data\n> +\tshould be immediately flushed to outgoing connection (excepting\n> +\tremote initial advertisments, which should be flushed on first\n> +\tflush packet (0000 as length) encountered.\n\nWhy is the initial advertisement special?  If the helper always\nflushes both sides, it shouldn't ever deadlock the protocol.  Also,\nnote that a helper should be able to implement a tiny delay like\nNagle's algorithm does in TCP.  It just can't sit on a byte forever.\n\n> @@ -72,7 +73,10 @@ static struct child_process *get_helper(struct transport *transport)\n>  \thelper->argv = xcalloc(4, sizeof(*helper->argv));\n>  \tstrbuf_addf(&buf, \"remote-%s\", data->name);\n>  \thelper->argv[0] = strbuf_detach(&buf, NULL);\n> -\thelper->argv[1] = transport->remote->name;\n> +\tif(transport->remote)\n> +\t\thelper->argv[1] = transport->remote->name;\n> +\telse\n> +\t\thelper->argv[1] = \"\";\n\nThis hunk appears to be unrelated.  And actually, if transport has\nno remote, shouldn't the arg here be NULL so the helper gets only\n1 argument and not 2 arguments?\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(exec && 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\nI think exec winds up defaulting to name if --upload-pack was not\nused on the command line, and remote.$name.uploadpack was not set.\nSee transport.c where you initialize the git options struct, these\nfields were defaulted in.\n\nMy point is, we shouldn't send option servpath to the helper if\nname is equal to servpath, because the helper might not support\nservpath and the option command will issue a warning above for no\nreason at all.\n\n-- \nShawn.\n"},{"id":"129263","messageId":"20091205131630.GA1684@Knoppix","threadId":"21859","inReplyTo":"20091204183713.GE1483@spearce.org","subject":"Re: [RFC PATCH v2 4/8] Support remote helpers implementing smart transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-05T13:16:30Z","receivedAt":"2009-12-05T13:16:30Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Fri, Dec 04, 2009 at 10:37:13AM -0800, Shawn O. Pearce wrote:\n> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:\n> \n> Why not 'fallback' to remain consistent with this protocol and many\n> others in git where we stick to lowercase ASCII?\n\nChanged.\n \n> Why is the initial advertisement special?  If the helper always\n> flushes both sides, it shouldn't ever deadlock the protocol.  Also,\n> note that a helper should be able to implement a tiny delay like\n> Nagle's algorithm does in TCP.  It just can't sit on a byte forever.\n\nThat's just extremely badly worded. The point is that only place where\nbuffering has even seemed useful is initial adverts. Those can\nreally trigger large numbers of small transfers if unbuffered.\n\nGranted, small delay should work quite well.\n\nI'll reword it.\n\n> This hunk appears to be unrelated.  And actually, if transport has\n> no remote, shouldn't the arg here be NULL so the helper gets only\n> 1 argument and not 2 arguments?\n\nActually, it can't be NULL (the code would have already die()'d in\nthat case). Fixed.\n\n> I think exec winds up defaulting to name if --upload-pack was not\n> used on the command line, and remote.$name.uploadpack was not set.\n> See transport.c where you initialize the git options struct, these\n> fields were defaulted in.\n\nActually, there was bug in initializing git options struct. It was\nproperly initialized if one used foo://, but not with foo::bar://.\nTripping it caused NULL exec to be passed if you didn't override.\nI fixed that bug and now the servpath code has just strcmp.\n\n> My point is, we shouldn't send option servpath to the helper if\n> name is equal to servpath, because the helper might not support\n> servpath and the option command will issue a warning above for no\n> reason at all.\n\nThis is exactly what happens (and yes, I tested). No --receive-pack\ndoesn't trip warning, with --receive-pack it does.\n\n-Ilari\n"},{"id":"129264","messageId":"20091205131743.GB1684@Knoppix","threadId":"21859","inReplyTo":"20091204175545.GC1483@spearce.org","subject":"Re: [RFC PATCH v2 1/8] Pass unknown protocols to external protocol handlers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-05T13:17:44Z","receivedAt":"2009-12-05T13:17:44Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Fri, Dec 04, 2009 at 09:55:45AM -0800, Shawn O. Pearce wrote:\n\n<fd dupping code>\n \n> This is an entirely unrelated change.  Please split it into its\n> own commit so its easier to review, test, blah blah blah.\n\nMoved to 3/8, since that's where it actually will be needed (latter\nrearrangement renumbered that patch to 5/8).\n\n-Ilari\n"},{"id":"129265","messageId":"20091205131814.GC1684@Knoppix","threadId":"21859","inReplyTo":"20091204182753.GD1483@spearce.org","subject":"Re: [RFC PATCH v2 3/8] Support taking over transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-05T13:18:14Z","receivedAt":"2009-12-05T13:18:14Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Fri, Dec 04, 2009 at 10:27:53AM -0800, Shawn O. Pearce wrote:\n> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:\n> > Add support for taking over transports that turn out to be smart.\n> \n> Why can't you expose git_take_over_transport as a public function\n> and then the transport-helper.c code can instead do:\n> \n> \t... setup connect with helper ...\n> \ttransport_takeover(transport, child);\n> \treturn transport->fetch(....);\n> \n> Would this make the code simpler?\n\nThe code complexity difference is not big. The magic return was for\nhistoric reasons that no longer apply (and didn't get fixed when\ncorresponding upstream changes happned).\n\nNamely, the push/push_refs impedance mismatch. Helpers used push,\nsmart transports used push_refs. But now that both use push_refs,\nthe handler can be invoked directly from push_refs of helper.\n\nFixed.\n\n-Ilari\n"}]}