{"thread":{"id":"21807","subject":"[RFC PATCH 0/8] Git remote helpers to implement smart transports.","startedAt":"2009-12-01T13:57:10Z","lastAt":"2009-12-03T19:42:42Z","messageCount":42,"participants":["Ilari Liusvaara","Sverre Rabbelier","Shawn O. Pearce","Daniel Barkalow","Junio C Hamano","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"128871","messageId":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":null,"subject":"[RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:10Z","receivedAt":"2009-12-01T13:57:10Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"This series implements extensions to remote helpers for carrying smary\ntransports. It is against next, because master doesn't contain necressary\npatches (the allow specifying remote helper in url one).\n\nFirst patch reworks URL handling so that unknown protocols are passed\nto remote helpers. This allows having remote helpers implement git\ntransports without duplicating the protocol part.\n\nSecond patch refactors git transport option parsing to split smart\ntransport option to own structure and keep this structure up to date\nwith encountered options. This is needed if transport turns out to\nbe smart transport.\n\nThird patch adds capabilty to have git smart transport code take\nover connection, replacing \"layer 7\" with git smart transport protocols.\n\nFourth patch actually adds the extensions to external transport code to\nallow helpers signal that transport should be taken over (become smart\ntransport).\n\nFifth patch extends 'git archive' to allow snapshotting off any transport\nthat uses git smart transport code, not just file://, git:// and ssh://\n\nSixth patch removes special casing of http, https and ftp. And while\nat it, adds ftps, since CURL supports it.\n\nSeventh patch adds debug mode for remote helpers. Might be useful for\ndebugging deadlocks by showing command traffic between git executable\nand remote helper.\n\nEighth patch adds support for remote helper to signal that it requires\nsome capability and have git complain if it doesn't know it.\n\n\nMisc remarks:\n\nUnderlying network link is assumed to be full-duplex since most of the\ntime if the underlying link isn't HTTP, it will be full-duplex (most of the\ntime even TCP).\n\nSimplest deadlock-free buffering is just to read incoming pipe from git\nwhen there's no data to send to remote end. This gives adequate performance\nin all cases except when sending large initial ref adverts (and those are \nended by flush anyway, so those can be safely buffered). So no extensions\nto add missing flushes are needed.\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 | 5 +-\n Documentation/git-remote-helpers.txt | 35 ++++-\n Makefile | 16 ++-\n builtin-archive.c | 17 ++-\n transport-helper.c | 270 ++++++++++++++++++++++++++++-----\n transport.c | 258 ++++++++++++++++++++++++++------\n transport.h | 32 ++++\n 7 files changed, 533 insertions(+), 100 deletions(-)\n"},{"id":"128872","messageId":"1259675838-14692-2-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 1/8] Pass unknown protocols to external protocol handlers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:11Z","receivedAt":"2009-12-01T13:57:11Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Change URL handling to allow external protocol handlers to provde\nnew protocols without the '::' syntax if helper name does not conflict\nwith any builtin protocol.\n\nfoo:// now invokes git-remote-foo with foo:// URL.\n\nSigned-off-by: Ilari Liusvaara \n---\n transport-helper.c | 57 ++++++++++++++++++++++++++++++++----\n transport.c | 82 +++++++++++++++++++++++++++++++++++++++++++++-------\n 2 files changed, 122 insertions(+), 17 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 6182413..a499751 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,8 @@ 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+\tint seen_line = 0;\n \n \tif (data->helper)\n \t\treturn data->helper;\n@@ -39,21 +61,43 @@ static struct child_process *get_helper(struct transport *transport)\n \thelper->out = -1;\n \thelper->err = 0;\n \thelper->argv = xcalloc(4, sizeof(*helper->argv));\n-\tstrbuf_addf(&buf, \"remote-%s\", data->name);\n+\t/* We use the dashed form because git \n+\t would run and print totally inapporiate error message. */\n+\tstrbuf_addf(&buf, \"git-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->git_cmd = 1;\n+\thelper->argv[2] = remove_ext_force(transport->url);\n+\thelper->git_cmd = 0;\n \tif (start_command(helper))\n-\t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\n+\t\tdie(\"Unable to run helper: %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\tif (strbuf_getline(&buf, data->out, '\\n') == EOF) {\n+\t\t\t/* If we haven't seen line yet, try to finish the\n+\t\t\t command so we get error message about failed\n+\t\t\t execution. */\n+\t\t\tif(!seen_line)\n+\t\t\t\tfinish_command(helper);\n+\n \t\t\texit(128); /* child died, message supplied already */\n+\t\t}\n+\n+\t\tseen_line = 1;\n \n \t\tif (!*buf.buf)\n \t\t\tbreak;\n@@ -91,6 +135,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..148b9e1 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+#else\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.rc0.64.g5593e\n"},{"id":"128873","messageId":"1259675838-14692-3-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 2/8] Refactor git transport options parsing","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:12Z","receivedAt":"2009-12-01T13:57:12Z","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 \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 148b9e1..7956892 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.rc0.64.g5593e\n"},{"id":"128878","messageId":"1259675838-14692-4-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 3/8] Support taking over transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:13Z","receivedAt":"2009-12-01T13:57:13Z","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 \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 a499751..777ecbb 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@@ -562,5 +572,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 7956892..09e4c97 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.rc0.64.g5593e\n"},{"id":"128876","messageId":"1259675838-14692-5-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 4/8] Support remote helpers implementing smart transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:14Z","receivedAt":"2009-12-01T13:57:14Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Signed-off-by: Ilari Liusvaara \n---\n Documentation/git-remote-helpers.txt | 30 ++++++++++-\n transport-helper.c | 94 +++++++++++++++++++++++++++++++--\n transport.c | 21 ++++++++\n transport.h | 5 ++\n 4 files changed, 143 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 5cfdc0c..adf815c 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -90,6 +90,28 @@ Supported if the helper has the \"push\" capability.\n +\n Supported if the helper has the \"import\" capability.\n \n+'connect-r' ::\n+\tConnects to given service. Stdin and stdout of helper are\n+\tconnected to specified service (no git or git- prefixes are used,\n+\tso e.g. fetching uses 'upload-pack' as service) on remote side.\n+\tValid replies to this command are 'OK' (connection established),\n+\t'FALLBACK' (no smart transport support, fall back to dumb\n+\ttransports) and 'ERROR' (can't connect, don't bother trying to\n+\tfall back). After line feed terminating the OK response, the\n+\toutput of 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.\n++\n+Supported if the helper has the \"connect-r\" capability. Not used if\n+helper has the \"invoke-r\" capability, as invoke is preferred to connect.\n+\n+'invoke-r' ::\n+\tLike connect-r command, but instead of service name, command\n+\tline is given. The length of command field is given in command\n+\tlength field.\n++\n+Supported if the helper has the \"invoke-r\" 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 +145,12 @@ 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-r'::\n+\tThis helper supports the 'connect-r' command.\n+\n+'invoke-r'::\n+\tThis helper supports the 'invoke-r' command.\n+\n REF LIST ATTRIBUTES\n -------------------\n \n@@ -167,7 +195,7 @@ OPTIONS\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 777ecbb..0e4da79 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -16,7 +16,9 @@ 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_r : 1,\n+\t\tinvoke_r : 1;\n \t/* These go from remote name (as in \"list\") to private name */\n \tstruct refspec *refspecs;\n \tint refspec_nr;\n@@ -75,7 +77,10 @@ static struct child_process *get_helper(struct transport *transport)\n \t would run and print totally inapporiate error message. */\n \tstrbuf_addf(&buf, \"git-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 = 0;\n \tif (start_command(helper))\n@@ -125,6 +130,10 @@ 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-r\"))\n+\t\t\tdata->connect_r = 1;\n+\t\tif (!strcmp(buf.buf, \"invoke-r\"))\n+\t\t\tdata->invoke_r = 1;\n \t}\n \tif (refspecs) {\n \t\tint i;\n@@ -344,12 +353,83 @@ static int fetch_with_import(struct transport *transport,\n \treturn 0;\n }\n \n+static int _process_connect_or_invoke(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+\n+\thelper = get_helper(transport);\n+\n+\tif(data->invoke_r) {\n+\t\tstrbuf_addf(&cmdbuf, \"invoke-r %i %s\\n\",\n+\t\t\t strlen(exec), exec);\n+\t} else if(data->connect_r) {\n+\t\tstrbuf_addf(&cmdbuf, \"connect-r %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, \"OK\"))\n+\t\treturn 1;\n+\telse if(!strcmp(cmdbuf.buf, \"FALLBACK\"))\n+\t\treturn 0;\n+\telse if(!strcmp(cmdbuf.buf, \"ERROR\"))\n+\t\texit(128); /* Error already suppiled. */\n+\telse\n+\t\tdie(\"Unknown response to invoke/connect: %s\",\n+\t\t\tcmdbuf.buf);\n+\n+\treturn 0;\t/* Shouldn't be here. */\n+}\n+\n+static int process_connect_or_invoke(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 ? \"receive-pack\" : \"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_or_invoke(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 invoke_r and connect_r are inited. */\n+\tget_helper(transport);\n+\tif(!data->invoke_r && !data->connect_r)\n+\t\tdie(\"Operation not supported by protocol.\");\n+\n+\tif(!_process_connect_or_invoke(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_or_invoke(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@@ -377,6 +457,9 @@ static int push_refs(struct transport *transport,\n \tstruct child_process *helper;\n \tstruct ref *ref;\n \n+\tif(process_connect_or_invoke(transport, 1))\n+\t\treturn TRANSPORT_LAYER6_READY;\n+\n \tif (!remote_refs)\n \t\treturn 0;\n \n@@ -520,10 +603,8 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \n \thelper = get_helper(transport);\n \n-\tif (data->push && for_push)\n-\t\twrite_str_in_full(helper->in, \"list for-push\\n\");\n-\telse\n-\t\twrite_str_in_full(helper->in, \"list\\n\");\n+\tif(process_connect_or_invoke(transport, for_push))\n+\t\treturn &special_transport_layer6_ready;\n \n \twhile (1) {\n \t\tchar *eov, *eon;\n@@ -572,6 +653,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 09e4c97..a32f405 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.rc0.64.g5593e\n"},{"id":"128874","messageId":"1259675838-14692-6-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 5/8] Support remote archive from external protocol helpers","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:15Z","receivedAt":"2009-12-01T13:57:15Z","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 \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..3c053b4 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, \"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.rc0.64.g5593e\n"},{"id":"128879","messageId":"1259675838-14692-7-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 6/8] Remove special casing of http, https and ftp","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:16Z","receivedAt":"2009-12-01T13:57:16Z","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 \n---\n .gitignore | 5 ++++-\n Makefile | 16 ++++++++++++++--\n transport.c | 8 --------\n 3 files changed, 18 insertions(+), 11 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 7cc54b4..65508ea 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -106,7 +106,10 @@\n /git-reflog\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..be0be87 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1097,7 +1097,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 +1676,19 @@ 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+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+git-remote-https$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+git-remote-ftp$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+git-remote-ftps$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 \ndiff --git a/transport.c b/transport.c\nindex a32f405..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-#else\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.rc0.64.g5593e\n"},{"id":"128875","messageId":"1259675838-14692-8-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 7/8] Add remote helper debug mode","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:17Z","receivedAt":"2009-12-01T13:57:17Z","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 \n---\n transport-helper.c | 110 ++++++++++++++++++++++++++++++++++------------------\n 1 files changed, 72 insertions(+), 38 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 0e4da79..697f026 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@@ -25,6 +27,47 @@ 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+\t\t int supress_finish)\n+{\n+\tstrbuf_reset(buffer);\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\tif(!supress_finish)\n+\t\t\tfinish_command(helper->helper);\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, 1);\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@@ -99,23 +142,15 @@ 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\t/* If we haven't seen line yet, try to finish the\n-\t\t\t command so we get error message about failed\n-\t\t\t execution. */\n-\t\t\tif(!seen_line)\n-\t\t\t\tfinish_command(helper);\n-\n-\t\t\texit(128); /* child died, message supplied already */\n-\t\t}\n-\n+\t\trecvline(data, &buf, seen_line);\n \t\tseen_line = 1;\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@@ -145,14 +180,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@@ -182,10 +222,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@@ -208,12 +249,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@@ -273,13 +309,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, 1);\n \n \t\tif (!prefixcmp(buf.buf, \"lock \")) {\n \t\t\tconst char *name = buf.buf + 5;\n@@ -314,12 +347,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@@ -329,7 +363,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@@ -370,12 +404,12 @@ static int _process_connect_or_invoke(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-\tif(!strcmp(cmdbuf.buf, \"OK\"))\n+\txchgline(data, &cmdbuf);\n+\tif(!strcmp(cmdbuf.buf, \"OK\")) {\n+\t\tif(debug) fprintf(stderr, \"Debug: Layer 6 link ready, \"\n+\t\t\t\t \"starting layer 7...\\n\");\n \t\treturn 1;\n+\t}\n \telse if(!strcmp(cmdbuf.buf, \"FALLBACK\"))\n \t\treturn 0;\n \telse if(!strcmp(cmdbuf.buf, \"ERROR\"))\n@@ -508,17 +542,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, 1);\n \t\tif (!buf.len)\n \t\t\tbreak;\n \n@@ -608,8 +639,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, 1);\n \n \t\tif (!*buf.buf)\n \t\t\tbreak;\n@@ -634,6 +664,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@@ -647,6 +678,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.rc0.64.g5593e\n"},{"id":"128877","messageId":"1259675838-14692-9-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"[RFC PATCH 8/8] Support mandatory capabilities","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T13:57:18Z","receivedAt":"2009-12-01T13:57:18Z","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 \n---\n Documentation/git-remote-helpers.txt | 5 ++++-\n transport-helper.c | 31 +++++++++++++++++++++++--------\n 2 files changed, 27 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex adf815c..eab9c03 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 \" \ndiff --git a/transport-helper.c b/transport-helper.c\nindex 697f026..a128560 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -145,30 +145,45 @@ 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, seen_line);\n \t\tseen_line = 1;\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-r\"))\n+\t\telse if (!strcmp(capname, \"connect-r\"))\n \t\t\tdata->connect_r = 1;\n-\t\tif (!strcmp(buf.buf, \"invoke-r\"))\n+\t\telse if (!strcmp(capname, \"invoke-r\"))\n \t\t\tdata->invoke_r = 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.rc0.64.g5593e\n"},{"id":"128887","messageId":"fabb9a1e0912010812t4de8027dj1faf828051d1adc2@mail.gmail.com","threadId":"21807","inReplyTo":"1259675838-14692-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-01T16:12:16Z","receivedAt":"2009-12-01T16:12:16Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Dec 1, 2009 at 14:57, Ilari Liusvaara\n wrote:\n> This series implements extensions to remote helpers for carrying smary\n> transports. It is against next, because master doesn't contain necressary\n> patches (the allow specifying remote helper in url one).\n\nCould you please explain how this relates to Shawn's smart http series\nand the sr/vcs-helper series?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"128894","messageId":"20091201165245.GF21299@spearce.org","threadId":"21807","inReplyTo":"fabb9a1e0912010812t4de8027dj1faf828051d1adc2@mail.gmail.com","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T16:52:45Z","receivedAt":"2009-12-01T16:52:45Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier wrote:\n> On Tue, Dec 1, 2009 at 14:57, Ilari Liusvaara\n> wrote:\n> > This series implements extensions to remote helpers for carrying smary\n> > transports. It is against next, because master doesn't contain necressary\n> > patches (the allow specifying remote helper in url one).\n> \n> Could you please explain how this relates to Shawn's smart http series\n> and the sr/vcs-helper series?\n\nOr better, why this is even necessary?\n\nI thought git:// over TCP is pretty simple and efficient, and fairly\nwidely deployed. Smart http(s):// will be in 1.6.6 and available\nsoon, and isn't all that ugly.\n\nSince the introduction of git:// nobody has asked for another\nprotocol... other than wanting to make http:// as efficient as\ngit:// is. Which is now done.\n\nSo why do we need this?\n\nThe sr/vcs-helper series makes sense if you want to make SVN, Hg,\nor P4 remotes act transparently like Git remotes. But that's not\nembedding the git:// protocol inside of another protocol, its doing a\nfull up conversion from a non-Git set of semantics to Git semantics.\n\n-- \nShawn.\n"},{"id":"128899","messageId":"20091201171908.GA15436@Knoppix","threadId":"21807","inReplyTo":"20091201165245.GF21299@spearce.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T17:19:08Z","receivedAt":"2009-12-01T17:19:08Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Dec 01, 2009 at 08:52:45AM -0800, Shawn O. Pearce wrote:\n> Sverre Rabbelier wrote:\n> > On Tue, Dec 1, 2009 at 14:57, Ilari Liusvaara\n> > wrote:\n> > > This series implements extensions to remote helpers for carrying smary\n> > > transports. It is against next, because master doesn't contain necressary\n> > > patches (the allow specifying remote helper in url one).\n> > \n> > Could you please explain how this relates to Shawn's smart http series\n> > and the sr/vcs-helper series?\n\nAbility to easily implement smart transports with underlying full-duplex\nconnection. The smart http stuff has loads of code to reimplement smart\ntransport client side.\n\n> Or better, why this is even necessary?\n> \n> I thought git:// over TCP is pretty simple and efficient, and fairly\n> widely deployed. Smart http(s):// will be in 1.6.6 and available\n> soon, and isn't all that ugly.\n\nI consider the authentication parts of smart http pretty ugly. TLS has some\nnicer methods, but support for those is nonexistent. Also, I consider piggy-\nbacking on HTTP when you can have full-duplex connectivity ugly.\n \n> Since the introduction of git:// nobody has asked for another\n> protocol... other than wanting to make http:// as efficient as\n> git:// is. Which is now done.\n\nIncorrect. I have seen requests for gits:// (and in fact, I have plans to\nimplement that protocol).\n\n> So why do we need this?\n\nFor instance, to support new types of authentication for smart transports\nwithout patching client git binaries (SSH has lots of failure modes that\nare quite nasty to debug) or abusing GIT_PROXY (yuck). \n\nIf the server can also handle authentication, it has a lot better idea\nwhere things go wrong and can give better errors to logs or to client\n(of course, not too much can be leaked to client or it will be too useful\nfor attack, but that's seperate topic).\n\n> The sr/vcs-helper series makes sense if you want to make SVN, Hg,\n> or P4 remotes act transparently like Git remotes. But that's not\n> embedding the git:// protocol inside of another protocol, its doing a\n> full up conversion from a non-Git set of semantics to Git semantics.\n\nThis is not about embedding git:// protol inside another. Its about\ncarrying the subprotocols. These transports share with git:// as much\nas file:// and ssh:// share with git:// (note that service request is\ngiven as command, not inside data pipe).\n\nAnd IIRC, the only thing this needs from sr/vcs-helper is the patch to\nallow selecting helper with URL. The first versions of series did contain\nself-standing functionality equivalent to that, but that got dropped as\nequivalent functionality appeared in upstream.\n\n-Ilari\n"},{"id":"128910","messageId":"20091201182414.GK21299@spearce.org","threadId":"21807","inReplyTo":"1259675838-14692-7-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH 6/8] Remove special casing of http, https and ftp","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T18:24:14Z","receivedAt":"2009-12-01T18:24:14Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara 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...\n> diff --git a/Makefile b/Makefile\n> index 42744a4..be0be87 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1676,7 +1676,19 @@ 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> +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> +git-remote-https$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> +git-remote-ftp$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> +git-remote-ftps$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\nThese should all be hardlinks to a single executable, not duplicate\nrelinks of the same object files.\n \n-- \nShawn.\n"},{"id":"128917","messageId":"alpine.LNX.2.00.0912011351220.14365@iabervon.org","threadId":"21807","inReplyTo":"1259675838-14692-7-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH 6/8] Remove special casing of http, https and ftp","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-12-01T19:15:17Z","receivedAt":"2009-12-01T19:15:17Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 1 Dec 2009, Ilari Liusvaara wrote:\n\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\nWe've been through this extensively, and settled on having a special case \nfor URLs that specify a pure location. That is, the distinction between \nhttp and ftp is at the level of how you get to the content for that \nlocation, not what you do to interact with it. (Even with webdav or the \ngit-specific smart server support, we use the same detection method on all \nlocations, and ftp simply never has the possibility of having these \nfeatures detected.)\n\nIt would be fine to add \"ftps\" to the list of URL schemes that indicate a \npure location, except that it's plausible that ftps supports writing, but \nobviously not by webdav, which is what the push support via curl will \nattempt, so it's more likely to be confusing than helpful.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"128919","messageId":"20091201192233.GL21299@spearce.org","threadId":"21807","inReplyTo":"1259675838-14692-5-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T19:22:33Z","receivedAt":"2009-12-01T19:22:33Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara wrote:\n> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\n> index 5cfdc0c..adf815c 100644\n> --- a/Documentation/git-remote-helpers.txt\n> +++ b/Documentation/git-remote-helpers.txt\n> @@ -90,6 +90,28 @@ Supported if the helper has the \"push\" capability.\n> +\n> Supported if the helper has the \"import\" capability.\n> \n> +'connect-r' ::\n> +\tConnects to given service. Stdin and stdout of helper are\n> +\tconnected to specified service (no git or git- prefixes are used,\n> +\tso e.g. fetching uses 'upload-pack' as service) on remote side.\n\nThis flies against every other convention we have. git:// uses the\nstring 'git-upload-pack' and 'git-receive-pack', and so does the\nsmart-http code. We should continue to use the git- prefix here,\nto be consistent, even though by context its clearly implied.\n\n> +\tValid replies to this command are 'OK' (connection established),\n\nWhy 'OK'? Currently remote-helpers return an empty blank line\nto any successful command, not 'OK'.\n\n> +\t'FALLBACK' (no smart transport support, fall back to dumb\n> +\ttransports) and 'ERROR' (can't connect, don't bother trying to\n> +\tfall back).\n\nFALLBACK almost makes sense, but ERROR we don't do in the\nthe existing helper protocol. Instead the helper simply\nprints its error message(s) to stderr and does exit(128).\naka what die() does.\n\n> +Supported if the helper has the \"connect-r\" capability. Not used if\n> +helper has the \"invoke-r\" capability, as invoke is preferred to connect.\n> +\n> +'invoke-r' ::\n> +\tLike connect-r command, but instead of service name, command\n> +\tline is given. The length of command field is given in command\n> +\tlength field.\n> ++\n> +Supported if the helper has the \"invoke-r\" capability.\n\nWhy both connect-r and invoke-r? Why isn't connect-r sufficient\nhere? Isn't it sufficient for any service that runs over git:// ?\n\n-- \nShawn.\n"},{"id":"128921","messageId":"20091201193009.GM21299@spearce.org","threadId":"21807","inReplyTo":"20091201171908.GA15436@Knoppix","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T19:30:09Z","receivedAt":"2009-12-01T19:30:09Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara wrote:\n> On Tue, Dec 01, 2009 at 08:52:45AM -0800, Shawn O. Pearce wrote:\n> \n> > Or better, why this is even necessary?\n> \n> I have seen requests for gits:// (and in fact, I have plans to\n> implement that protocol).\n> \n> For instance, to support new types of authentication for smart transports\n> without patching client git binaries (SSH has lots of failure modes that\n> are quite nasty to debug) or abusing GIT_PROXY (yuck). \n\nSo the bulk of this series is about making a proxy for git://\neasier to tie into git?\n\nForgive me if I sound stupid, but for gits:// shouldn't that just\nbe a matter of git_connect() forking a git-remote-gits process\nlinked against openssl? Or, maybe it just runs `openssl s_client`?\n\nWhy go through all of this effort of making a really generic proxy\nprotocol system when the long-term plan is to just ship native\ngits:// support as part of git-core?\n \n-- \nShawn.\n"},{"id":"128922","messageId":"20091201193943.GA15571@Knoppix","threadId":"21807","inReplyTo":"20091201182414.GK21299@spearce.org","subject":"Re: [RFC PATCH 6/8] Remove special casing of http, https and ftp","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-01T19:39:44Z","receivedAt":"2009-12-01T19:39:44Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Dec 01, 2009 at 10:24:14AM -0800, Shawn O. Pearce wrote:\n> Ilari Liusvaara wrote:\n> \n> These should all be hardlinks to a single executable, not duplicate\n> relinks of the same object files.\n\nFixed for next round (probably send that out in few days).\n\n-Ilari\n"},{"id":"128923","messageId":"7vskbuwhmy.fsf@alter.siamese.dyndns.org","threadId":"21807","inReplyTo":"20091201193009.GM21299@spearce.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-01T20:42:29Z","receivedAt":"2009-12-01T20:42:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Ilari Liusvaara wrote:\n>> \n>> For instance, to support new types of authentication for smart transports\n>> without patching client git binaries (SSH has lots of failure modes that\n>> are quite nasty to debug) or abusing GIT_PROXY (yuck). \n>\n> So the bulk of this series is about making a proxy for git://\n> easier to tie into git?\n>\n> Forgive me if I sound stupid, but for gits:// shouldn't that just\n> be a matter of git_connect() forking a git-remote-gits process\n> linked against openssl? Or, maybe it just runs `openssl s_client`?\n>\n> Why go through all of this effort of making a really generic proxy\n> protocol system when the long-term plan is to just ship native\n> gits:// support as part of git-core?\n\nI didn't know what the long-term plan was to be honest, but after skimming\nthe series, I think your response is a good summary.\n\nIt is somewhat unfortunate that a few changes I liked (e.g. the \"debug\"\nbit), even though it was somewhat painful to read them due to coding style\ndifferences, were not at the beginning of the series but instead buried\nafter changes that are much bigger and controversial (e.g. [6/8]).\n"},{"id":"128936","messageId":"20091201232056.GA29632@spearce.org","threadId":"21807","inReplyTo":"7vskbuwhmy.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T23:20:56Z","receivedAt":"2009-12-01T23:20:56Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> It is somewhat unfortunate that a few changes I liked (e.g. the \"debug\"\n> bit), even though it was somewhat painful to read them due to coding style\n> differences, were not at the beginning of the series but instead buried\n> after changes that are much bigger and controversial (e.g. [6/8]).\n\nI agree about that debug patch, I actually thought that was\ninteresting, and wish I had done more of that sort of work during\nsmart-http. It would have helped me to debug it in the early stages.\n\n-- \nShawn.\n"},{"id":"128967","messageId":"20091202055048.GA31244@Knoppix","threadId":"21807","inReplyTo":"20091201193009.GM21299@spearce.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T05:50:48Z","receivedAt":"2009-12-02T05:50:48Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Dec 01, 2009 at 11:30:09AM -0800, Shawn O. Pearce wrote:\n> Ilari Liusvaara wrote:\n> > \n> > For instance, to support new types of authentication for smart transports\n> > without patching client git binaries (SSH has lots of failure modes that\n> > are quite nasty to debug) or abusing GIT_PROXY (yuck). \n> \n> So the bulk of this series is about making a proxy for git://\n> easier to tie into git?\n\nThis is making the \"layer 5/6\" parts of git:// easier to replace, for whatever\nreason that replacement may be desired (and the lower layer is just assumed to\nbe some kind of full-duplex link).\n\nThe part about abusing GIT_PROXY is _very_ nasty hack to be able to layer 6\ngateway git smart transports.\n\nThe git:// protocol stack is: \n- Git smart transport subprotocols (upload-pack, upload-archive and receive-pack)\n- git:// (request signaling and data passing)\n- TCP/IP (or comparable)\n\nAnd ssh://:\n\n- Git smart transport subprotocols (upload-pack, upload-archive and receive-pack)\n- SSH (request signaling, data passing, encrypt & auth).\n- TCP/IP (or comparable)\n\nSmart-HTTP:\n\n- RPC versions of git smart transport subprotocols\n- HTTP\n- TLS (optional)\n- TCP/IP (or comparable)\n\nThis is about:\n\n- Git smart transport subprotocols (upload-pack, upload-archive and receive-pack)\n- Some prtocol layer(s) (request signaling, data passing, maybe encrypt & auth, etc...)\n- TCP/IP (or comparable)\n\n> Forgive me if I sound stupid, but for gits:// shouldn't that just\n> be a matter of git_connect() forking a git-remote-gits process\n> linked against openssl? Or, maybe it just runs `openssl s_client`?\n\ngits:// was just an example. There can be other interesting stuff (I don't\neven pretend my imagination is the limit). And I would rather link the gits://\nhandler to GnuTLS than OpenSSL, but that's seperate matter...\n\nAs for \"other interesting stuff\": Smart transport using Kerberos auth (just\nthrowing ideas, probably not going to implement that)?\n\n> Why go through all of this effort of making a really generic proxy\n> protocol system when the long-term plan is to just ship native\n> gits:// support as part of git-core?\n\ngits:// is not actual goal of this series. Its just something to build on\ntop of it.\n\n-Ilari\n"},{"id":"128968","messageId":"20091202055253.GB31244@Knoppix","threadId":"21807","inReplyTo":"alpine.LNX.2.00.0912011351220.14365@iabervon.org","subject":"Re: [RFC PATCH 6/8] Remove special casing of http, https and ftp","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T05:52:53Z","receivedAt":"2009-12-02T05:52:53Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Dec 01, 2009 at 02:15:17PM -0500, Daniel Barkalow wrote:\n> On Tue, 1 Dec 2009, Ilari Liusvaara wrote:\n> \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> \n> We've been through this extensively, and settled on having a special case \n> for URLs that specify a pure location. That is, the distinction between \n> http and ftp is at the level of how you get to the content for that \n> location, not what you do to interact with it. (Even with webdav or the \n> git-specific smart server support, we use the same detection method on all \n> locations, and ftp simply never has the possibility of having these \n> features detected.)\n\nCurrently the only thing about http:// and co git main executable knows is\nto pass them to curl remote helper (and print error if compiled with NO_CURL,\npossibly causing problems with version desync). Git main executable does\nnot know any difference between say http:// and ftp:// (the remote helper must\nobiviously know the difference, but remote helper is not part of git main\nexecutable).\n\n> It would be fine to add \"ftps\" to the list of URL schemes that indicate a \n> pure location, except that it's plausible that ftps supports writing, but \n> obviously not by webdav, which is what the push support via curl will \n> attempt, so it's more likely to be confusing than helpful.\n\nremote-curl.c code doesn't seem to do anything stupid with ftps:// that it\nwouldn't try with ftp://, and trying to push counts as \"stupid\" here (and\nremember that many FTP servers do allow unencrypted uploads, especially with\nauthentication and CURL can AFAIK handle that).\n\n-Ilari\n"},{"id":"128969","messageId":"20091202055543.GC31244@Knoppix","threadId":"21807","inReplyTo":"20091201192233.GL21299@spearce.org","subject":"Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T05:55:43Z","receivedAt":"2009-12-02T05:55:43Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Dec 01, 2009 at 11:22:33AM -0800, Shawn O. Pearce wrote:\n> \n> This flies against every other convention we have. git:// uses the\n> string 'git-upload-pack' and 'git-receive-pack', and so does the\n> smart-http code. We should continue to use the git- prefix here,\n> to be consistent, even though by context its clearly implied.\n\nChanged for next round (put the git- -prefixes into names).\n \n> Why 'OK'? Currently remote-helpers return an empty blank line\n> to any successful command, not 'OK'.\n\nChanged to \"\" (i.e. blank line) for next round.\n \n> FALLBACK almost makes sense, but ERROR we don't do in the\n> the existing helper protocol. Instead the helper simply\n> prints its error message(s) to stderr and does exit(128).\n> aka what die() does.\n\nERROR case changed to exit(128) of helper for next round.\n\n> Why both connect-r and invoke-r? Why isn't connect-r sufficient\n> here? Isn't it sufficient for any service that runs over git:// ?\n\nInvoke supports those --upload-pack &co (a'la ssh://). connect\ndoesn't (a'la to git://).\n\n-Ilari\n"},{"id":"128970","messageId":"20091202055632.GD31244@Knoppix","threadId":"21807","inReplyTo":"7vskbuwhmy.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T05:56:32Z","receivedAt":"2009-12-02T05:56:32Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Dec 01, 2009 at 12:42:29PM -0800, Junio C Hamano wrote:\n> \n> It is somewhat unfortunate that a few changes I liked (e.g. the \"debug\"\n> bit), even though it was somewhat painful to read them due to coding style\n> differences, were not at the beginning of the series but instead buried\n> after changes that are much bigger and controversial (e.g. [6/8]).\n\nFunny, I considered some other stuff in series much more controversial than\nthe 6/8 one.\n\nAnd 6/8 large? Its smallest (source code files only) or second smallest (all\nfiles) in number of line changes in the series.\n\nIf one looks at 6/8, what it basically does is:\n- Alias remote-curl as remote-{http,ftp}{,s} since the special case dispatch\n rules are no more (.gitignore + makefile).\n- Remove the special case dispatch rules (transport.c).\n\nTaking diffstat of fixed version of 6/8 (I'll send that later as second round,\npossibly with additional fixes):\n\n .gitignore | 4 ++++\n Makefile | 19 +++++++++++++++++++\n transport.c | 8 --------\n 3 files changed, 23 insertions(+), 8 deletions(-)\n\nAnd here's what it does to transport.c:\n\n- } else if (!prefixcmp(url, \"http://\")\n- || !prefixcmp(url, \"https://\")\n- || !prefixcmp(url, \"ftp://\")) {\n- /* These three are just plain special. */\n- transport_helper_init(ret, \"curl\");\n-#ifdef NO_CURL\n- error(\"git was compiled without libcurl support.\");\n-#endif\n\nThat's 8 lines killed in transport.c, 4 new binary aliases (yeah, I'm not that\ngood with makefiles plus this seems to be somewhat nasty case).\n\n-Ilari \n"},{"id":"128973","messageId":"7vy6llnar5.fsf@alter.siamese.dyndns.org","threadId":"21807","inReplyTo":"20091202055632.GD31244@Knoppix","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-02T06:35:58Z","receivedAt":"2009-12-02T06:35:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ilari Liusvaara writes:\n\n> On Tue, Dec 01, 2009 at 12:42:29PM -0800, Junio C Hamano wrote:\n>> \n>> It is somewhat unfortunate that a few changes I liked (e.g. the \"debug\"\n>> bit), even though it was somewhat painful to read them due to coding style\n>> differences, were not at the beginning of the series but instead buried\n>> after changes that are much bigger and controversial (e.g. [6/8]).\n>\n> Funny, I considered some other stuff in series much more controversial than\n> the 6/8 one.\n\nI didn't mean the line count by \"large\". I was referring to the size of\nchange at the conceptual level. As Daniel already explained, it has been\none of the design assumption so far that there are built-in mappings from\nsome common :// to backend \"helpers\".\n\nI am _not_ saying that that particular design assumption must be cast in\nstone (nothing is)---that is a totally different matter to be debated.\nBut the fact that it needs to be debated means it is not \"a trivial 8-line\nreduction\", but rather a large conceptual change (perhaps improvement).\n"},{"id":"129010","messageId":"20091202160446.GA32667@Knoppix","threadId":"21807","inReplyTo":"7vy6llnar5.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T16:04:47Z","receivedAt":"2009-12-02T16:04:47Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Dec 01, 2009 at 10:35:58PM -0800, Junio C Hamano wrote:\n> Ilari Liusvaara writes:\n> \n> I didn't mean the line count by \"large\". I was referring to the size of\n> change at the conceptual level. As Daniel already explained, it has been\n> one of the design assumption so far that there are built-in mappings from\n> some common :// to backend \"helpers\".\n\nNo implicit mappings from :// to helpers existed before this series\n(except for forcing in URL, which are different). Thus, any mapping had to\nbe explicit and built-in.\n\nAnd if mappings http -> curl, https -> curl, ftp -> curl are to remain explicit\nin main git binary, I would put them into table and build stub remote-curl if\nNO_CURL is defined instead of special casing the error in main git binary\n(but I consider that worse than just removing the association from main\ngit binary).\n\n>From file system listing on this computer (note the I-node numbers, this is\non newer version of change than the one sent):\n\n2068945 -rwxr-xr-x 4 Ilari users 1547231 2009-12-02 15:12 /home/Ilari/.local/git-testing/libexec/git-core/git-remote-ftp\n2068945 -rwxr-xr-x 4 Ilari users 1547231 2009-12-02 15:12 /home/Ilari/.local/git-testing/libexec/git-core/git-remote-ftps\n2068945 -rwxr-xr-x 4 Ilari users 1547231 2009-12-02 15:12 /home/Ilari/.local/git-testing/libexec/git-core/git-remote-http\n2068945 -rwxr-xr-x 4 Ilari users 1547231 2009-12-02 15:12 /home/Ilari/.local/git-testing/libexec/git-core/git-remote-https\n\nSo instead of mapping explicitly, those are effectively mapped by filesystem\n(that's after the fixes for next round that make helpers hardlinked instead\nof copied).\n\n-Ilari\n"},{"id":"129017","messageId":"20091202170457.GC31648@spearce.org","threadId":"21807","inReplyTo":"20091202055543.GC31244@Knoppix","subject":"Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-02T17:04:57Z","receivedAt":"2009-12-02T17:04:57Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara wrote:\n> On Tue, Dec 01, 2009 at 11:22:33AM -0800, Shawn O. Pearce wrote:\n> > Why both connect-r and invoke-r? Why isn't connect-r sufficient\n> > here? Isn't it sufficient for any service that runs over git:// ?\n> \n> Invoke supports those --upload-pack &co (a'la ssh://). connect\n> doesn't (a'la to git://).\n\nDrop invoke-r.\n\nModify transport-helper.c to allow pushing TRANS_OPT_UPLOADPACK and\nTRANS_OPT_RECEIVEPACK down into the helper via the option capability.\n\nI'd rename connect-r to just connect.\n\nFor the command line:\n\n $ git fetch --upload-pack='/path to my /git-upload-pack' origin\n\nThe conversation with the helper will be:\n\n > capabilities\n < option\n < connect\n <\n > option uploadpack /path to my /git-upload-pack\n < ok\n > connect git-upload-pack\n <\n\nWhich gives the helper the full information it needs to pass along\nthe --upload-pack command line argument to the remote system.\n\n-- \nShawn.\n"},{"id":"129021","messageId":"20091202171208.GD31648@spearce.org","threadId":"21807","inReplyTo":"20091202055543.GC31244@Knoppix","subject":"Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-02T17:12:08Z","receivedAt":"2009-12-02T17:12:08Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara wrote:\n> On Tue, Dec 01, 2009 at 11:22:33AM -0800, Shawn O. Pearce wrote:\n> \n> > Why 'OK'? Currently remote-helpers return an empty blank line\n> > to any successful command, not 'OK'.\n> \n> Changed to \"\" (i.e. blank line) for next round.\n\nArrrgh. Just to correct myself... the 'option' command uses 'ok',\n'error', 'unsupported' as its response messages. Which means\n'option' breaks the blank-line-is-ok convention I tried to hold\nyou to above.\n\nI consider this a mistake on my part. 'option' should respond with\na blank line on success just like 'fetch' or 'push' does.\n\n-- \nShawn.\n"},{"id":"129024","messageId":"7vy6llgucr.fsf@alter.siamese.dyndns.org","threadId":"21807","inReplyTo":"20091202160446.GA32667@Knoppix","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-02T17:26:44Z","receivedAt":"2009-12-02T17:26:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ilari Liusvaara writes:\n\n> On Tue, Dec 01, 2009 at 10:35:58PM -0800, Junio C Hamano wrote:\n>> Ilari Liusvaara writes:\n>> \n>> I didn't mean the line count by \"large\". I was referring to the size of\n>> change at the conceptual level. As Daniel already explained, it has been\n>> one of the design assumption so far that there are built-in mappings from\n>> some common :// to backend \"helpers\".\n>\n> No implicit mappings from :// to helpers existed before this series\n> (except for forcing in URL, which are different). Thus, any mapping had to\n> be explicit and built-in.\n> ...\n> So instead of mapping explicitly, those are effectively mapped by filesystem\n> (that's after the fixes for next round that make helpers hardlinked instead\n> of copied).\n\nSure, it may be a good change; didn't I say that in the part you omitted\nfrom your quote? But it is conceptually a big change nevertheless, and it\nis to be debated --- that makes the parts leading to 6/8 rather a large\nchange, which was my primary point in the message you are responding to,\nso I think we are in agreement that it would have been nicer if the other\nbits that are independently useful (like the debug one) were earlier in\nthe series.\n\nThe other minor point in my message was that this is to be debated (which\nI think you are doing now), but I am not the best person to debate the\ndesign of this part with. Daniel and Shawn are the guys who have\nprimarily worked on the helper interface and relationship between the\nhelpers and the transport layer, and will have much better insights.\n\nBy the way, are you the helpful git expert we often see on #git IRC\nchannel who goes by the same name? Welcome to the list; I don't remember\nseeing you here, and let me thank you for having helped many new to\nintermediate git users over there for a long time.\n"},{"id":"129026","messageId":"alpine.DEB.1.00.0912021832480.4985@pacific.mpi-cbg.de","threadId":"21807","inReplyTo":"20091202160446.GA32667@Knoppix","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-12-02T17:39:19Z","receivedAt":"2009-12-02T17:39:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 2 Dec 2009, Ilari Liusvaara wrote:\n\n> And if mappings http -> curl, https -> curl, ftp -> curl are to remain \n> explicit in main git binary, I would put them into table and build stub \n> remote-curl if NO_CURL is defined instead of special casing the error in \n> main git binary (but I consider that worse than just removing the \n> association from main git binary).\n\nThis is definitely a good direction, and it would be even better if the \nabsence of the remote helper was also handled gracefully. Just think \nabout a (as of now fictious) git-remote-http.rpm relying on git-core.rpm \nand libcurl.rpm. If you do not want to access http:// URLs, you can \ninstall just git-core. Once you encounter an http:// URL you need to \naccess, you install git-remote-http. Keeping git-core. (I like to call \nthis setup \"modular\".)\n\nOf course, I never understood why the backend should know the \nimplementation detail that it is based on cURL, so it would be even more \nmodular (at least by my definition) if there was no hard-coded mapping \n(Sverre -- Cc'ed -- seemed to like URLs of the form \"svn::http://...\" and \n\"cvs::pserver...\" to trigger looking for a remote helper explicitely). I \nfind the compiled-in mapping rather limiting.\n\nCiao,\nDscho\n"},{"id":"129032","messageId":"fabb9a1e0912021006x2905578bo16dbcaedc0d97bc6@mail.gmail.com","threadId":"21807","inReplyTo":"alpine.DEB.1.00.0912021832480.4985@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-02T18:06:34Z","receivedAt":"2009-12-02T18:06:34Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Dec 2, 2009 at 18:39, Johannes Schindelin\n wrote:\n> This is definitely a good direction, and it would be even better if the\n> absence of the remote helper was also handled gracefully.\n\nYes, that is definitely an improvement we can and should make\nregardless of how we handle http(s) and ftp(s), since currently \"git\nclone nonsense::http://....\" will error out with the message that\n\"git-remote-nonsense\" cannot be found.\n\n> Of course, I never understood why the backend should know the\n> implementation detail that it is based on cURL, so it would be even more\n> modular (at least by my definition) if there was no hard-coded mapping.\n\nAgreed.\n\n> Sverre -- Cc'ed -- seemed to like URLs of the form \"svn::http://...\" and\n> \"cvs::pserver...\" to trigger looking for a remote helper explicitely.  I\n> find the compiled-in mapping rather limiting.\n\nYes, I do think the double-colon syntax is very nice. That is, someone\nwho sees \"git clone svn::http://\" is likely to understand that it is a\nsvn repo over http that git treats specially.\n\nHowever, I am not convinced that we should do any magic to map\n\"foo://\" to git-remote-foo. On the other hand, I do think it makes\nsense to have something modular that allows \"git-remote-http\" to be\nimplemented as a separate package that can be installed.\n\nPerhaps instead of the current special case where \"git-remote-curl\" is\ninvoked, it would make more sense to instead special case on \"http://\"\n(etc) and invoke \"git-remote-http\" in that case. So \"git clone svn://\"\nwould not work, but \"git clone svn::svn://\" would (as is the case\nnow), as well as \"git clone http://\" being handled by\n\"git-remote-http\".\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"129033","messageId":"7vskbtfdvo.fsf@alter.siamese.dyndns.org","threadId":"21807","inReplyTo":"alpine.DEB.1.00.0912021832480.4985@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-02T18:07:55Z","receivedAt":"2009-12-02T18:07:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This is definitely a good direction, and it would be even better if the \n> absence of the remote helper was also handled gracefully. Just think \n> about a (as of now fictious) git-remote-http.rpm relying on git-core.rpm \n> and libcurl.rpm. If you do not want to access http:// URLs, you can \n> install just git-core. Once you encounter an http:// URL you need to \n> access, you install git-remote-http. Keeping git-core. (I like to call \n> this setup \"modular\".)\n\nThe \"modular\" setup is a good thing to do, but I do not know how it\nrelates to the change Ilari did. Isn't it simply a matter of excluding\ngit-remote-curl from the current set of binaries to be shipped with\ngit-core.rpm and making a separate git-remote-http.rpm to contain it, or\ndoes it involve a lot more than that from the packager's side?\n"},{"id":"129034","messageId":"7vk4x5fcbf.fsf@alter.siamese.dyndns.org","threadId":"21807","inReplyTo":"fabb9a1e0912021006x2905578bo16dbcaedc0d97bc6@mail.gmail.com","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-02T18:41:40Z","receivedAt":"2009-12-02T18:41:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n>> Of course, I never understood why the backend should know the\n>> implementation detail that it is based on cURL, so it would be even more\n>> modular (at least by my definition) if there was no hard-coded mapping.\n>\n> Agreed.\n\nI don't get this point at all.\n\nBackend is _very_ aware of how it is implemented itself. Naming one\nimplementation git-remote-http is to declare that \"I am the one and only\nimplementation of http handler\" and forces another implementation of http\nhandler, perhaps based on different toolkit than libcurl, to forever be a\nsecond class citizen that need to use name other than 'http'.\n\nThe \"mapping\" you two are calling \"hard-coded\" may be \"hard-coded\" but is\na better kind of hard-coding than hard-coding \"http\" to \"this particular\nimplementation\" implicitly like you two seem to be advocating. Think of\nit as having one extra layer of indirection.\n\nWhen the second implementation of http handler proves to be better than\nthe current one, we can flip the mapping, and anybody who were using\n\"http://\" to access some repository will automatically updated to use the\nnew backend instead of the old one. With your scheme, you probably could\nchange the name of the old \"http\" backend to \"http-deprecated\" and the new\none from \"second-class-citizen-http\" to \"http\" to achieve a similar\neffect, but I do not think it is as nice as having one extra level of\nindirection.\n\n> However, I am not convinced that we should do any magic to map\n> \"foo://\" to git-remote-foo. On the other hand, I do think it makes\n> sense to have something modular that allows \"git-remote-http\" to be\n> implemented as a separate package that can be installed.\n\nAs I said, I do think modular is good, but I think what Dscho is\nadvocating does not have much to achieve that goal.\n"},{"id":"129035","messageId":"20091202184723.GA10228@Knoppix","threadId":"21807","inReplyTo":"7vskbtfdvo.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T18:47:23Z","receivedAt":"2009-12-02T18:47:23Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Wed, Dec 02, 2009 at 10:07:55AM -0800, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n> \n> The \"modular\" setup is a good thing to do, but I do not know how it\n> relates to the change Ilari did. \n\nRemoving core dependency on NO_CURL is required for that. 6/8 is one\nway to do that given support for dispatching :// to remote\nhelpers (1/8).\n\n> Isn't it simply a matter of excluding\n> git-remote-curl from the current set of binaries to be shipped with\n> git-core.rpm and making a separate git-remote-http.rpm to contain it, or\n> does it involve a lot more than that from the packager's side?\n\nSee above. There's also issues with git remote helper execution, namely\ninability to properly handle failure before cap exchange.\n\nWith properly fixed main git binary, it is just matter of splitting\nremote-xxx executable(s) relating to HTTP to seperate package.\n\n-Ilari\n"},{"id":"129036","messageId":"fabb9a1e0912021050r4fd96ed4rf863917a965a42fb@mail.gmail.com","threadId":"21807","inReplyTo":"7vk4x5fcbf.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-02T18:50:54Z","receivedAt":"2009-12-02T18:50:54Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Dec 2, 2009 at 19:41, Junio C Hamano wrote:\n> When the second implementation of http handler proves to be better than\n> the current one, we can flip the mapping, and anybody who were using\n> \"http://\" to access some repository will automatically updated to use the\n> new backend instead of the old one.  With your scheme, you probably could\n> change the name of the old \"http\" backend to \"http-deprecated\" and the new\n> one from \"second-class-citizen-http\" to \"http\" to achieve a similar\n> effect, but I do not think it is as nice as having one extra level of\n> indirection.\n\nI don't see how what you are talking about is any different. With the\nmapping the executable of the alternative implementation still needs\nto have a different name, no?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"129037","messageId":"7v3a3tfbsx.fsf@alter.siamese.dyndns.org","threadId":"21807","inReplyTo":"fabb9a1e0912021050r4fd96ed4rf863917a965a42fb@mail.gmail.com","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-02T18:52:46Z","receivedAt":"2009-12-02T18:52:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> I don't see how what you are talking about is any different. With the\n> mapping the executable of the alternative implementation still needs\n> to have a different name, no?\n\nSure, but please search for \"second class citizen\" in my message.\n"},{"id":"129038","messageId":"fabb9a1e0912021055m56328d87y11a090a9a9bcb046@mail.gmail.com","threadId":"21807","inReplyTo":"7v3a3tfbsx.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-02T18:55:04Z","receivedAt":"2009-12-02T18:55:04Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Dec 2, 2009 at 19:52, Junio C Hamano wrote:\n> Sure, but please search for \"second class citizen\" in my message.\n\nI read that, but I don't understand how exactly the mapping will make\nit a non-second class citizen. How will your mapping include the\nalternative implementation? The word mapping suggests a 1:1 relation\nbetween protocol and implementation, so I don't see how the\nalternative implementation would become first-class :(.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"129039","messageId":"7vy6lldwyo.fsf@alter.siamese.dyndns.org","threadId":"21807","inReplyTo":"7v3a3tfbsx.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-02T18:58:39Z","receivedAt":"2009-12-02T18:58:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Sverre Rabbelier writes:\n>\n>> I don't see how what you are talking about is any different. With the\n>> mapping the executable of the alternative implementation still needs\n>> to have a different name, no?\n>\n> Sure, but please search for \"second class citizen\" in my message.\n\nAlso \"extra level of indication\".\n\nI do not think \"remote-curl\" was the best name, and hindsight tells me\nthat \"remote-walker\" might have been a better name (it tells us how it\ndoes it more clearly).\n\nAnd I do not at all mind making the current hard-coded mapping from\nhttp:// to remote-walker to an external table look-up, perhaps something\nthat can be controlled by .git/config, with a built-in default that is\nhard-coded like the way we have now.\n\nAfter all my main objection is against closing the door to others by one\nparticular implementation squating on \"remote-http\" name and refusing the\nuse of that nice, authoritative-sounding name by others.\n"},{"id":"129044","messageId":"20091202192507.GB10228@Knoppix","threadId":"21807","inReplyTo":"7vk4x5fcbf.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T19:25:08Z","receivedAt":"2009-12-02T19:25:08Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Wed, Dec 02, 2009 at 10:41:40AM -0800, Junio C Hamano wrote:\n> Sverre Rabbelier writes:\n> \n> >> Of course, I never understood why the backend should know the\n> >> implementation detail that it is based on cURL, so it would be even more\n> >> modular (at least by my definition) if there was no hard-coded mapping.\n> >\n> > Agreed.\n> \n> I don't get this point at all.\n> \n> Backend is _very_ aware of how it is implemented itself. Naming one\n> implementation git-remote-http is to declare that \"I am the one and only\n> implementation of http handler\" and forces another implementation of http\n> handler, perhaps based on different toolkit than libcurl, to forever be a\n> second class citizen that need to use name other than 'http'.\n\nAt least it can be called as 'foo::http://' (That may be tolerable for\nalternate implementations but not for primary ones).\n\n> The \"mapping\" you two are calling \"hard-coded\" may be \"hard-coded\" but is\n> a better kind of hard-coding than hard-coding \"http\" to \"this particular\n> implementation\" implicitly like you two seem to be advocating. Think of\n> it as having one extra layer of indirection.\n\nIts already indirected: By filesystem.\n\n> When the second implementation of http handler proves to be better than\n> the current one, we can flip the mapping, and anybody who were using\n> \"http://\" to access some repository will automatically updated to use the\n> new backend instead of the old one. With your scheme, you probably could\n> change the name of the old \"http\" backend to \"http-deprecated\" and the new\n> one from \"second-class-citizen-http\" to \"http\" to achieve a similar\n> effect, but I do not think it is as nice as having one extra level of\n> indirection.\n\nThe new HTTP support must either be internal or not. And:\n\n- If it is internal, renaming can be done anyway.\n- If it is not, change can not be made.\n\nAnd at package manager level, this is what 'conflicts: ' is about (and\nalternates of apt).\n\n> > However, I am not convinced that we should do any magic to map\n> > \"foo://\" to git-remote-foo. On the other hand, I do think it makes\n> > sense to have something modular that allows \"git-remote-http\" to be\n> > implemented as a separate package that can be installed.\n> \n> As I said, I do think modular is good, but I think what Dscho is\n> advocating does not have much to achieve that goal.\n\nWhy should adding new git native protocol (that doesn't have so special\ncapabilities new core support is fundamentially required) require recompiling\ngit core? Why it should require more than dropping executable handler to\nsuitable place?[*]\n\nWhy should such protocols need to be specified 'foo::foo://'?\n\nGranted, even with current dispatch meachanisms, its possible to hack\ntogether something that accepts 'foo:://', 'foo::/' or 'foo::' but that\nbreaks user expectations rather badly (the U in URL stands for 'uniform')... \n\nAnd currently the URL space to be reassigned just produces fatal error\nmessages anyway.\n\n[*] Merge strategies have similar issues. IMO, on encountering unknown\nmerge strategy, git merging code should check if there's handler for it,\nif yes, obtain flags from it and then use it.\n\n-Ilari \n"},{"id":"129056","messageId":"20091202193902.GE30778@coredump.intra.peff.net","threadId":"21807","inReplyTo":"7vy6lldwyo.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-12-02T19:39:02Z","receivedAt":"2009-12-02T19:39:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 02, 2009 at 10:58:39AM -0800, Junio C Hamano wrote:\n\n> After all my main objection is against closing the door to others by one\n> particular implementation squating on \"remote-http\" name and refusing the\n> use of that nice, authoritative-sounding name by others.\n\nI would think that it would be useful to use the \"remote-http\" name as\nthe extra level of indirection (as a symlink, hardlink, or wrapper\nscript to remote-curl). Then you could have competing first-class\nimplementations that would be easy for the user (or package manager) to\nswitch between.\n\nFor example, Debian contains versions of curl built against gnutls and\nagainst openssl. Right now the debian git package requires the gnutls\nversion. But let's say they ship two packages: git-http-curl-openssl and\ngit-http-curl-gnutls. Then you can install whichever you prefer, and the\npackage will contain the file \"git-remote-http\" pointing to\n\"git-remote-curl-$whatever\".\n\nAnd yes, if you think about it, this particular situation already works\nwith a hard-coded \"git-remote-curl\", since both are built on top of\ncurl, and that makes a reasonable name. But now extend it to \"you don't\nwant to use curl, but rather some other http library\". I don't think we\nhave any interest in providing a non-curl version as part of git itself,\nbut it provides a hook should somebody want to write their own http\nhandler (either using a different library, or maybe a wrapper that does\ncaching, or whatever).\n\nJust my two cents. I don't plan on writing any such third-party remote\nhandlers, but it seems simple enough to leave the door open.\n\n-Peff\n"},{"id":"129057","messageId":"20091202195233.GA11301@Knoppix","threadId":"21807","inReplyTo":"alpine.DEB.1.00.0912021832480.4985@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T19:52:33Z","receivedAt":"2009-12-02T19:52:33Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Wed, Dec 02, 2009 at 06:39:19PM +0100, Johannes Schindelin wrote:\n> \n> This is definitely a good direction, and it would be even better if the \n> absence of the remote helper was also handled gracefully. Just think \n> about a (as of now fictious) git-remote-http.rpm relying on git-core.rpm \n> and libcurl.rpm. If you do not want to access http:// URLs, you can \n> install just git-core. Once you encounter an http:// URL you need to \n> access, you install git-remote-http. Keeping git-core. (I like to call \n> this setup \"modular\".)\n\nThere are some rather unfortunate details relating to this.\n\nMain git executable currently has no good way to discover what went wrong\nwith remote helper execution that fails before reaching capabilities\nexchange.\n\nIt would be ideal if executions failing due to ENOENT would be reported\nas remote helper not existing, other exec errors reported as failed execution,\nfatal signals as remote helper crashing and other exits rely on remote helper\nreporting the problem.\n\nUnfortunately, this can't be done without breaking remote helper interface,\neither by requiring initial response from helper or requiring helpers not\nto explicitly fail due to bad parameters before reaching capabilities exchange,\nsince one can't know if execution was successuful without seeing at least\none incoming line.\n\nIIRC, current versions print some rather funky error if you try to use\nnonexistent helper: 'remote-foo is not git command' or some such.\n\n> Of course, I never understood why the backend should know the \n> implementation detail that it is based on cURL, so it would be even more \n> modular (at least by my definition) if there was no hard-coded mapping \n> (Sverre -- Cc'ed -- seemed to like URLs of the form \"svn::http://...\" and \n> \"cvs::pserver...\" to trigger looking for a remote helper explicitely). I \n> find the compiled-in mapping rather limiting.\n\nThat syntax is rather nice for handling foregin VCSes that may have URL forms\nthat overlap with native ones. But it sure isn't nice for those remote helpers\nthat implement git native transports (remote-curl is already a precedent on\ndoing that). \n\nThe API is already general enough to do both: Git native transports (currently\ndumb only without lots of effort, which this patchset is about) and foregin \nVCS bridges.\n\n-Ilari\n"},{"id":"129061","messageId":"20091202201008.GB11301@Knoppix","threadId":"21807","inReplyTo":"20091202170457.GC31648@spearce.org","subject":"Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2009-12-02T20:10:08Z","receivedAt":"2009-12-02T20:10:08Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Wed, Dec 02, 2009 at 09:04:57AM -0800, Shawn O. Pearce wrote:\n> Ilari Liusvaara wrote:\n> \n> Drop invoke-r.\n\nDropped.\n\n> Modify transport-helper.c to allow pushing TRANS_OPT_UPLOADPACK and\n> TRANS_OPT_RECEIVEPACK down into the helper via the option capability.\n\nNAK. Modified _process_connect_or_invoke (now _process_connect) to pass\nnew option that appiles to connecting all subprotocols (if needed).\n\nIt looks like following:\n\n > capabilities\n < option\n < connect\n <\n > option servpath \n < ok\n > connect git-upload-pack\n < \n\nAnd from helper POV, all subprotocols should appear identical from\nlayer 6 POV so it doesn't make sense to diffrentiate between path\nfor upload-pack and receive-pack (or upload-archive!).\n\n> I'd rename connect-r to just connect.\n\nYeah, putting repository in RPC explicit signature is bit ugly (there\nisn't probaby ever going to be signature that doesn't contain repo as\nargument). That would make it 'connect'.\n\nRenamed.\n\n-Ilari\n"},{"id":"129137","messageId":"20091203194242.GD23281@spearce.org","threadId":"21807","inReplyTo":"20091202201008.GB11301@Knoppix","subject":"Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-03T19:42:42Z","receivedAt":"2009-12-03T19:42:42Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Ilari Liusvaara wrote:\n> On Wed, Dec 02, 2009 at 09:04:57AM -0800, Shawn O. Pearce wrote:\n> > Modify transport-helper.c to allow pushing TRANS_OPT_UPLOADPACK and\n> > TRANS_OPT_RECEIVEPACK down into the helper via the option capability.\n> \n> NAK. Modified _process_connect_or_invoke (now _process_connect) to pass\n> new option that appiles to connecting all subprotocols (if needed).\n...\n> And from helper POV, all subprotocols should appear identical from\n> layer 6 POV so it doesn't make sense to diffrentiate between path\n> for upload-pack and receive-pack (or upload-archive!).\n\nThat may be true, but the remote.origin.uploadpack and\nremote.origin.receivepack configuration options exist\nand are passed through as these option uploadpack and\noption receivepack callbacks.\n\nIf you want to pass through a single option with the remote program\nname, you need to do that immediately before the connect invoke\noccurs, and instead buffer the two different configuration options\nin the struct transport. I think that would make the series messier\nthan it is.\n \n-- \nShawn.\n"}]}