{"thread":{"id":"44095","subject":"[PATCH 00/11] Resumable clone","startedAt":"2016-09-16T00:12:31Z","lastAt":"2016-09-28T20:46:32Z","messageCount":39,"participants":["Kevin Wern","Junio C Hamano","Johannes Schindelin","Duy Nguyen","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"302033","messageId":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":null,"subject":"[PATCH 00/11] Resumable clone","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:11Z","receivedAt":"2016-09-16T00:12:31Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Hey, all,\n\nIt's been a while (sent a very short patch in May), but I've\nstill been working on the resumable clone feature and checking up on\nthe mailing list for any updates. After submitting the prime-clone\nservice alone, I figured implementing the whole thing would be the best\nway to understand the full scope of the problem (this is my first real\ncontribution here, and learning while working on such an involved\nfeature has not been easy). \n\nThis is a functional implementation handling a direct http/ftp URI to a\nsingle, fully connected packfile (i.e. the link is a direct path to the\nfile, not a prefix or guess). My hope is that this acts as a bare\nminimum cross-section spanning the full requirments that can expand in\nwidth as more cases are added (.info file, split bundle, daemon\ndownload service). This is certainly not perfect, but I think it at\nleast prototypes each component involved in the workflow.\n\nThis patch series is based on jc/bundle, because the logic to find the\ntips of a pack's history already exists there (I call index-pack\n--clone-bundle on the downloaded file, and read the file to write the\nreferences to a temporary directory). If I need to re-implement this\nlogic or base it on another branch, let me know. For ease of pulling\nand testing, I included the branch here:\n\nhttps://github.com/kevinwern/git/tree/feature/prime-clone\n\nAlthough there are a few changes internally from the last patch,\nthe \"alternate resource\" url to download is configured on the\nserver side in exactly the same way:\n\n[primeclone]\n\turl = http://location/pack-$NAME.pack\n\tfiletype = pack\n\nThe prime-clone service simply outputs the components as:\n\n####url filetype\n0000\n\nOn the client side, the transport_prime_clone and\ntransport_download_primer APIs are built to be more robust (i.e. read\nmessages without dying due to protocol errors), so that git clone can\nalways try them without being dependent on the capability output of\ngit-upload-pack. transport_download_primer is dependent on the success\nof transport_prime_clone, but transport_prime_clone is always run on an\ninitial clone. Part of achieving this robustness involves adding\n*_gentle functions to pkt_line, so that prime_clone can fail silently\nwithout dying.\n\nThe transport_download_primer function uses a resumable download,\nwhich is applicable to both automatic and manual resuming. Automatic\nis programmatically reconnecting to the resource after being\ninterrupted (up to a set number of times). Manual is using a newly\ntaught --resume option on the command line:\n\ngit clone --resume \n\nRight now, a manually resumable directory is left behind only if the\n*client* is interrupted while a new junk mode, JUNK_LEAVE_RESUMABLE,\nis set (right before the download). For an initial clone, if the\nconnection fails after automatic resuming, the client erases the\npartial resources and falls through to a normal clone. However, once a\nresumable directory is left behind by the program, it is NEVER\ndeleted/abandoned after it is continued with --resume.\n\nI think determining when a resource is \"unsalvageable\" should be more\nnuanced. Especially in a case where a connection is perpetually poor\nand the user wishes to resume over a long period of time. The timeout\nlogic itself *definitely* needs more nuance than \"repeat 5 times\", such\nas expanding wait times and using earlier successes when deciding to\ntry again. Right now, I think the most important part of this patch is\nthat these two paths (falling through after a failed download, exiting\nto be manually resumed later) exist.\n\nOff the top of my head, outstanding issues/TODOs inlcude:\n\t- The above issue of determining when to fall through, when to\n\t reattempt, and when to write the resumable info and exit\n\t in git clone.\n\t- Creating git-daemon service to download a resumable resource.\n\t Pretty straightforward, I think, especially if\n\t http.getanyfile already exists. This falls more under\n\t \"haven't gotten to yet\" than dilemma.\n\t- Logic for git clone to determine when a full clone would\n\t be superior, such as when a clone is local or a reference is\n\t given.\n\t- Configuring prime-clone for multiple resources, in two\n\t dimensions: (a) resources to choose from (e.g. fall back to\n\t a second resource if the first one doesn't work) and (b)\n\t resources to be downloaded together or in sequence (e.g.\n\t download http://host/this, then http://host/that). Maybe\n\t prime-clone could also handle client preferences in terms of\n\t filetype or protocol. For this, I just have to re-read a few\n\t discussions about the filetypes we use to see if there are\n\t any outliers that aren't representable in this way. I think\n\t this is another \"haven't gotten to yet\".\n\t- Related to the above, seeing if there are any outlying\n\t resource types whose process can't be modularized into:\n\t download to location, use, clean one way if failed, clean\n\t another way if succeeded. The \"split bundle,\" for example,\n\t is retrieved (download), read for the pack location (use),\n\t and then the packfile is retrieved (download). I believe, in\n\t this case, all of that can be considered the \"download,\" and\n\t then indexing/writing can be considered \"use.\" But I'm not\n\t sure if there are more extreme cases.\n\t- Creating the logic to guess a packfile, and append that to a\n\t prefix specified by the admin. Additionally, allowing the\n\t admin to use a custom script to use their own logic to\n\t output the URL.\n\t- Preventing the retry wait period (currently set by using\n\t select()) from being interrupted by other system calls.\n\t I believe there is a setting in libcurl, but I don't want\n\t to make any potentially large-impact changes without\n\t discussing it first. Plus, I believe changes to http.c were\n\t up for discussion anyway.\n\t- Finding if there's a more elegant way to access the alternate\n\t resource than invoking remote-helper with a url we don't care\n\t about (the same url that will be specified later to stdin\n\t with \"download-primer\").\n\t- Finding if there is a better way to suppress index-pack's\n\t output than creating a run-command option specifically to\n\t suppress stdout.\n\t- When running with ssh and a password, the credentials are\n\t prompted for twice. I don't know if there is a way to\n\t preserve credentials between executions. I couldn't find any\n\t examples in git's source.\n\nSome of these are issues I've been actively working on, but I'm\nhitting a point where keeping everyone up-to-date trumps completeness.\nHopefully, the bulk of the 'learning and re-doing' is done and I can\nupdate more frequently in smaller increments.\n\nI will probably work on the git-daemon download service, the curl\ntimeout issue, and supporting other filetypes next.\n\nFeedback is appreciated.\n\nKevin Wern (11):\n Resumable clone: create service git-prime-clone\n Resumable clone: add prime-clone endpoints\n pkt-line: create gentle packet_read_line functions\n Resumable clone: add prime-clone to remote-curl\n Resumable clone: add output parsing to connect.c\n Resumable clone: implement transport_prime_clone\n Resumable clone: add resumable download to http/curl\n Resumable clone: create transport_download_primer\n path: add resumable marker\n run command: add RUN_COMMAND_NO_STDOUT\n Resumable clone: implement primer logic in git-clone\n\n .gitignore | 1 +\n Documentation/git-clone.txt | 16 +\n Documentation/git-daemon.txt | 7 +\n Documentation/git-http-backend.txt | 7 +\n Documentation/git-prime-clone.txt | 39 +++\n Makefile | 2 +\n builtin.h | 1 +\n builtin/clone.c | 590 +++++++++++++++++++++++++++++++------\n builtin/prime-clone.c | 77 +++++\n cache.h | 1 +\n connect.c | 47 +++\n connect.h | 10 +-\n daemon.c | 7 +\n git.c | 1 +\n http-backend.c | 22 +-\n http.c | 86 +++++-\n http.h | 7 +-\n path.c | 1 +\n pkt-line.c | 47 ++-\n pkt-line.h | 16 +\n remote-curl.c | 192 +++++++++---\n run-command.c | 1 +\n run-command.h | 1 +\n t/t9904-git-prime-clone.sh | 181 ++++++++++++\n transport-helper.c | 75 ++++-\n transport.c | 53 ++++\n transport.h | 27 ++\n 27 files changed, 1361 insertions(+), 154 deletions(-)\n create mode 100644 Documentation/git-prime-clone.txt\n create mode 100644 builtin/prime-clone.c\n create mode 100755 t/t9904-git-prime-clone.sh\n\n-- \n2.7.4\n\n"},{"id":"302034","messageId":"1473984742-12516-2-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 01/11] Resumable clone: create service git-prime-clone","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:12Z","receivedAt":"2016-09-16T00:12:36Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Create git-prime-clone, a program to be executed on the server that\nreturns the location and type of static resource to download before\nperforming the rest of a clone.\n\nAdditionally, as this executable's location will be configurable (see:\nupload-pack and receive-pack), add the program to\nBINDIR_PROGRAMS_NEED_X, in addition to the usual builtin places. Add\ngit-prime-clone executable to gitignore, as well\n\nSigned-off-by: Kevin Wern \n---\n .gitignore | 1 +\n Documentation/git-prime-clone.txt | 39 ++++++++++++++++++++\n Makefile | 2 +\n builtin.h | 1 +\n builtin/prime-clone.c | 77 +++++++++++++++++++++++++++++++++++++++\n git.c | 1 +\n 6 files changed, 121 insertions(+)\n create mode 100644 Documentation/git-prime-clone.txt\n create mode 100644 builtin/prime-clone.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 5087ce1..bfea25c 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -106,6 +106,7 @@\n /git-pack-refs\n /git-parse-remote\n /git-patch-id\n+/git-prime-clone\n /git-prune\n /git-prune-packed\n /git-pull\ndiff --git a/Documentation/git-prime-clone.txt b/Documentation/git-prime-clone.txt\nnew file mode 100644\nindex 0000000..fc5917d\n--- /dev/null\n+++ b/Documentation/git-prime-clone.txt\n@@ -0,0 +1,39 @@\n+git-prime-clone(1)\n+============\n+\n+NAME\n+----\n+git-prime-clone - Get the location of an alternate resource\n+to fetch before clone\n+\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git prime-clone' [--strict] \n+\n+DESCRIPTION\n+-----------\n+\n+Outputs the resource, if configured to do so. Otherwise, returns\n+nothing (packet flush 0000).\n+\n+CONFIGURE\n+---------\n+\n+primeclone.url::\n+\tThe full url of the resource (e.g.\n+\thttp://examplehost/pack-$NAME.pack).\n+\n+primeclone.filetype::\n+\tThe type of the resource (e.g. pack).\n+\n+primeclone.enabled::\n+\tWhen 'false', git-prime-clone will return an empty response,\n+\tregardless of what the rest of the configuration specifies;\n+\totherwise, it will return the configured response. Is 'true'\n+\tby default.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 24bef8d..f2564ec 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -648,6 +648,7 @@ OTHER_PROGRAMS = git$X\n # what test wrappers are needed and 'install' will install, in bindir\n BINDIR_PROGRAMS_NEED_X += git\n BINDIR_PROGRAMS_NEED_X += git-upload-pack\n+BINDIR_PROGRAMS_NEED_X += git-prime-clone\n BINDIR_PROGRAMS_NEED_X += git-receive-pack\n BINDIR_PROGRAMS_NEED_X += git-upload-archive\n BINDIR_PROGRAMS_NEED_X += git-shell\n@@ -904,6 +905,7 @@ BUILTIN_OBJS += builtin/pack-objects.o\n BUILTIN_OBJS += builtin/pack-redundant.o\n BUILTIN_OBJS += builtin/pack-refs.o\n BUILTIN_OBJS += builtin/patch-id.o\n+BUILTIN_OBJS += builtin/prime-clone.o\n BUILTIN_OBJS += builtin/prune-packed.o\n BUILTIN_OBJS += builtin/prune.o\n BUILTIN_OBJS += builtin/pull.o\ndiff --git a/builtin.h b/builtin.h\nindex 6b95006..c9e2254 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -97,6 +97,7 @@ extern int cmd_notes(int argc, const char **argv, const char *prefix);\n extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);\n extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix);\n extern int cmd_patch_id(int argc, const char **argv, const char *prefix);\n+extern int cmd_prime_clone(int argc, const char **argv, const char *prefix);\n extern int cmd_prune(int argc, const char **argv, const char *prefix);\n extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);\n extern int cmd_pull(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/prime-clone.c b/builtin/prime-clone.c\nnew file mode 100644\nindex 0000000..ce914d3\n--- /dev/null\n+++ b/builtin/prime-clone.c\n@@ -0,0 +1,77 @@\n+#include \"cache.h\"\n+#include \"parse-options.h\"\n+#include \"pkt-line.h\"\n+\n+static char const * const prime_clone_usage[] = {\n+\tN_(\"git prime-clone [--strict] \"),\n+\tNULL\n+};\n+\n+static unsigned int enabled = 1;\n+static const char *url = NULL, *filetype = NULL;\n+static int strict;\n+\n+static struct option prime_clone_options[] = {\n+\tOPT_BOOL(0, \"strict\", &strict, N_(\"Do not attempt /.git if \"\n+\t\t\t\t\t \"is not a git directory\")),\n+\tOPT_END(),\n+};\n+\n+static void prime_clone(void)\n+{\n+\tif (!enabled) {\n+\t\tfprintf(stderr, _(\"prime-clone not enabled\\n\"));\n+\t}\n+\telse if (url && filetype){\n+\t\tpacket_write(1, \"%s %s\\n\", filetype, url);\n+\t}\n+\telse if (url || filetype) {\n+\t\tif (filetype)\n+\t\t\tfprintf(stderr, _(\"prime-clone not properly \"\n+\t\t\t\t\t \"configured: missing url\\n\"));\n+\t\telse if (url)\n+\t\t\tfprintf(stderr, _(\"prime-clone not properly \"\n+\t\t\t\t\t \"configured: missing filetype\\n\"));\n+\t}\n+\tpacket_flush(1);\n+}\n+\n+static int prime_clone_config(const char *var, const char *value, void *unused)\n+{\n+\tif (!strcmp(\"primeclone.url\",var)) {\n+\t\treturn git_config_pathname(&url, var, value);\n+\t}\n+\tif (!strcmp(\"primeclone.enabled\",var)) {\n+\t\tenabled = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(\"primeclone.filetype\",var)) {\n+\t\treturn git_config_string(&filetype, var, value);\n+\t}\n+\treturn git_default_config(var, value, unused);\n+}\n+\n+int cmd_prime_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *dir;\n+\targc = parse_options(argc, argv, prefix, prime_clone_options,\n+\t\t\t prime_clone_usage, 0);\n+\tif (argc == 0) {\n+\t\tusage_msg_opt(_(\"No repository specified.\"), prime_clone_usage,\n+\t\t\t prime_clone_options);\n+\t}\n+\telse if (argc > 1) {\n+\t\tusage_msg_opt(_(\"Too many arguments.\"), prime_clone_usage,\n+\t\t\t prime_clone_options);\n+\t}\n+\n+\tdir = argv[0];\n+\n+\tif (!enter_repo(dir, 0)){\n+\t\tdie(_(\"'%s' does not appear to be a git repository\"), dir);\n+\t}\n+\n+\tgit_config(prime_clone_config, NULL);\n+\tprime_clone();\n+\treturn 0;\n+}\ndiff --git a/git.c b/git.c\nindex 6cc0c07..a5681fb 100644\n--- a/git.c\n+++ b/git.c\n@@ -447,6 +447,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pack-refs\", cmd_pack_refs, RUN_SETUP },\n \t{ \"patch-id\", cmd_patch_id },\n \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n+\t{ \"prime-clone\", cmd_prime_clone },\n \t{ \"prune\", cmd_prune, RUN_SETUP },\n \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\n \t{ \"pull\", cmd_pull, RUN_SETUP | NEED_WORK_TREE },\n-- \n2.7.4\n\n"},{"id":"302035","messageId":"1473984742-12516-3-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 02/11] Resumable clone: add prime-clone endpoints","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:13Z","receivedAt":"2016-09-16T00:12:39Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Add logic to serve git-prime-clone to git and http clients.\n\nDo not pass --stateless-rpc and --advertise-refs options to\nprime-clone. It is inherently stateless and an 'advertisement'.\n\nSigned-off-by: Kevin Wern \n---\n Documentation/git-daemon.txt | 7 +++++++\n Documentation/git-http-backend.txt | 7 +++++++\n daemon.c | 7 +++++++\n http-backend.c | 22 +++++++++++++++++-----\n 4 files changed, 38 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex a69b361..853faab 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -231,6 +231,13 @@ receive-pack::\n \tenabled by setting `daemon.receivepack` configuration item to\n \t`true`.\n \n+primeclone::\n+\tThis serves 'git prime-clone' service to clients, allowing\n+\t'git clone' clients to get the location of a static resource\n+\tto download and integrate before performing an incremental\n+\tfetch. It is 'false' by default, but can be enabled by setting\n+\tit to `true`.\n+\n EXAMPLES\n --------\n We assume the following in /etc/services::\ndiff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt\nindex 9268fb6..40be74e 100644\n--- a/Documentation/git-http-backend.txt\n+++ b/Documentation/git-http-backend.txt\n@@ -54,6 +54,13 @@ http.receivepack::\n \tdisabled by setting this item to `false`, or enabled for all\n \tusers, including anonymous users, by setting it to `true`.\n \n+http.primeclone::\n+\tThis serves 'git prime-clone' service to clients, allowing\n+\t'git clone' clients to get the location of a static resource\n+\tto download and integrate before performing an incremental\n+\tfetch. It is 'false' by default, but can be enabled by setting\n+\tit to `true`.\n+\n URL TRANSLATION\n ---------------\n To determine the location of the repository on disk, 'git http-backend'\ndiff --git a/daemon.c b/daemon.c\nindex 8d45c33..c2f539c 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -475,10 +475,17 @@ static int receive_pack(void)\n \treturn run_service_command(argv);\n }\n \n+static int prime_clone(void)\n+{\n+\tstatic const char *argv[] = { \"prime-clone\", \"--strict\", \".\", NULL };\n+\treturn run_service_command(argv);\n+}\n+\n static struct daemon_service daemon_service[] = {\n \t{ \"upload-archive\", \"uploadarch\", upload_archive, 0, 1 },\n \t{ \"upload-pack\", \"uploadpack\", upload_pack, 1, 1 },\n \t{ \"receive-pack\", \"receivepack\", receive_pack, 0, 1 },\n+\t{ \"prime-clone\", \"primeclone\", prime_clone, 0, 1 },\n };\n \n static void enable_service(const char *name, int ena)\ndiff --git a/http-backend.c b/http-backend.c\nindex 8870a26..9c89a10 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -27,6 +27,7 @@ struct rpc_service {\n static struct rpc_service rpc_service[] = {\n \t{ \"upload-pack\", \"uploadpack\", 1, 1 },\n \t{ \"receive-pack\", \"receivepack\", 0, -1 },\n+\t{ \"prime-clone\", \"primeclone\", 0, -1 },\n };\n \n static struct string_list *get_parameters(void)\n@@ -450,11 +451,22 @@ static void get_info_refs(char *arg)\n \thdr_nocache();\n \n \tif (service_name) {\n-\t\tconst char *argv[] = {NULL /* service name */,\n-\t\t\t\"--stateless-rpc\", \"--advertise-refs\",\n-\t\t\t\".\", NULL};\n+\t\tstruct argv_array argv;\n \t\tstruct rpc_service *svc = select_service(service_name);\n \n+\t\targv_array_init(&argv);\n+\t\targv_array_push(&argv, svc->name);\n+\n+\t\t// prime-clone does not need --stateless-rpc and\n+\t\t// --advertise-refs options. Maybe it will in the future, but\n+\t\t// until then it seems best to do this instead of adding\n+\t\t// \"dummy\" options.\n+\t\tif (strcmp(svc->name, \"prime-clone\") != 0) {\n+\t\t\targv_array_pushl(&argv, \"--stateless-rpc\",\n+\t\t\t\t\t \"--advertise-refs\", NULL);\n+\t\t}\n+\n+\t\targv_array_pushl(&argv, \".\", NULL);\n \t\tstrbuf_addf(&buf, \"application/x-git-%s-advertisement\",\n \t\t\tsvc->name);\n \t\thdr_str(content_type, buf.buf);\n@@ -463,8 +475,8 @@ static void get_info_refs(char *arg)\n \t\tpacket_write(1, \"# service=git-%s\\n\", svc->name);\n \t\tpacket_flush(1);\n \n-\t\targv[0] = svc->name;\n-\t\trun_service(argv, 0);\n+\t\trun_service(argv.argv, 0);\n+\t\targv_array_clear(&argv);\n \n \t} else {\n \t\tselect_getanyfile();\n-- \n2.7.4\n\n"},{"id":"302036","messageId":"1473984742-12516-4-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 03/11] pkt-line: create gentle packet_read_line functions","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:14Z","receivedAt":"2016-09-16T00:12:42Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Create a functions that can read malformed messages without dying.\nIncludes creation of flag PACKET_READ_GENTLE_ALL. For use handling\nprime-clone (or other server error) responses.\n\nSigned-off-by: Kevin Wern \n---\n pkt-line.c | 47 ++++++++++++++++++++++++++++++++++++++---------\n pkt-line.h | 16 ++++++++++++++++\n 2 files changed, 54 insertions(+), 9 deletions(-)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 62fdb37..96060e5 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -155,13 +155,17 @@ static int get_packet_data(int fd, char **src_buf, size_t *src_size,\n \t\t*src_size -= ret;\n \t} else {\n \t\tret = read_in_full(fd, dst, size);\n-\t\tif (ret < 0)\n+\t\tif (ret < 0) {\n+\t\t\tif (options & PACKET_READ_GENTLE_ALL)\n+\t\t\t\treturn -1;\n+\n \t\t\tdie_errno(\"read error\");\n+\t\t}\n \t}\n \n \t/* And complain if we didn't get enough bytes to satisfy the read. */\n \tif (ret < size) {\n-\t\tif (options & PACKET_READ_GENTLE_ON_EOF)\n+\t\tif (options & (PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ALL))\n \t\t\treturn -1;\n \n \t\tdie(\"The remote end hung up unexpectedly\");\n@@ -205,15 +209,23 @@ int packet_read(int fd, char **src_buf, size_t *src_len,\n \tif (ret < 0)\n \t\treturn ret;\n \tlen = packet_length(linelen);\n-\tif (len < 0)\n+\tif (len < 0) {\n+\t\tif (options & PACKET_READ_GENTLE_ALL)\n+\t\t\treturn -1;\n+\n \t\tdie(\"protocol error: bad line length character: %.4s\", linelen);\n+\t}\n \tif (!len) {\n \t\tpacket_trace(\"0000\", 4, 0);\n \t\treturn 0;\n \t}\n \tlen -= 4;\n-\tif (len >= size)\n+\tif (len >= size) {\n+\t\tif (options & PACKET_READ_GENTLE_ALL)\n+\t\t\treturn -1;\n+\n \t\tdie(\"protocol error: bad line length %d\", len);\n+\t}\n \tret = get_packet_data(fd, src_buf, src_len, buffer, len, options);\n \tif (ret < 0)\n \t\treturn ret;\n@@ -229,22 +241,39 @@ int packet_read(int fd, char **src_buf, size_t *src_len,\n \n static char *packet_read_line_generic(int fd,\n \t\t\t\t char **src, size_t *src_len,\n-\t\t\t\t int *dst_len)\n+\t\t\t\t int *dst_len, int flags)\n {\n \tint len = packet_read(fd, src, src_len,\n \t\t\t packet_buffer, sizeof(packet_buffer),\n-\t\t\t PACKET_READ_CHOMP_NEWLINE);\n+\t\t\t flags);\n \tif (dst_len)\n \t\t*dst_len = len;\n-\treturn len ? packet_buffer : NULL;\n+\treturn len > 0 ? packet_buffer : NULL;\n }\n \n char *packet_read_line(int fd, int *len_p)\n {\n-\treturn packet_read_line_generic(fd, NULL, NULL, len_p);\n+\treturn packet_read_line_generic(fd, NULL, NULL, len_p,\n+\t\t\tPACKET_READ_CHOMP_NEWLINE);\n }\n \n char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)\n {\n-\treturn packet_read_line_generic(-1, src, src_len, dst_len);\n+\treturn packet_read_line_generic(-1, src, src_len, dst_len,\n+\t\t\tPACKET_READ_CHOMP_NEWLINE);\n+}\n+\n+char *packet_read_line_gentle(int fd, int *len_p)\n+{\n+\treturn packet_read_line_generic(fd, NULL, NULL, len_p,\n+\t\t\tPACKET_READ_CHOMP_NEWLINE |\n+\t\t\tPACKET_READ_GENTLE_ALL);\n+}\n+\n+\n+char *packet_read_line_buf_gentle(char **src, size_t *src_len, int *dst_len)\n+{\n+\treturn packet_read_line_generic(-1, src, src_len, dst_len,\n+\t\t\tPACKET_READ_CHOMP_NEWLINE |\n+\t\t\tPACKET_READ_GENTLE_ALL);\n }\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 3cb9d91..553e42e 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -52,11 +52,15 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((f\n * condition 4 (truncated input), but instead return -1. However, we will still\n * die for the other 3 conditions.\n *\n+ * If options contains PACKET_READ_GENTLE_ALL, we will not die on any of the\n+ * conditions, but return -1 instead.\n+ *\n * If options contains PACKET_READ_CHOMP_NEWLINE, a trailing newline (if\n * present) is removed from the buffer before returning.\n */\n #define PACKET_READ_GENTLE_ON_EOF (1u<<0)\n #define PACKET_READ_CHOMP_NEWLINE (1u<<1)\n+#define PACKET_READ_GENTLE_ALL (1u<<2)\n int packet_read(int fd, char **src_buffer, size_t *src_len, char\n \t\t*buffer, unsigned size, int options);\n \n@@ -75,6 +79,18 @@ char *packet_read_line(int fd, int *size);\n */\n char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);\n \n+/*\n+ * Same as packet_read_line, but does not die on any errors (uses\n+ * PACKET_READ_GENTLE_ALL).\n+ */\n+char *packet_read_line_gentle(int fd, int *len_p);\n+\n+/*\n+ * Same as packet_read_line_buf, but does not die on any errors (uses\n+ * PACKET_READ_GENTLE_ALL).\n+ */\n+char *packet_read_line_buf_gentle(char **src_buf, size_t *src_len, int *size);\n+\n #define DEFAULT_PACKET_MAX 1000\n #define LARGE_PACKET_MAX 65520\n extern char packet_buffer[LARGE_PACKET_MAX];\n-- \n2.7.4\n\n"},{"id":"302037","messageId":"1473984742-12516-5-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 04/11] Resumable clone: add prime-clone to remote-curl","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:15Z","receivedAt":"2016-09-16T00:12:44Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Add function and interface to handle prime-clone input, extracting\nand using duplicate functionality from discover_refs as function\nrequest_service.\n\nBecause part of our goal is for prime_clone to recover from errors,\nHTTP errors are only optionally printed to screen and never cause\ndeath in this case.\n\nSigned-off-by: Kevin Wern \n---\n remote-curl.c | 165 ++++++++++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 121 insertions(+), 44 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 15e48e2..8ebb587 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -13,6 +13,8 @@\n #include \"sha1-array.h\"\n #include \"send-pack.h\"\n \n+#define HTTP_ERROR_GENTLE (1u << 0)\n+\n static struct remote *remote;\n /* always ends with a trailing slash */\n static struct strbuf url = STRBUF_INIT;\n@@ -244,7 +246,31 @@ static int show_http_message(struct strbuf *type, struct strbuf *charset,\n \treturn 0;\n }\n \n-static struct discovery *discover_refs(const char *service, int for_push)\n+static char *http_handle_result(int http_return)\n+{\n+\tstruct strbuf error = STRBUF_INIT;\n+\n+\tswitch (http_return) {\n+\tcase HTTP_OK:\n+\t\treturn NULL;\n+\tcase HTTP_MISSING_TARGET:\n+\t\tstrbuf_addf(&error, \"repository '%s' not found\", url.buf);\n+\t\tbreak;\n+\tcase HTTP_NOAUTH:\n+\t\tstrbuf_addf(&error, \"Authentication failed for '%s'\",\n+\t\t\t url.buf);\n+\t\tbreak;\n+\tdefault:\n+\t\tstrbuf_addf(&error, \"unable to access '%s': %s\", url.buf,\n+\t\t\t curl_errorstr);\n+\t\tbreak;\n+\t}\n+\n+\treturn strbuf_detach(&error, NULL);\n+}\n+\n+static int request_service(char const *const service, char **buffer_full,\n+\t\t\t char **buffer_msg, size_t *buffer_len, int flags)\n {\n \tstruct strbuf exp = STRBUF_INIT;\n \tstruct strbuf type = STRBUF_INIT;\n@@ -252,13 +278,9 @@ static struct discovery *discover_refs(const char *service, int for_push)\n \tstruct strbuf buffer = STRBUF_INIT;\n \tstruct strbuf refs_url = STRBUF_INIT;\n \tstruct strbuf effective_url = STRBUF_INIT;\n-\tstruct discovery *last = last_discovery;\n-\tint http_ret, maybe_smart = 0;\n-\tstruct http_get_options options;\n-\n-\tif (last && !strcmp(service, last->service))\n-\t\treturn last;\n-\tfree_discovery(last);\n+\tint http_ret, maybe_smart = 0, ran_smart = 0;\n+\tstruct http_get_options get_options;\n+\tconst char *error_string;\n \n \tstrbuf_addf(&refs_url, \"%sinfo/refs\", url.buf);\n \tif ((starts_with(url.buf, \"http://\") || starts_with(url.buf, \"https://\")) &&\n@@ -271,45 +293,41 @@ static struct discovery *discover_refs(const char *service, int for_push)\n \t\tstrbuf_addf(&refs_url, \"service=%s\", service);\n \t}\n \n-\tmemset(&options, 0, sizeof(options));\n-\toptions.content_type = &type;\n-\toptions.charset = &charset;\n-\toptions.effective_url = &effective_url;\n-\toptions.base_url = &url;\n-\toptions.no_cache = 1;\n-\toptions.keep_error = 1;\n-\n-\thttp_ret = http_get_strbuf(refs_url.buf, &buffer, &options);\n-\tswitch (http_ret) {\n-\tcase HTTP_OK:\n-\t\tbreak;\n-\tcase HTTP_MISSING_TARGET:\n-\t\tshow_http_message(&type, &charset, &buffer);\n-\t\tdie(\"repository '%s' not found\", url.buf);\n-\tcase HTTP_NOAUTH:\n-\t\tshow_http_message(&type, &charset, &buffer);\n-\t\tdie(\"Authentication failed for '%s'\", url.buf);\n-\tdefault:\n-\t\tshow_http_message(&type, &charset, &buffer);\n-\t\tdie(\"unable to access '%s': %s\", url.buf, curl_errorstr);\n+\tmemset(&get_options, 0, sizeof(get_options));\n+\tget_options.content_type = &type;\n+\tget_options.charset = &charset;\n+\tget_options.effective_url = &effective_url;\n+\tget_options.base_url = &url;\n+\tget_options.no_cache = 1;\n+\tget_options.keep_error = 1;\n+\n+\thttp_ret = http_get_strbuf(refs_url.buf, &buffer, &get_options);\n+\terror_string = http_handle_result(http_ret);\n+\tif (error_string) {\n+\t\tif (!(flags & HTTP_ERROR_GENTLE)) {\n+\t\t\tshow_http_message(&type, &charset, &buffer);\n+\t\t\tdie(\"%s\", error_string);\n+\t\t}\n+\t\telse if (options.verbosity > 1) {\n+\t\t\tshow_http_message(&type, &charset, &buffer);\n+\t\t\tfprintf(stderr, \"%s\\n\", error_string);\n+\t\t}\n \t}\n \n-\tlast= xcalloc(1, sizeof(*last_discovery));\n-\tlast->service = service;\n-\tlast->buf_alloc = strbuf_detach(&buffer, &last->len);\n-\tlast->buf = last->buf_alloc;\n+\t*buffer_full = strbuf_detach(&buffer, buffer_len);\n+\t*buffer_msg = *buffer_full;\n \n \tstrbuf_addf(&exp, \"application/x-%s-advertisement\", service);\n \tif (maybe_smart &&\n-\t (5 <= last->len && last->buf[4] == '#') &&\n-\t !strbuf_cmp(&exp, &type)) {\n+\t (5 <= *buffer_len && (*buffer_msg)[4] == '#') &&\n+\t !strbuf_cmp(&exp, &type) && http_ret == HTTP_OK) {\n \t\tchar *line;\n \n \t\t/*\n \t\t * smart HTTP response; validate that the service\n \t\t * pkt-line matches our request.\n \t\t */\n-\t\tline = packet_read_line_buf(&last->buf, &last->len, NULL);\n+\t\tline = packet_read_line_buf(buffer_msg, buffer_len, NULL);\n \n \t\tstrbuf_reset(&exp);\n \t\tstrbuf_addf(&exp, \"# service=%s\", service);\n@@ -321,23 +339,80 @@ static struct discovery *discover_refs(const char *service, int for_push)\n \t\t * until a packet flush marker. Ignore these now, but\n \t\t * in the future we might start to scan them.\n \t\t */\n-\t\twhile (packet_read_line_buf(&last->buf, &last->len, NULL))\n+\t\twhile (packet_read_line_buf(buffer_msg, buffer_len, NULL))\n \t\t\t;\n \n-\t\tlast->proto_git = 1;\n+\t\tran_smart = 1;\n \t}\n \n-\tif (last->proto_git)\n-\t\tlast->refs = parse_git_refs(last, for_push);\n-\telse\n-\t\tlast->refs = parse_info_refs(last);\n-\n \tstrbuf_release(&refs_url);\n \tstrbuf_release(&exp);\n \tstrbuf_release(&type);\n \tstrbuf_release(&charset);\n \tstrbuf_release(&effective_url);\n \tstrbuf_release(&buffer);\n+\n+\treturn ran_smart;\n+}\n+\n+static void prime_clone(void)\n+{\n+\tchar *result, *result_full, *line;\n+\tsize_t result_len;\n+\tint err = 0, one_successful = 0;\n+\n+\tif (request_service(\"git-prime-clone\", &result_full, &result,\n+\t\t\t&result_len, HTTP_ERROR_GENTLE)) {\n+\t\twhile (line = packet_read_line_buf_gentle(&result, &result_len,\n+\t\t\t\t\t\t\t NULL)) {\n+\t\t\tchar *space = strchr(line ,' ');\n+\n+\t\t\t// We will eventually support multiple resources, so\n+\t\t\t// always parse the whole message\n+\t\t\tif (err)\n+\t\t\t\tcontinue;\n+\t\t\tif (!space || strchr(space + 1, ' ')) {\n+\t\t\t\tif (options.verbosity > 1)\n+\t\t\t\t\tfprintf(stderr, \"prime clone \"\n+\t\t\t\t\t\t\"protocol error: got '%s'\\n\",\n+\t\t\t\t\t\tline);\n+\t\t\t\tprintf(\"error\\n\");\n+\t\t\t\terr = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tone_successful = 1;\n+\t\t\tprintf(\"%s\\n\", line);\n+\t\t}\n+\t\tif (!one_successful && options.verbosity > 1)\n+\t\t\tfprintf(stderr, \"did not get required components for \"\n+\t\t\t\t\"alternate resource\\n\");\n+\t}\n+\n+\tprintf(\"\\n\");\n+\tfflush(stdout);\n+\tfree(result_full);\n+}\n+\n+\n+static struct discovery *discover_refs(const char *service, int for_push)\n+{\n+\tstruct discovery *last = last_discovery;\n+\n+\tif (last && !strcmp(service, last->service))\n+\t\treturn last;\n+\tfree_discovery(last);\n+\n+\tlast= xcalloc(1, sizeof(*last_discovery));\n+\tlast->service = service;\n+\tlast->proto_git = request_service(service, &last->buf_alloc,\n+\t\t\t\t\t &last->buf, &last->len, 0);\n+\n+\tif (last->proto_git)\n+\t\tlast->refs = parse_git_refs(last, for_push);\n+\telse\n+\t\tlast->refs = parse_info_refs(last);\n+\n \tlast_discovery = last;\n \treturn last;\n }\n@@ -1030,7 +1105,8 @@ int main(int argc, const char **argv)\n \t\t} else if (!strcmp(buf.buf, \"list\") || starts_with(buf.buf, \"list \")) {\n \t\t\tint for_push = !!strstr(buf.buf + 4, \"for-push\");\n \t\t\toutput_refs(get_refs(for_push));\n-\n+\t\t} else if (!strcmp(buf.buf, \"prime-clone\")) {\n+\t\t\tprime_clone();\n \t\t} else if (starts_with(buf.buf, \"push \")) {\n \t\t\tparse_push(&buf);\n \n@@ -1056,6 +1132,7 @@ int main(int argc, const char **argv)\n \t\t\tprintf(\"fetch\\n\");\n \t\t\tprintf(\"option\\n\");\n \t\t\tprintf(\"push\\n\");\n+\t\t\tprintf(\"prime-clone\\n\");\n \t\t\tprintf(\"check-connectivity\\n\");\n \t\t\tprintf(\"\\n\");\n \t\t\tfflush(stdout);\n-- \n2.7.4\n\n"},{"id":"302038","messageId":"1473984742-12516-6-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 05/11] Resumable clone: add output parsing to connect.c","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:16Z","receivedAt":"2016-09-16T00:12:47Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Add method for transport to call when parsing primeclone output from\nstdin. Suppress stderr when using git_connect with ssh, unless output\nis verbose.\n\nSigned-off-by: Kevin Wern \n---\n connect.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++\n connect.h | 10 ++++++----\n 2 files changed, 53 insertions(+), 4 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 0478631..284de53 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -804,6 +804,10 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\t}\n \t\targv_array_push(&conn->args, cmd.buf);\n \n+\t\tif (flags & CONNECT_SUPPRESS_ERRORS) {\n+\t\t\tconn->no_stderr = 1;\n+\t\t}\n+\n \t\tif (start_command(conn))\n \t\t\tdie(\"unable to fork\");\n \n@@ -831,3 +835,46 @@ int finish_connect(struct child_process *conn)\n \tfree(conn);\n \treturn code;\n }\n+\n+const struct alt_resource *const get_alt_res_connect(int fd, int flags)\n+{\n+\tstruct alt_resource *res = NULL;\n+\tconst char *line;\n+\tchar *url = NULL, *filetype = NULL;\n+\tchar *error_string = NULL;\n+\n+\twhile (line = packet_read_line_gentle(fd, NULL)) {\n+\t\tconst char *space = strchr(line, ' ');\n+\n+\t\t// We will eventually support multiple resources, so always\n+\t\t// parse the whole message\n+\t\tif ((filetype && url) || error_string) {\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (skip_prefix(line, \"ERR \", &line) || !space ||\n+\t\t\t\tstrchr(space + 1, ' ')) {\n+\t\t\terror_string = xstrdup(line);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfiletype = xstrndup(line, (space - line));\n+\t\turl = xstrdup(space + 1);\n+\t}\n+\n+\tif (filetype && url && !error_string){\n+\t\tres = xcalloc(1, sizeof(*res));\n+\t\tres->filetype = filetype;\n+\t\tres->url = url;\n+\t}\n+\telse {\n+\t\tif (!(flags & CONNECT_SUPPRESS_ERRORS)) {\n+\t\t\tif (error_string)\n+\t\t\t\tfprintf(stderr, \"prime clone protocol error: \"\n+\t\t\t\t\t\"got '%s'\\n\", error_string);\n+\t\t\telse\n+\t\t\t\tfprintf(stderr, \"did not get required \"\n+\t\t\t\t\t\"components for alternate resource\\n\");\n+\t\t}\n+\t}\n+\n+\treturn res;\n+}\ndiff --git a/connect.h b/connect.h\nindex 01f14cd..966c0eb 100644\n--- a/connect.h\n+++ b/connect.h\n@@ -1,10 +1,11 @@\n #ifndef CONNECT_H\n #define CONNECT_H\n \n-#define CONNECT_VERBOSE (1u << 0)\n-#define CONNECT_DIAG_URL (1u << 1)\n-#define CONNECT_IPV4 (1u << 2)\n-#define CONNECT_IPV6 (1u << 3)\n+#define CONNECT_VERBOSE (1u << 0)\n+#define CONNECT_DIAG_URL (1u << 1)\n+#define CONNECT_IPV4 (1u << 2)\n+#define CONNECT_IPV6 (1u << 3)\n+#define CONNECT_SUPPRESS_ERRORS (1u << 4)\n extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n extern int finish_connect(struct child_process *conn);\n extern int git_connection_is_socket(struct child_process *conn);\n@@ -12,5 +13,6 @@ extern int server_supports(const char *feature);\n extern int parse_feature_request(const char *features, const char *feature);\n extern const char *server_feature_value(const char *feature, int *len_ret);\n extern int url_is_local_not_ssh(const char *url);\n+const struct alt_resource *const get_alt_res_connect(int fd, int flags);\n \n #endif\n-- \n2.7.4\n\n"},{"id":"302039","messageId":"1473984742-12516-7-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 06/11] Resumable clone: implement transport_prime_clone","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:17Z","receivedAt":"2016-09-16T00:12:50Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Create transport_prime_clone API, as well as all internal methods.\nCreate representations of alt_resource and prime-clone path options.\n\nThe intention of get_alt_res_helper is solely to parse the output of\nremote-curl because transport-helper does not handle verbose options\nor speaking to the user verbosely. Therefore, all error parsing is\ndone with remote-curl, and any protocol breach between remote-curl and\ntransport-helper will treated as a bug and result in death.\n\nSigned-off-by: Kevin Wern \n---\n transport-helper.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-\n transport.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n transport.h | 20 ++++++++++++++++++++\n 3 files changed, 114 insertions(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex b934183..eb185d5 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -28,7 +28,8 @@ struct helper_data {\n \t\tsigned_tags : 1,\n \t\tcheck_connectivity : 1,\n \t\tno_disconnect_req : 1,\n-\t\tno_private_update : 1;\n+\t\tno_private_update : 1,\n+\t\tprime_clone : 1;\n \tchar *export_marks;\n \tchar *import_marks;\n \t/* These go from remote name (as in \"list\") to private name */\n@@ -180,6 +181,8 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\tdata->export = 1;\n \t\telse if (!strcmp(capname, \"check-connectivity\"))\n \t\t\tdata->check_connectivity = 1;\n+\t\telse if (!strcmp(capname, \"prime-clone\"))\n+\t\t\tdata->prime_clone = 1;\n \t\telse if (!data->refspecs && skip_prefix(capname, \"refspec \", &arg)) {\n \t\t\tALLOC_GROW(refspecs,\n \t\t\t\t refspec_nr + 1,\n@@ -248,6 +251,7 @@ static int disconnect_helper(struct transport *transport)\n }\n \n static const char *unsupported_options[] = {\n+\tTRANS_OPT_PRIMECLONE,\n \tTRANS_OPT_UPLOADPACK,\n \tTRANS_OPT_RECEIVEPACK,\n \tTRANS_OPT_THIN,\n@@ -1054,6 +1058,50 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \treturn ret;\n }\n \n+static const struct alt_resource *const get_alt_res_helper(struct transport *transport)\n+{\n+\tstruct helper_data *data = transport->data;\n+\tchar *url = NULL, *filetype = NULL;\n+\tstruct alt_resource *ret = NULL;\n+\tstruct strbuf out = STRBUF_INIT;\n+\tstruct child_process *helper = get_helper(transport);\n+\tint err = 0;\n+\n+\thelper = get_helper(transport);\n+\twrite_constant(helper->in, \"prime-clone\\n\");\n+\n+\twhile (!recvline(data, &out)) {\n+\t\tchar *space = strchr(out.buf, ' ');\n+\n+\t\tif (!*out.buf)\n+\t\t\tbreak;\n+\n+\t\tif (starts_with(out.buf, \"error\")) {\n+\t\t\terr = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!space || strchr(space + 1, ' '))\n+\t\t\tdie(\"malformed alternate resource response: %s\\n\",\n+\t\t\t out.buf);\n+\n+\t\tif ((filetype && url) || err)\n+\t\t\tcontinue;\n+\n+\t\tfiletype = xstrndup(out.buf, (space - out.buf));\n+\t\turl = xstrdup(space + 1);\n+\t}\n+\n+\tif (filetype && url && !err) {\n+\t\tret = xcalloc(1, sizeof(*ret));\n+\t\tret->filetype = filetype;\n+\t\tret->url = url;\n+\t}\n+\n+\tstrbuf_release(&out);\n+\treturn ret;\n+}\n+\n int transport_helper_init(struct transport *transport, const char *name)\n {\n \tstruct helper_data *data = xcalloc(1, sizeof(*data));\n@@ -1067,6 +1115,7 @@ int transport_helper_init(struct transport *transport, const char *name)\n \ttransport->data = data;\n \ttransport->set_option = set_helper_option;\n \ttransport->get_refs_list = get_refs_list;\n+\ttransport->prime_clone = get_alt_res_helper;\n \ttransport->fetch = fetch;\n \ttransport->push_refs = push_refs;\n \ttransport->disconnect = release_helper;\ndiff --git a/transport.c b/transport.c\nindex 7bd3206..dd0d839 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -131,6 +131,9 @@ static int set_git_option(struct git_transport_options *opts,\n \t} else if (!strcmp(name, TRANS_OPT_RECEIVEPACK)) {\n \t\topts->receivepack = value;\n \t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_PRIMECLONE)) {\n+\t\topts->primeclone = value;\n+\t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_THIN)) {\n \t\topts->thin = !!value;\n \t\treturn 0;\n@@ -533,6 +536,42 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \treturn ret;\n }\n \n+const struct alt_resource *const get_alt_res_via_connect(struct transport *transport)\n+{\n+\tstruct git_transport_data *data = transport->data;\n+\tconst struct alt_resource *res = NULL;\n+\tint flags = transport->verbose > 0 ? 0 : CONNECT_SUPPRESS_ERRORS;\n+\n+\tdata->conn = git_connect(data->fd, transport->url,\n+\t\t\t\t transport->smart_options->primeclone, flags);\n+\tres = get_alt_res_connect(data->fd[0], flags);\n+\n+\tclose(data->fd[0]);\n+\tclose(data->fd[1]);\n+\tfinish_connect(data->conn);\n+\tdata->conn = NULL;\n+\n+\treturn res;\n+}\n+\n+const struct alt_resource *const transport_prime_clone(struct transport *transport)\n+{\n+\tif (transport->prime_clone && !transport->alt_res)\n+\t\ttransport->alt_res = transport->prime_clone(transport);\n+\tif (transport->verbose > 0) {\n+\t\tif (transport->alt_res)\n+\t\t\t// redundant at this point, but will be\n+\t\t\t// more useful in future iterations with\n+\t\t\t// lists of potential resources\n+\t\t\tfprintf(stderr, \"alt resource found: %s (%s)\\n\",\n+\t\t\t\ttransport->alt_res->url,\n+\t\t\t\ttransport->alt_res->filetype);\n+\t\telse\n+\t\t\tfprintf(stderr, \"alt res not found\\n\");\n+\t}\n+\treturn transport->alt_res;\n+}\n+\n static int connect_git(struct transport *transport, const char *name,\n \t\t const char *executable, int fd[2])\n {\n@@ -691,6 +730,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\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->prime_clone = get_alt_res_via_connect;\n \t\tret->push_refs = git_transport_push;\n \t\tret->connect = connect_git;\n \t\tret->disconnect = disconnect_git;\n@@ -713,6 +753,10 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->smart_options->receivepack = \"git-receive-pack\";\n \t\tif (remote->receivepack)\n \t\t\tret->smart_options->receivepack = remote->receivepack;\n+\t\t// No remote.*.primeclone config because prime-clone only\n+\t\t// applies to clone. After that, it is never used the repo\n+\t\t// again.\n+\t\tret->smart_options->primeclone = \"git-prime-clone\";\n \t}\n \n \treturn ret;\ndiff --git a/transport.h b/transport.h\nindex c681408..2bb6963 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -15,6 +15,7 @@ struct git_transport_options {\n \tint depth;\n \tconst char *uploadpack;\n \tconst char *receivepack;\n+\tconst char *primeclone;\n \tstruct push_cas_option *cas;\n };\n \n@@ -24,11 +25,17 @@ enum transport_family {\n \tTRANSPORT_FAMILY_IPV6\n };\n \n+struct alt_resource {\n+\tchar *url;\n+\tchar *filetype;\n+};\n+\n struct transport {\n \tstruct remote *remote;\n \tconst char *url;\n \tvoid *data;\n \tconst struct ref *remote_refs;\n+\tconst struct alt_resource *alt_res;\n \n \t/**\n \t * Indicates whether we already called get_refs_list(); set by\n@@ -68,6 +75,15 @@ struct transport {\n \tstruct ref *(*get_refs_list)(struct transport *transport, int for_push);\n \n \t/**\n+\t * Returns the location of an alternate resource to fetch before\n+\t * cloning.\n+\t *\n+\t * If the transport cannot determine an alternate resource, then\n+\t * NULL is returned.\n+\t **/\n+\tconst struct alt_resource *const (*prime_clone)(struct transport *transport);\n+\n+\t/**\n \t * Fetch the objects for the given refs. Note that this gets\n \t * an array, and should ignore the list structure.\n \t *\n@@ -164,6 +180,9 @@ int transport_restrict_protocols(void);\n /* The program to use on the remote side to send a pack */\n #define TRANS_OPT_UPLOADPACK \"uploadpack\"\n \n+/* The program to use on the remote side to check for alternate resource */\n+#define TRANS_OPT_PRIMECLONE \"primeclone\"\n+\n /* The program to use on the remote side to receive a pack */\n #define TRANS_OPT_RECEIVEPACK \"receivepack\"\n \n@@ -208,6 +227,7 @@ int transport_push(struct transport *connection,\n \t\t unsigned int * reject_reasons);\n \n const struct ref *transport_get_remote_refs(struct transport *transport);\n+const struct alt_resource *const transport_prime_clone(struct transport *transport);\n \n int transport_fetch_refs(struct transport *transport, struct ref *refs);\n void transport_unlock_pack(struct transport *transport);\n-- \n2.7.4\n\n"},{"id":"302040","messageId":"1473984742-12516-8-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 07/11] Resumable clone: add resumable download to http/curl","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:18Z","receivedAt":"2016-09-16T00:12:53Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Create resumable download procedure and progress display function.\nThe conversion from B to KB occurs because otherwise the byte counts\nfor large repos (i.e. Linux) overflow calculating percentage.\n\nThe download protocol includes the resource's URL, and the directory\nthe resource will be downloaded to. The url passed to remote-curl on\ninvocation does not matter (git clone will use the resource url\nagain here).\n\nSigned-off-by: Kevin Wern \n---\n http.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n http.h | 7 ++++-\n remote-curl.c | 27 +++++++++++++++++++\n 3 files changed, 118 insertions(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 1d5e3bb..93d6324 100644\n--- a/http.c\n+++ b/http.c\n@@ -10,6 +10,8 @@\n #include \"pkt-line.h\"\n #include \"gettext.h\"\n #include \"transport.h\"\n+#include \"progress.h\"\n+#include \"dir.h\"\n \n #if LIBCURL_VERSION_NUM >= 0x070a08\n long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;\n@@ -1136,7 +1138,10 @@ static int handle_curl_result(struct slot_results *results)\n \t\t\t\tcurl_easy_strerror(results->curl_result),\n \t\t\t\tsizeof(curl_errorstr));\n #endif\n-\t\treturn HTTP_ERROR;\n+\t\tif (results->http_code >= 400)\n+\t\t\treturn HTTP_ERROR;\n+\t\telse\n+\t\t\treturn HTTP_ERROR_RESUMABLE;\n \t}\n }\n \n@@ -1365,6 +1370,40 @@ static void http_opt_request_remainder(CURL *curl, off_t pos)\n #define HTTP_REQUEST_STRBUF\t0\n #define HTTP_REQUEST_FILE\t1\n \n+static int bytes_to_rounded_kb(double bytes)\n+{\n+\treturn (int) (bytes + 512)/1024;\n+}\n+\n+int progress_func(void *data, double total_to_download, double now_downloaded,\n+\t\t double total_to_upload, double now_uploadeded)\n+{\n+\tstruct progress **progress = data;\n+\tint kilobytes = total_to_download >= 1024;\n+\n+\tif (total_to_download <= 0.0) {\n+\t\treturn 0;\n+\t}\n+\tif (kilobytes) {\n+\t\tnow_downloaded = bytes_to_rounded_kb(now_downloaded);\n+\t\ttotal_to_download = bytes_to_rounded_kb(total_to_download);\n+\t}\n+\tif (!*progress && now_downloaded < total_to_download) {\n+\t\tif (total_to_download > 1024)\n+\t\t\t*progress = start_progress(\"Downloading (KB)\",\n+\t\t\t\t\t\t total_to_download);\n+\t\telse\n+\t\t\t*progress = start_progress(\"Downloading (B)\",\n+\t\t\t\t\t\t total_to_download);\n+\t}\n+\tdisplay_progress(*progress, now_downloaded);\n+\tif (now_downloaded == total_to_download) {\n+\t\tstop_progress(progress);\n+\t}\n+\treturn 0;\n+}\n+\n+\n static int http_request(const char *url,\n \t\t\tvoid *result, int target,\n \t\t\tconst struct http_get_options *options)\n@@ -1373,6 +1412,7 @@ static int http_request(const char *url,\n \tstruct slot_results results;\n \tstruct curl_slist *headers = NULL;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct progress *progress = NULL;\n \tconst char *accept_language;\n \tint ret;\n \n@@ -1389,6 +1429,16 @@ static int http_request(const char *url,\n \t\t\toff_t posn = ftello(result);\n \t\t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,\n \t\t\t\t\t fwrite);\n+\t\t\tif (options && options->progress) {\n+\t\t\t\tcurl_easy_setopt(slot->curl,\n+\t\t\t\t\t\t CURLOPT_NOPROGRESS, 0);\n+\t\t\t\tcurl_easy_setopt(slot->curl,\n+\t\t\t\t\t\t CURLOPT_PROGRESSFUNCTION,\n+\t\t\t\t\t\t progress_func);\n+\t\t\t\tcurl_easy_setopt(slot->curl,\n+\t\t\t\t\t\t CURLOPT_PROGRESSDATA,\n+\t\t\t\t\t\t &progress);\n+\t\t\t}\n \t\t\tif (posn > 0)\n \t\t\t\thttp_opt_request_remainder(slot->curl, posn);\n \t\t} else\n@@ -1559,6 +1609,40 @@ cleanup:\n \treturn ret;\n }\n \n+int http_download_primer(const char *url, const char *out_file)\n+{\n+\tint ret = 0, try_count = HTTP_TRY_COUNT;\n+\tstruct http_get_options options = {0};\n+\toptions.progress = 1;\n+\n+\tif (file_exists(out_file)) {\n+\t\tfprintf(stderr,\n+\t\t\t\"File already downloaded: '%s', skipping...\\n\",\n+\t\t\tout_file);\n+\t\treturn ret;\n+\t}\n+\n+\tdo {\n+\t\tif (try_count != HTTP_TRY_COUNT) {\n+\t\t\tfprintf(stderr, \"Connection interrupted for some \"\n+\t\t\t\t\"reason, retrying (%d attempts left)\\n\",\n+\t\t\t\ttry_count);\n+\t\t\tstruct timeval time = {10, 0}; // 1s\n+\t\t\tselect(0, NULL, NULL, NULL, &time);\n+\t\t}\n+\t\tret = http_get_file(url, out_file, &options);\n+\t\ttry_count--;\n+\t} while (try_count > 0 && ret == HTTP_ERROR_RESUMABLE);\n+\n+\tif (ret != HTTP_OK) {\n+\t\terror(\"Unable to get resource: %s\", url);\n+\t\tret = -1;\n+\t}\n+\n+\treturn ret;\n+}\n+\n+\n int http_fetch_ref(const char *base, struct ref *ref)\n {\n \tstruct http_get_options options = {0};\ndiff --git a/http.h b/http.h\nindex 4ef4bbd..6a7ce7b 100644\n--- a/http.h\n+++ b/http.h\n@@ -138,7 +138,8 @@ extern char *get_remote_object_url(const char *url, const char *hex,\n /* Options for http_get_*() */\n struct http_get_options {\n \tunsigned no_cache:1,\n-\t\t keep_error:1;\n+\t\t keep_error:1,\n+\t\t progress:1;\n \n \t/* If non-NULL, returns the content-type of the response. */\n \tstruct strbuf *content_type;\n@@ -172,6 +173,7 @@ struct http_get_options {\n #define HTTP_START_FAILED\t3\n #define HTTP_REAUTH\t4\n #define HTTP_NOAUTH\t5\n+#define HTTP_ERROR_RESUMABLE\t6\n \n /*\n * Requests a URL and stores the result in a strbuf.\n@@ -180,6 +182,9 @@ struct http_get_options {\n */\n int http_get_strbuf(const char *url, struct strbuf *result, struct http_get_options *options);\n \n+#define HTTP_TRY_COUNT 5\n+int http_download_primer(const char *url, const char *out_file);\n+\n extern int http_fetch_ref(const char *base, struct ref *ref);\n \n /* Helpers for fetching packs */\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 8ebb587..051ba52 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -394,6 +394,30 @@ static void prime_clone(void)\n \tfree(result_full);\n }\n \n+static void download_primer(const char *url, const char *base_dir)\n+{\n+\tchar *slash_ptr = strchr(url, '/'), *out_file;\n+\tstruct strbuf out_path = STRBUF_INIT;\n+\tdo {\n+\t\tout_file = slash_ptr + 1;\n+\t} while (slash_ptr = strchr(out_file, '/'));\n+\tstrbuf_addf(&out_path, \"%s/%s\", base_dir, out_file);\n+\tif (!http_download_primer(url, out_path.buf))\n+\t\tprintf(\"%s\\n\", out_path.buf);\n+\tprintf(\"\\n\");\n+\tfflush(stdout);\n+}\n+\n+static void parse_download_primer(struct strbuf *buf)\n+{\n+\tconst char *remote_url;\n+\tif (skip_prefix(buf->buf, \"download-primer \", &remote_url)) {\n+\t\tchar *base_path;\n+\t\tbase_path = strchr(remote_url, ' ');\n+\t\t*base_path++ = '\\0';\n+\t\tdownload_primer(remote_url, base_path);\n+\t}\n+}\n \n static struct discovery *discover_refs(const char *service, int for_push)\n {\n@@ -1105,6 +1129,8 @@ int main(int argc, const char **argv)\n \t\t} else if (!strcmp(buf.buf, \"list\") || starts_with(buf.buf, \"list \")) {\n \t\t\tint for_push = !!strstr(buf.buf + 4, \"for-push\");\n \t\t\toutput_refs(get_refs(for_push));\n+\t\t} else if (starts_with(buf.buf, \"download-primer\")) {\n+\t\t\tparse_download_primer(&buf);\n \t\t} else if (!strcmp(buf.buf, \"prime-clone\")) {\n \t\t\tprime_clone();\n \t\t} else if (starts_with(buf.buf, \"push \")) {\n@@ -1132,6 +1158,7 @@ int main(int argc, const char **argv)\n \t\t\tprintf(\"fetch\\n\");\n \t\t\tprintf(\"option\\n\");\n \t\t\tprintf(\"push\\n\");\n+\t\t\tprintf(\"download-primer\\n\");\n \t\t\tprintf(\"prime-clone\\n\");\n \t\t\tprintf(\"check-connectivity\\n\");\n \t\t\tprintf(\"\\n\");\n-- \n2.7.4\n\n"},{"id":"302041","messageId":"1473984742-12516-9-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 08/11] Resumable clone: create transport_download_primer","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:19Z","receivedAt":"2016-09-16T00:12:55Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Create function transport_download_primer and components\nto invoke and pass commands to remote-curl.\n\nSigned-off-by: Kevin Wern \n---\n transport-helper.c | 24 ++++++++++++++++++++++++\n transport.c | 9 +++++++++\n transport.h | 7 +++++++\n 3 files changed, 40 insertions(+)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex eb185d5..2ff96ef 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -29,6 +29,7 @@ struct helper_data {\n \t\tcheck_connectivity : 1,\n \t\tno_disconnect_req : 1,\n \t\tno_private_update : 1,\n+\t\tdownload_primer : 1,\n \t\tprime_clone : 1;\n \tchar *export_marks;\n \tchar *import_marks;\n@@ -183,6 +184,8 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\tdata->check_connectivity = 1;\n \t\telse if (!strcmp(capname, \"prime-clone\"))\n \t\t\tdata->prime_clone = 1;\n+\t\telse if (!strcmp(capname, \"download-primer\"))\n+\t\t\tdata->download_primer = 1;\n \t\telse if (!data->refspecs && skip_prefix(capname, \"refspec \", &arg)) {\n \t\t\tALLOC_GROW(refspecs,\n \t\t\t\t refspec_nr + 1,\n@@ -1058,6 +1061,26 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \treturn ret;\n }\n \n+static const char *download_primer(struct transport *transport,\n+\t\t\t\t const struct alt_resource *res,\n+\t\t\t\t const char *base_path)\n+{\n+\tstruct helper_data *data = transport->data;\n+\tstruct child_process *helper;\n+\tstruct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;\n+\tchar *ret = NULL;\n+\n+\thelper = get_helper(transport);\n+\n+\tstrbuf_addf(&buf, \"download-primer %s %s\\n\", res->url, base_path);\n+\tsendline(data, &buf);\n+\trecvline(data, &out);\n+\tstrbuf_release(&buf);\n+\tif (out.len > 0)\n+\t\tret = strbuf_detach(&out, NULL);\n+\treturn ret;\n+}\n+\n static const struct alt_resource *const get_alt_res_helper(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n@@ -1115,6 +1138,7 @@ int transport_helper_init(struct transport *transport, const char *name)\n \ttransport->data = data;\n \ttransport->set_option = set_helper_option;\n \ttransport->get_refs_list = get_refs_list;\n+\ttransport->download_primer = download_primer;\n \ttransport->prime_clone = get_alt_res_helper;\n \ttransport->fetch = fetch;\n \ttransport->push_refs = push_refs;\ndiff --git a/transport.c b/transport.c\nindex dd0d839..3b33029 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -572,6 +572,15 @@ const struct alt_resource *const transport_prime_clone(struct transport *transpo\n \treturn transport->alt_res;\n }\n \n+const char *transport_download_primer(struct transport *transport,\n+\t\t\t\t const struct alt_resource *alt_res,\n+\t\t\t\t const char *base_dir)\n+{\n+\tif (transport->download_primer)\n+\t\treturn transport->download_primer(transport, alt_res, base_dir);\n+\treturn NULL;\n+}\n+\n static int connect_git(struct transport *transport, const char *name,\n \t\t const char *executable, int fd[2])\n {\ndiff --git a/transport.h b/transport.h\nindex 2bb6963..1484d6d 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -83,6 +83,10 @@ struct transport {\n \t **/\n \tconst struct alt_resource *const (*prime_clone)(struct transport *transport);\n \n+\tconst char *(*download_primer)(struct transport *transport,\n+\t\t\tconst struct alt_resource *alt_res,\n+\t\t\tconst char *base_path);\n+\n \t/**\n \t * Fetch the objects for the given refs. Note that this gets\n \t * an array, and should ignore the list structure.\n@@ -228,6 +232,9 @@ int transport_push(struct transport *connection,\n \n const struct ref *transport_get_remote_refs(struct transport *transport);\n const struct alt_resource *const transport_prime_clone(struct transport *transport);\n+const char *transport_download_primer(struct transport *transport,\n+\t\tconst struct alt_resource *alt_res,\n+\t\tconst char *base_path);\n \n int transport_fetch_refs(struct transport *transport, struct ref *refs);\n void transport_unlock_pack(struct transport *transport);\n-- \n2.7.4\n\n"},{"id":"302042","messageId":"1473984742-12516-11-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:21Z","receivedAt":"2016-09-16T00:12:59Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child\nprocess.\n\nThis will be used by git clone when calling index-pack on a downloaded\npackfile.\n\nSigned-off-by: Kevin Wern \n---\n run-command.c | 1 +\n run-command.h | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 863dad5..c4f82f9 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -574,6 +574,7 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const\n \tcmd.silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;\n \tcmd.use_shell = opt & RUN_USING_SHELL ? 1 : 0;\n \tcmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;\n+\tcmd.no_stdout = opt & RUN_COMMAND_NO_STDOUT ? 1 : 0;\n \tcmd.dir = dir;\n \tcmd.env = env;\n \treturn run_command(&cmd);\ndiff --git a/run-command.h b/run-command.h\nindex 42917e8..2d2c871 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -70,6 +70,7 @@ extern int run_hook_ve(const char *const *env, const char *name, va_list args);\n #define RUN_SILENT_EXEC_FAILURE 8\n #define RUN_USING_SHELL 16\n #define RUN_CLEAN_ON_EXIT 32\n+#define RUN_COMMAND_NO_STDOUT 64\n int run_command_v_opt(const char **argv, int opt);\n \n /*\n-- \n2.7.4\n\n"},{"id":"302043","messageId":"1473984742-12516-10-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 09/11] path: add resumable marker","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:20Z","receivedAt":"2016-09-16T00:13:02Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Create function to get gitdir file RESUMABLE.\n\nSigned-off-by: Kevin Wern \n---\n cache.h | 1 +\n path.c | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex d7ff46e..1f4117c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -811,6 +811,7 @@ const char *git_path_merge_mode(void);\n const char *git_path_merge_head(void);\n const char *git_path_fetch_head(void);\n const char *git_path_shallow(void);\n+const char *git_path_resumable(void);\n \n /*\n * Return the name of the file in the local object database that would\ndiff --git a/path.c b/path.c\nindex 8b7e168..9360ed9 100644\n--- a/path.c\n+++ b/path.c\n@@ -1201,4 +1201,5 @@ GIT_PATH_FUNC(git_path_merge_rr, \"MERGE_RR\")\n GIT_PATH_FUNC(git_path_merge_mode, \"MERGE_MODE\")\n GIT_PATH_FUNC(git_path_merge_head, \"MERGE_HEAD\")\n GIT_PATH_FUNC(git_path_fetch_head, \"FETCH_HEAD\")\n+GIT_PATH_FUNC(git_path_resumable, \"RESUMABLE\")\n GIT_PATH_FUNC(git_path_shallow, \"shallow\")\n-- \n2.7.4\n\n"},{"id":"302044","messageId":"1473984742-12516-12-git-send-email-kevin.m.wern@gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"[PATCH 11/11] Resumable clone: implement primer logic in git-clone","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-16T00:12:22Z","receivedAt":"2016-09-16T00:13:06Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"Use transport_download_primer and transport_prime_clone in git clone.\nThis only supports a fully connected packfile.\n\ntransport_prime_clone and transport_download_primer are executed\ncompletely independent of transport_(get|fetch)_remote_refs, et al.\ntransport_download_primer is executed based on the existence of an\nalt_resource. The idea is that the \"prime clone\" execution should be\nable to attempt retrieving an alternate resource without dying, as\nopposed to depending on the result of upload pack's \"capabilities\" to\nindicate whether or not the client can attempt it.\n\nIf a resumable resource is available, execute a codepath with the\nfollowing modular components:\n- downloading resource to a specific directory\n- using the resource (for pack, indexing and generating the bundle\n file)\n- cleaning up the resource (if the download or use fails)\n- cleaning up the resource (if the download or use succeeds)\n\nIf resume is interrupted on the client side, the alternate resource\ninfo is written to the RESUMABLE file in the git directory.\n\nOn resume, the required info is extracted by parsing the created\nconfig file, and that info is used to determine the work and git\ndirectories. If these cannot be determined, the program exits.\nThe writing of the refspec and determination of the initial git\ndirectories are skipped, along with transport_prime_clone.\n\nThe main purpose of this series of patches is to flesh out a codepath\nfor automatic resuming, manual resuming, and leaving a resumable\ndirectory on exit--the logic for when to do these still needs more\nwork.\n\nSigned-off-by: Kevin Wern \n---\n Documentation/git-clone.txt | 16 ++\n builtin/clone.c | 590 +++++++++++++++++++++++++++++++++++++-------\n t/t9904-git-prime-clone.sh | 181 ++++++++++++++\n 3 files changed, 698 insertions(+), 89 deletions(-)\n create mode 100755 t/t9904-git-prime-clone.sh\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex b7c467a..5934bb6 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -16,6 +16,7 @@ SYNOPSIS\n \t [--depth ] [--[no-]single-branch]\n \t [--recursive | --recurse-submodules] [--] \n \t []\n+'git clone --resume '\n \n DESCRIPTION\n -----------\n@@ -172,6 +173,12 @@ objects from the source repository into a pack in the cloned repository.\n \tvia ssh, this specifies a non-default path for the command\n \trun on the other end.\n \n+--prime-clone ::\n+-p ::\n+\tWhen given and the repository to clone from is accessed\n+\tvia ssh, this specifies a non-default path for the command\n+\trun on the other end.\n+\n --template=::\n \tSpecify the directory from which templates will be used;\n \t(See the \"TEMPLATE DIRECTORY\" section of linkgit:git-init[1].)\n@@ -232,6 +239,15 @@ objects from the source repository into a pack in the cloned repository.\n \tfor `host.xz:foo/.git`). Cloning into an existing directory\n \tis only allowed if the directory is empty.\n \n+--resume::\n+\tResume a partially cloned repo in a \"resumable\" state. This\n+\tcan only be specified with a single local directory (). This is incompatible with all other options.\n+\n+::\n+\tThe directory of the partial clone. This could be either the\n+\twork directory or the git directory.\n+\n :git-clone: 1\n include::urls.txt[]\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 9ac6c01..d9a13dc 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -8,7 +8,9 @@\n * Clone a repository into a different directory that does not yet exist.\n */\n \n+#include \"cache.h\"\n #include \"builtin.h\"\n+#include \"bundle.h\"\n #include \"lockfile.h\"\n #include \"parse-options.h\"\n #include \"fetch-pack.h\"\n@@ -40,17 +42,20 @@ static const char * const builtin_clone_usage[] = {\n \n static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n+static int option_resume;\n static char *option_template, *option_depth;\n-static char *option_origin = NULL;\n+static const char *option_origin = NULL;\n static char *option_branch = NULL;\n static const char *real_git_dir;\n static char *option_upload_pack = \"git-upload-pack\";\n+static char *option_prime_clone = \"git-prime-clone\";\n static int option_verbosity;\n static int option_progress = -1;\n static enum transport_family family;\n static struct string_list option_config;\n static struct string_list option_reference;\n static int option_dissociate;\n+static const struct alt_resource *alt_res = NULL;\n \n static struct option builtin_clone_options[] = {\n \tOPT__VERBOSITY(&option_verbosity),\n@@ -85,10 +90,14 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"checkout instead of the remote's HEAD\")),\n \tOPT_STRING('u', \"upload-pack\", &option_upload_pack, N_(\"path\"),\n \t\t N_(\"path to git-upload-pack on the remote\")),\n+\tOPT_STRING('p', \"prime-clone\", &option_prime_clone, N_(\"path\"),\n+\t\t N_(\"path to git-prime-clone on the remote\")),\n \tOPT_STRING(0, \"depth\", &option_depth, N_(\"depth\"),\n \t\t N_(\"create a shallow clone of that depth\")),\n \tOPT_BOOL(0, \"single-branch\", &option_single_branch,\n \t\t N_(\"clone only one branch, HEAD or --branch\")),\n+\tOPT_BOOL(0, \"resume\", &option_resume,\n+\t\t N_(\"continue a resumable clone\")),\n \tOPT_STRING(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n \t\t N_(\"separate git dir from working tree\")),\n \tOPT_STRING_LIST('c', \"config\", &option_config, N_(\"key=value\"),\n@@ -278,6 +287,21 @@ static void strip_trailing_slashes(char *dir)\n \t*end = '\\0';\n }\n \n+static char *get_filename(const char *dir)\n+{\n+\tchar *dir_copy = xstrdup(dir);\n+\tstrip_trailing_slashes(dir_copy);\n+\tchar *filename, *final = NULL;\n+\n+\tfilename = find_last_dir_sep(dir);\n+\n+\tif (filename && *(++filename))\n+\t\tfinal = xstrdup(filename);\n+\n+\tfree(dir_copy);\n+\treturn final;\n+}\n+\n static int add_one_reference(struct string_list_item *item, void *cb_data)\n {\n \tchar *ref_git;\n@@ -451,6 +475,7 @@ static const char *junk_work_tree;\n static const char *junk_git_dir;\n static enum {\n \tJUNK_LEAVE_NONE,\n+\tJUNK_LEAVE_RESUMABLE,\n \tJUNK_LEAVE_REPO,\n \tJUNK_LEAVE_ALL\n } junk_mode = JUNK_LEAVE_NONE;\n@@ -460,6 +485,29 @@ N_(\"Clone succeeded, but checkout failed.\\n\"\n \"You can inspect what was checked out with 'git status'\\n\"\n \"and retry the checkout with 'git checkout -f HEAD'\\n\");\n \n+static const char junk_leave_resumable_msg[] =\n+N_(\"Clone interrupted while copying resumable resource.\\n\"\n+ \"Try using 'git clone --resume ',\\n\"\n+ \"where is either the new working \\n\"\n+ \"directory or git directory.\\n\\n\"\n+ \"If this does not succeed, it could be because the\\n\"\n+ \"resource has been moved, corrupted, or changed.\\n\"\n+ \"If this is the case, you should remove \\n\"\n+ \"and run the original command.\\n\");\n+\n+static void write_resumable_resource()\n+{\n+\tconst char *filename = git_path_resumable();\n+\tstruct strbuf content = STRBUF_INIT;\n+\tstrbuf_addf(&content, \"%s\\n%s\\n\", alt_res->url, alt_res->filetype);\n+\tint fd = open(filename, O_WRONLY | O_CREAT, 0666);\n+\tif (fd < 0)\n+\t\tdie_errno(_(\"Could not open '%s' for writing\"), filename);\n+\tif (write_in_full(fd, content.buf, content.len) != content.len)\n+\t\tdie_errno(_(\"Could not write to '%s'\"), filename);\n+\tclose(fd);\n+}\n+\n static void remove_junk(void)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -467,7 +515,11 @@ static void remove_junk(void)\n \tswitch (junk_mode) {\n \tcase JUNK_LEAVE_REPO:\n \t\twarning(\"%s\", _(junk_leave_repo_msg));\n-\t\t/* fall-through */\n+\t\treturn;\n+\tcase JUNK_LEAVE_RESUMABLE:\n+\t\twrite_resumable_resource();\n+\t\twarning(\"%s\", _(junk_leave_resumable_msg));\n+\t\treturn;\n \tcase JUNK_LEAVE_ALL:\n \t\treturn;\n \tdefault:\n@@ -562,7 +614,7 @@ static void write_remote_refs(const struct ref *local_refs)\n \t\tdie(\"%s\", err.buf);\n \n \tfor (r = local_refs; r; r = r->next) {\n-\t\tif (!r->peer_ref)\n+\t\tif (!r->peer_ref || ref_exists(r->peer_ref->name))\n \t\t\tcontinue;\n \t\tif (ref_transaction_create(t, r->peer_ref->name, r->old_oid.hash,\n \t\t\t\t\t 0, NULL, &err))\n@@ -820,11 +872,296 @@ static void dissociate_from_references(void)\n \tfree(alternates);\n }\n \n+static int do_index_pack(const char *in_pack_file, const char *out_idx_file)\n+{\n+\tconst char *argv[] = { \"index-pack\", \"--clone-bundle\", \"-v\",\n+\t\t\t \"--check-self-contained-and-connected\", \"-o\",\n+\t\t\t out_idx_file, in_pack_file, NULL };\n+\treturn run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDOUT);\n+}\n+\n+static const char *replace_extension(const char *filename, const char *existing,\n+\t\t\t\t const char *replacement)\n+{\n+\tstruct strbuf new_filename = STRBUF_INIT;\n+\tint existing_len = strlen(existing);\n+\tint replacement_len = strlen(replacement);\n+\tint filename_len = strlen(filename);\n+\n+\tif (!(filename && existing && replacement)) {\n+\t\treturn NULL;\n+\t}\n+\n+\tif (!strncmp(filename + filename_len - existing_len,\n+\t\t existing, existing_len)) {\n+\t\tint existing_position = filename_len - existing_len;\n+\t\tstrbuf_addstr(&new_filename, filename);\n+\t\tstrbuf_splice(&new_filename, existing_position, existing_len,\n+\t\t\t\treplacement, replacement_len);\n+\t}\n+\n+\treturn strbuf_detach(&new_filename, NULL);\n+}\n+\n+static const char *setup_and_index_pack(const char *filename)\n+{\n+\tconst char *primer_idx_path = NULL, *primer_bndl_path = NULL;\n+\tprimer_idx_path = replace_extension(filename, \".pack\", \".idx\");\n+\tprimer_bndl_path = replace_extension(filename, \".pack\", \".bndl\");\n+\n+\tif (!(primer_idx_path && primer_bndl_path)) {\n+\t\twarning(\"invalid pack filename '%s', falling back to full \"\n+\t\t\t\"clone\", filename);\n+\t\treturn NULL;\n+\t}\n+\n+\tif (!file_exists(primer_bndl_path)) {\n+\t\tif (do_index_pack(filename, primer_idx_path)) {\n+\t\t\twarning(\"could not index primer pack, falling back to \"\n+\t\t\t\t\"full clone\");\n+\t\t\treturn NULL;\n+\t\t}\n+\t}\n+\n+\treturn primer_bndl_path;\n+}\n+\n+static int write_bundle_refs(const char *bundle_filename)\n+{\n+\tstruct ref_transaction *t;\n+\tstruct bundle_header history_tips;\n+\tconst char *temp_ref_base = \"resume\";\n+\tstruct strbuf err = STRBUF_INIT;\n+\tint i;\n+\n+\tinit_bundle_header(&history_tips, bundle_filename);\n+\tread_bundle_header(&history_tips);\n+\n+\tt = ref_transaction_begin(&err);\n+\tfor (i = 0; i < history_tips.references.nr; i++) {\n+\t\tstruct strbuf ref_name = STRBUF_INIT;\n+\t\tstrbuf_addf(&ref_name, \"refs/temp/%s/%s/temp-%s\",\n+\t\t\t option_origin, temp_ref_base,\n+\t\t\t sha1_to_hex(history_tips.references.list[i].sha1));\n+\t\tif (!ref_exists(ref_name.buf)) {\n+\t\t\tif (ref_transaction_create(t, ref_name.buf,\n+\t\t\t\t\thistory_tips.references.list[i].sha1,\n+\t\t\t\t\t0, NULL, &err)) {\n+\t\t\t\twarning(_(\"%s\"), err.buf);\n+\t\t\t\treturn -1;\n+\t\t\t}\n+\t\t}\n+\t\tstrbuf_release(&ref_name);\n+\t}\n+\n+\tif (initial_ref_transaction_commit(t, &err)) {\n+\t\twarning(\"%s\", err.buf);\n+\t\treturn -1;\n+\t}\n+\tref_transaction_free(t);\n+\trelease_bundle_header(&history_tips);\n+\treturn 0;\n+}\n+\n+static int use_alt_resource_pack(const char *alt_res_path)\n+{\n+\tint ret = -1;\n+\tconst char *bundle_path = setup_and_index_pack(alt_res_path);\n+\tif (bundle_path)\n+\t\tret = write_bundle_refs(bundle_path);\n+\treturn ret;\n+}\n+\n+static int use_alt_resource(const char *alt_res_path)\n+{\n+\tint ret = -1;\n+\tif (!strcmp(alt_res->filetype, \"pack\"))\n+\t\tret = use_alt_resource_pack(alt_res_path);\n+\treturn ret;\n+}\n+\n+static void clean_alt_resource_pack(const char *resource_path,\n+\t\t\t\t int prime_successful)\n+{\n+\tstruct bundle_header history_tips;\n+\tconst char *temp_ref_base = \"resume\";\n+\tconst char *bundle_path;\n+\n+\tif (!resource_path)\n+\t\treturn;\n+\n+\tbundle_path = replace_extension(resource_path, \".pack\", \".bndl\");\n+\n+\tif (prime_successful) {\n+\t\tinit_bundle_header(&history_tips, bundle_path);\n+\t\tread_bundle_header(&history_tips);\n+\n+\t\tfor (int i = 0; i < history_tips.references.nr; i++) {\n+\t\t\tstruct strbuf ref_name = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&ref_name, \"refs/temp/%s/%s/temp-%s\",\n+\t\t\t\t option_origin, temp_ref_base,\n+\t\t\t\t sha1_to_hex(history_tips.references.list[i].sha1));\n+\t\t\tif (ref_exists(ref_name.buf)) {\n+\t\t\t\tdelete_ref(ref_name.buf,\n+\t\t\t\t\t history_tips.references.list[i].sha1,\n+\t\t\t\t\t 0);\n+\t\t\t}\n+\t\t\tstrbuf_release(&ref_name);\n+\t\t}\n+\t\trelease_bundle_header(&history_tips);\n+\t}\n+\n+\tif (!prime_successful) {\n+\t\tconst char *tmp_path = mkpath(\"%s.temp\", resource_path);\n+\t\tconst char *idx_path = replace_extension(resource_path, \".pack\",\n+\t\t\t\t\t\t\t \".idx\");\n+\t\tif (file_exists(resource_path)) {\n+\t\t\tunlink(resource_path);\n+\t\t}\n+\t\tif (file_exists(tmp_path)) {\n+\t\t\tunlink(tmp_path);\n+\t\t}\n+\t\tif (file_exists(idx_path)) {\n+\t\t\tunlink(idx_path);\n+\t\t}\n+\t}\n+\tif (file_exists(bundle_path)) {\n+\t\tunlink(bundle_path);\n+\t}\n+}\n+\n+static const char *fetch_alt_resource_pack(struct transport *transport,\n+\t\t\t\t\t const char *base_dir)\n+{\n+\tstruct strbuf download_path = STRBUF_INIT;\n+\tconst char *resource_path = NULL;\n+\tstruct remote *primer_remote = remote_get(alt_res->url);\n+\tstruct transport *primer_transport = transport_get(primer_remote,\n+\t\t\t\t\t\t\t alt_res->url);\n+\tstrbuf_addf(&download_path, \"%s/objects/pack\", base_dir);\n+\tfprintf(stderr, \"Downloading primer: %s...\\n\", alt_res->url);\n+\tresource_path = transport_download_primer(primer_transport, alt_res,\n+\t\t\t\t\t\t download_path.buf);\n+\ttransport_disconnect(primer_transport);\n+\treturn resource_path;\n+}\n+\n+static void clean_alt_resource(const char *resource_path, int prime_successful)\n+{\n+\tif (!strcmp(alt_res->filetype, \"pack\"))\n+\t\tclean_alt_resource_pack(resource_path, prime_successful);\n+}\n+\n+static const char *fetch_alt_resource(struct transport *transport,\n+\t\t\t\t const char *base_dir)\n+{\n+\tconst char *resource_path = NULL;\n+\tif (!strcmp(alt_res->filetype, \"pack\"))\n+\t\tresource_path = fetch_alt_resource_pack(transport, base_dir);\n+\treturn resource_path;\n+}\n+\n+static const struct alt_resource *get_last_alt_resource(void)\n+{\n+\tstruct alt_resource *ret = NULL;\n+\tFILE *fp;\n+\tif (fp = fopen(git_path_resumable(), \"r\")) {\n+\t\tret = xcalloc(1, sizeof(struct alt_resource));\n+\t\tstruct strbuf line = STRBUF_INIT;\n+\t\tstrbuf_getline(&line, fp);\n+\t\tret->url = strbuf_detach(&line, NULL);\n+\t\tstrbuf_getline(&line, fp);\n+\t\tret->filetype = strbuf_detach(&line, NULL);\n+\t\tfclose(fp);\n+\t}\n+\treturn ret;\n+}\n+\n+struct remote_config {\n+\tconst char *name;\n+\tconst char *fetch_pattern;\n+\tconst char *worktree;\n+\tint bare;\n+\tint mirror;\n+};\n+\n+static int get_remote_info(const char *key, const char *value, void *priv)\n+{\n+\tstruct remote_config *p = priv;\n+\tchar *sub_key;\n+\tchar *name;\n+\n+\tif (skip_prefix(key, \"remote.\", &key)) {\n+\t\tname = xstrdup(key);\n+\t\tsub_key = strchr(name, '.');\n+\t\t*sub_key++ = 0;\n+\t\tif (!p->name)\n+\t\t\tp->name = xstrdup(name);\n+\t\telse if (!strcmp(sub_key, \"fetch\"))\n+\t\t\tgit_config_string(&p->fetch_pattern, key, value);\n+\t\telse if (!strcmp(sub_key, \"mirror\"))\n+\t\t\tp->mirror = git_config_bool(key, value);\n+\t\tfree(name);\n+\t}\n+\telse if (!strcmp(key, \"core.bare\"))\n+\t\tp->bare = git_config_bool(key, value);\n+\telse if (!strcmp(key, \"core.worktree\"))\n+\t\tgit_config_string(&p->worktree, key, value);\n+\n+\treturn 0;\n+}\n+\n+static void get_existing_state(char *dir, const char **git_dir,\n+\t\t\t const char **work_tree,\n+\t\t\t struct remote_config *past_info)\n+{\n+\tif (is_git_directory(dir)) {\n+\t\t*git_dir = xstrdup(dir);\n+\t\t*work_tree = NULL;\n+\t}\n+\telse if (file_exists(mkpath(\"%s/.git\", dir))){\n+\t\t*work_tree = xstrdup(dir);\n+\t\t*git_dir = xstrdup(resolve_gitdir(mkpath(\"%s/.git\", dir)));\n+\t}\n+\n+\tif (!*git_dir)\n+\t\tdie(_(\"'%s' does not appear to be a git repo.\"), dir);\n+\n+\tset_git_dir(*git_dir);\n+\tgit_config(get_remote_info, past_info);\n+\n+\tif (!*work_tree) {\n+\t\tif (past_info->worktree) {\n+\t\t\t*work_tree = past_info->worktree;\n+\t\t}\n+\t\telse if (!past_info->bare) {\n+\t\t\tint containing_dir_success = 1;\n+\t\t\tchar *filename = get_filename(*git_dir);\n+\t\t\tif (filename && !strcmp(filename, \".git\")) {\n+\t\t\t\tconst char *parent_dir = mkpath(\"%s/..\",\n+\t\t\t\t\t\t\t\t*git_dir);\n+\t\t\t\t*work_tree = xstrdup(real_path(parent_dir));\n+\t\t\t\tif (access(*work_tree, W_OK) < 0)\n+\t\t\t\t\tcontaining_dir_success = 0;\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tcontaining_dir_success = 0;\n+\t\t\t}\n+\t\t\tif (!containing_dir_success)\n+\t\t\t\tdie(_(\"'%s' is configured for a work tree, \"\n+\t\t\t\t \"but no candidate exists.\"), dir);\n+\t\t}\n+\t}\n+\tif (*work_tree)\n+\t\tset_git_work_tree(*work_tree);\n+}\n+\n int cmd_clone(int argc, const char **argv, const char *prefix)\n {\n-\tint is_bundle = 0, is_local;\n+\tint is_bundle = 0, is_local, argc_original, option_count;\n \tstruct stat buf;\n-\tconst char *repo_name, *repo, *work_tree, *git_dir;\n+\tconst char *repo_name, *repo, *work_tree, *git_dir = NULL;\n+\tconst char *resource_path;\n \tchar *path, *dir;\n \tint dest_exists;\n \tconst struct ref *refs, *remote_head;\n@@ -838,13 +1175,23 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tconst char *src_ref_prefix = \"refs/heads/\";\n \tstruct remote *remote;\n \tint err = 0, complete_refs_before_fetch = 1;\n-\n \tstruct refspec *refspec;\n \tconst char *fetch_pattern;\n \n \tpacket_trace_identity(\"clone\");\n+\targc_original = argc;\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t builtin_clone_usage, 0);\n+\toption_count = argc_original - argc;\n+\n+\tif (option_resume && option_count > 2) {\n+\t\tdie(_(\"--resume is incompatible with all other options.\"));\n+\t}\n+\n+\tif (option_resume && argc != 1) {\n+\t\tdie(_(\"--resume must be specified with a single resumable \"\n+\t\t \"directory.\"));\n+\t}\n \n \tif (argc > 2)\n \t\tusage_msg_opt(_(\"Too many arguments.\"),\n@@ -872,105 +1219,140 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (!option_origin)\n \t\toption_origin = \"origin\";\n \n-\trepo_name = argv[0];\n+\tif (option_resume) {\n+\t\tstruct remote_config past_info = {0};\n+\t\tdir = xstrdup(real_path(argv[0]));\n+\t\tstrip_trailing_slashes(dir);\n+\t\tif (!file_exists(dir))\n+\t\t\tdie(_(\"directory '%s' does not exist.\"), dir);\n+\t\tget_existing_state(dir, &git_dir, &work_tree, &past_info);\n+\n+\t\tif (!work_tree)\n+\t\t\toption_no_checkout = 1;\n+\t\tif (past_info.fetch_pattern)\n+\t\t\tfetch_pattern = past_info.fetch_pattern;\n+\t\telse {\n+\t\t\tstruct strbuf fetch_temp = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&branch_top, src_ref_prefix);\n+\t\t\tstrbuf_addf(&fetch_temp, \"+%s*:%s*\", src_ref_prefix,\n+\t\t\t\t\tbranch_top.buf);\n+\t\t\tfetch_pattern = strbuf_detach(&fetch_temp, NULL);\n+\t\t}\n \n-\tpath = get_repo_path(repo_name, &is_bundle);\n-\tif (path)\n-\t\trepo = xstrdup(absolute_path(repo_name));\n-\telse if (!strchr(repo_name, ':'))\n-\t\tdie(_(\"repository '%s' does not exist\"), repo_name);\n-\telse\n-\t\trepo = repo_name;\n+\t\toption_origin = past_info.name;\n+\t\toption_mirror = past_info.mirror;\n+\t\toption_bare = past_info.bare;\n+\t\trefspec = parse_fetch_refspec(1, &fetch_pattern);\n \n-\t/* no need to be strict, transport_set_option() will validate it again */\n-\tif (option_depth && atoi(option_depth) < 1)\n-\t\tdie(_(\"depth %s is not a positive number\"), option_depth);\n+\t\tif (!(alt_res = get_last_alt_resource()))\n+\t\t\tdie(_(\"--resume option used, but current \"\n+\t\t\t \"directory is not resumable\"));\n \n-\tif (argc == 2)\n-\t\tdir = xstrdup(argv[1]);\n-\telse\n-\t\tdir = guess_dir_name(repo_name, is_bundle, option_bare);\n-\tstrip_trailing_slashes(dir);\n+\t\tjunk_mode = JUNK_LEAVE_RESUMABLE;\n+\t\tatexit(remove_junk);\n+\t\tsigchain_push_common(remove_junk_on_signal);\n+\t}\n+\telse {\n+\t\trepo_name = argv[0];\n+\n+\t\tpath = get_repo_path(repo_name, &is_bundle);\n+\t\tif (path)\n+\t\t\trepo = xstrdup(absolute_path(repo_name));\n+\t\telse if (!strchr(repo_name, ':'))\n+\t\t\tdie(_(\"repository '%s' does not exist\"), repo_name);\n+\t\telse\n+\t\t\trepo = repo_name;\n \n-\tdest_exists = !stat(dir, &buf);\n-\tif (dest_exists && !is_empty_dir(dir))\n-\t\tdie(_(\"destination path '%s' already exists and is not \"\n-\t\t\t\"an empty directory.\"), dir);\n+\t\t/* no need to be strict, transport_set_option() will validate it again */\n+\t\tif (option_depth && atoi(option_depth) < 1)\n+\t\t\tdie(_(\"depth %s is not a positive number\"), option_depth);\n \n-\tstrbuf_addf(&reflog_msg, \"clone: from %s\", repo);\n+\t\tif (argc == 2)\n+\t\t\tdir = xstrdup(argv[1]);\n+\t\telse\n+\t\t\tdir = guess_dir_name(repo_name, is_bundle, option_bare);\n+\t\tstrip_trailing_slashes(dir);\n \n-\tif (option_bare)\n-\t\twork_tree = NULL;\n-\telse {\n-\t\twork_tree = getenv(\"GIT_WORK_TREE\");\n-\t\tif (work_tree && !stat(work_tree, &buf))\n-\t\t\tdie(_(\"working tree '%s' already exists.\"), work_tree);\n-\t}\n+\t\tdest_exists = !stat(dir, &buf);\n+\t\tif (dest_exists && !is_empty_dir(dir))\n+\t\t\tdie(_(\"destination path '%s' already exists and is not \"\n+\t\t\t\t\"an empty directory.\"), dir);\n \n-\tif (option_bare || work_tree)\n-\t\tgit_dir = xstrdup(dir);\n-\telse {\n-\t\twork_tree = dir;\n-\t\tgit_dir = mkpathdup(\"%s/.git\", dir);\n-\t}\n+\t\tstrbuf_addf(&reflog_msg, \"clone: from %s\", repo);\n \n-\tatexit(remove_junk);\n-\tsigchain_push_common(remove_junk_on_signal);\n-\n-\tif (!option_bare) {\n-\t\tif (safe_create_leading_directories_const(work_tree) < 0)\n-\t\t\tdie_errno(_(\"could not create leading directories of '%s'\"),\n-\t\t\t\t work_tree);\n-\t\tif (!dest_exists && mkdir(work_tree, 0777))\n-\t\t\tdie_errno(_(\"could not create work tree dir '%s'\"),\n-\t\t\t\t work_tree);\n-\t\tjunk_work_tree = work_tree;\n-\t\tset_git_work_tree(work_tree);\n-\t}\n+\t\tif (option_bare)\n+\t\t\twork_tree = NULL;\n+\t\telse {\n+\t\t\twork_tree = getenv(\"GIT_WORK_TREE\");\n+\t\t\tif (work_tree && !stat(work_tree, &buf))\n+\t\t\t\tdie(_(\"working tree '%s' already exists.\"), work_tree);\n+\t\t}\n \n-\tjunk_git_dir = git_dir;\n-\tif (safe_create_leading_directories_const(git_dir) < 0)\n-\t\tdie(_(\"could not create leading directories of '%s'\"), git_dir);\n+\t\tif (option_bare || work_tree)\n+\t\t\tgit_dir = xstrdup(dir);\n+\t\telse {\n+\t\t\twork_tree = dir;\n+\t\t\tgit_dir = mkpathdup(\"%s/.git\", dir);\n+\t\t}\n \n-\tset_git_dir_init(git_dir, real_git_dir, 0);\n-\tif (real_git_dir) {\n-\t\tgit_dir = real_git_dir;\n-\t\tjunk_git_dir = real_git_dir;\n-\t}\n+\t\tatexit(remove_junk);\n+\t\tsigchain_push_common(remove_junk_on_signal);\n \n-\tif (0 <= option_verbosity) {\n-\t\tif (option_bare)\n-\t\t\tfprintf(stderr, _(\"Cloning into bare repository '%s'...\\n\"), dir);\n-\t\telse\n-\t\t\tfprintf(stderr, _(\"Cloning into '%s'...\\n\"), dir);\n-\t}\n-\tinit_db(option_template, INIT_DB_QUIET);\n-\twrite_config(&option_config);\n+\t\tif (!option_bare) {\n+\t\t\tif (safe_create_leading_directories_const(work_tree) < 0)\n+\t\t\t\tdie_errno(_(\"could not create leading directories of '%s'\"),\n+\t\t\t\t\t work_tree);\n+\t\t\tif (!dest_exists && mkdir(work_tree, 0777))\n+\t\t\t\tdie_errno(_(\"could not create work tree dir '%s'\"),\n+\t\t\t\t\t work_tree);\n+\t\t\tjunk_work_tree = work_tree;\n+\t\t\tset_git_work_tree(work_tree);\n+\t\t}\n \n-\tgit_config(git_default_config, NULL);\n+\t\tjunk_git_dir = git_dir;\n+\t\tif (safe_create_leading_directories_const(git_dir) < 0)\n+\t\t\tdie(_(\"could not create leading directories of '%s'\"), git_dir);\n \n-\tif (option_bare) {\n-\t\tif (option_mirror)\n-\t\t\tsrc_ref_prefix = \"refs/\";\n-\t\tstrbuf_addstr(&branch_top, src_ref_prefix);\n+\t\tset_git_dir_init(git_dir, real_git_dir, 0);\n+\t\tif (real_git_dir) {\n+\t\t\tgit_dir = real_git_dir;\n+\t\t\tjunk_git_dir = real_git_dir;\n+\t\t}\n \n-\t\tgit_config_set(\"core.bare\", \"true\");\n-\t} else {\n-\t\tstrbuf_addf(&branch_top, \"refs/remotes/%s/\", option_origin);\n-\t}\n+\t\tif (0 <= option_verbosity) {\n+\t\t\tif (option_bare)\n+\t\t\t\tfprintf(stderr, _(\"Cloning into bare repository '%s'...\\n\"), dir);\n+\t\t\telse\n+\t\t\t\tfprintf(stderr, _(\"Cloning into '%s'...\\n\"), dir);\n+\t\t}\n+\t\tinit_db(option_template, INIT_DB_QUIET);\n+\t\twrite_config(&option_config);\n+\n+\t\tgit_config(git_default_config, NULL);\n \n-\tstrbuf_addf(&value, \"+%s*:%s*\", src_ref_prefix, branch_top.buf);\n-\tstrbuf_addf(&key, \"remote.%s.url\", option_origin);\n-\tgit_config_set(key.buf, repo);\n-\tstrbuf_reset(&key);\n+\t\tif (option_bare) {\n+\t\t\tif (option_mirror)\n+\t\t\t\tsrc_ref_prefix = \"refs/\";\n+\t\t\tstrbuf_addstr(&branch_top, src_ref_prefix);\n \n-\tif (option_reference.nr)\n-\t\tsetup_reference();\n+\t\t\tgit_config_set(\"core.bare\", \"true\");\n+\t\t} else {\n+\t\t\tstrbuf_addf(&branch_top, \"refs/remotes/%s/\", option_origin);\n+\t\t}\n+\n+\t\tstrbuf_addf(&value, \"+%s*:%s*\", src_ref_prefix, branch_top.buf);\n+\t\tstrbuf_addf(&key, \"remote.%s.url\", option_origin);\n+\t\tgit_config_set(key.buf, repo);\n+\t\tstrbuf_reset(&key);\n \n-\tfetch_pattern = value.buf;\n-\trefspec = parse_fetch_refspec(1, &fetch_pattern);\n+\t\tif (option_reference.nr)\n+\t\t\tsetup_reference();\n \n-\tstrbuf_reset(&value);\n+\t\tfetch_pattern = value.buf;\n+\t\trefspec = parse_fetch_refspec(1, &fetch_pattern);\n+\n+\t\tstrbuf_reset(&value);\n+\t}\n \n \tremote = remote_get(option_origin);\n \ttransport = transport_get(remote, remote->url[0]);\n@@ -1003,6 +1385,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (option_single_branch)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n \n+\tif (option_prime_clone)\n+\t\ttransport_set_option(transport, TRANS_OPT_PRIMECLONE,\n+\t\t\t\t option_prime_clone);\n+\n \tif (option_upload_pack)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\n \t\t\t\t option_upload_pack);\n@@ -1010,6 +1396,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (transport->smart_options && !option_depth)\n \t\ttransport->smart_options->check_self_contained_and_connected = 1;\n \n+\tif (!is_local && option_reference.nr == 0 && !alt_res)\n+\t\talt_res = transport_prime_clone(transport);\n \trefs = transport_get_remote_refs(transport);\n \n \tif (refs) {\n@@ -1064,8 +1452,24 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t \"refs/heads/master\");\n \t}\n \n-\twrite_refspec_config(src_ref_prefix, our_head_points_at,\n-\t\t\tremote_head_points_at, &branch_top);\n+\tif (!option_resume) {\n+\t\twrite_refspec_config(src_ref_prefix, our_head_points_at,\n+\t\t\t\t remote_head_points_at, &branch_top);\n+\t}\n+\n+\tif (alt_res) {\n+\t\tjunk_mode = JUNK_LEAVE_RESUMABLE;\n+\t\tresource_path = fetch_alt_resource(transport, git_dir);\n+\t\tif (!resource_path || use_alt_resource(resource_path) < 0) {\n+\t\t\tif (option_resume)\n+\t\t\t\tdie(_(\"resumable resource is no longer \"\n+\t\t\t\t \"available or usable\"));\n+\t\t\tjunk_mode = JUNK_LEAVE_NONE;\n+\t\t\tclean_alt_resource(resource_path, 0);\n+\t\t\tresource_path = NULL;\n+\t\t\talt_res = NULL;\n+\t\t}\n+\t}\n \n \tif (is_local)\n \t\tclone_local(path, git_dir);\n@@ -1085,9 +1489,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tdissociate_from_references();\n \t}\n \n+\tif (resource_path) {\n+\t\tclean_alt_resource(resource_path, 1);\n+\t}\n+\n \tjunk_mode = JUNK_LEAVE_REPO;\n \terr = checkout();\n \n+\tif (file_exists(git_path_resumable())) {\n+\t\tunlink(git_path_resumable());\n+\t}\n+\n \tstrbuf_release(&reflog_msg);\n \tstrbuf_release(&branch_top);\n \tstrbuf_release(&key);\ndiff --git a/t/t9904-git-prime-clone.sh b/t/t9904-git-prime-clone.sh\nnew file mode 100755\nindex 0000000..257cea9\n--- /dev/null\n+++ b/t/t9904-git-prime-clone.sh\n@@ -0,0 +1,181 @@\n+#!/bin/sh\n+\n+test_description='tests for git prime-clone'\n+. ./test-lib.sh\n+\n+ROOT_PATH=\"$PWD\"\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'resume fails for no parameters' '\n+\ttest_must_fail git clone --resume\n+'\n+\n+test_expect_success 'resume fails with other options' '\n+\ttest_must_fail git clone --resume --bare\n+'\n+\n+test_expect_success 'resume fails for excess parameters' '\n+\ttest_must_fail git clone --resume a b\n+'\n+\n+test_expect_success 'resume fails for nonexistent directory' '\n+\ttest_must_fail git clone --resume nonexistent\n+'\n+\n+test_expect_success 'setup repo and httpd' '\n+\tmkdir server &&\n+\tcd server &&\n+\tgit init &&\n+\techo \"content\\\\n\" >example.c &&\n+\tgit add example.c &&\n+\tgit commit -m \"I am a packed commit\" &&\n+\tgit repack . &&\n+\tgit config --local http.primeclone true &&\n+\tgit config --local primeclone.url \\\n+\t\t$HTTPD_URL/server/.git/objects/pack/$(find .git/objects/pack/*.pack -printf \"%f\") &&\n+\tgit config --local primeclone.filetype pack &&\n+\techo \"content\\\\n\" >example2.c &&\n+\techo \"new content\\\\n\" >example.c &&\n+\tgit add example.c example2.c &&\n+\tgit commit -m \"I am an unpacked commit\" &&\n+\tcd - &&\n+\tmv server \"$HTTPD_DOCUMENT_ROOT_PATH\"\n+'\n+\n+test_expect_success 'prime-clone works http' '\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\trm -r client\n+'\n+\n+test_expect_success 'prime-clone falls back not permitted' '\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tgit config --local http.primeclone false &&\n+\tcd - &&\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\trm -r client &&\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tgit config --local http.primeclone true &&\n+\tcd -\n+'\n+\n+test_expect_success 'prime-clone falls back not enabled' '\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tgit config --local primeclone.enabled 0 &&\n+\tcd - &&\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\trm -r client &&\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tgit config --local --unset-all primeclone.enabled &&\n+\tcd -\n+'\n+\n+test_expect_success 'prime-clone falls back incorrect config' '\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tgit config --local --unset-all primeclone.filetype &&\n+\tcd - &&\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\trm -r client &&\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tgit config --local primeclone.filetype pack &&\n+\tcd -\n+'\n+\n+test_expect_success 'clone resume fails in complete/unmarked directory' '\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\ttest_must_fail git clone --resume client &&\n+\trm -r client\n+'\n+\n+test_expect_success 'clone resume works with marked repo (work dir, normal)' '\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\tcd client &&\n+\trm .git/objects/pack/*.idx &&\n+\techo -n \"$HTTPD_URL/server/\" > .git/RESUMABLE &&\n+\tfind .git/objects/pack/*.pack >> .git/RESUMABLE &&\n+\techo \"pack\" >> .git/RESUMABLE &&\n+\tmv $(find .git/objects/pack/*.pack) $(find .git/objects/pack/*.pack).tmp &&\n+\tsed -i \"2,$ d\" $(find .git/objects/pack/*.pack.tmp) &&\n+\trm * &&\n+\tgit clone --resume . &&\n+\tcd - &&\n+\trm -r client\n+'\n+\n+test_expect_success 'clone resume works with marked repo (git dir, normal)' '\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\tcd client &&\n+\trm .git/objects/pack/*.idx &&\n+\techo -n \"$HTTPD_URL/server/\" > .git/RESUMABLE &&\n+\tfind .git/objects/pack/*.pack >> .git/RESUMABLE &&\n+\techo \"pack\" >> .git/RESUMABLE &&\n+\tmv $(find .git/objects/pack/*.pack) $(find .git/objects/pack/*.pack).tmp &&\n+\tsed -i \"2,$ d\" $(find .git/objects/pack/*.pack.tmp) &&\n+\trm * &&\n+\tgit clone --resume .git &&\n+\tcd - &&\n+\trm -r client\n+'\n+\n+test_expect_success 'clone resume works inside marked repo (git dir, normal)' '\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\tcd client &&\n+\trm .git/objects/pack/*.idx &&\n+\techo -n \"$HTTPD_URL/server/\" > .git/RESUMABLE &&\n+\tfind .git/objects/pack/*.pack >> .git/RESUMABLE &&\n+\techo \"pack\" >> .git/RESUMABLE &&\n+\tmv $(find .git/objects/pack/*.pack) $(find .git/objects/pack/*.pack).tmp &&\n+\tsed -i \"2,$ d\" $(find .git/objects/pack/*.pack.tmp) &&\n+\trm * &&\n+\tcd .git &&\n+\tgit clone --resume . &&\n+\tcd ../.. &&\n+\trm -r client\n+'\n+\n+test_expect_success 'clone resume works with marked repo (work dir, split)' '\n+\tgit clone --separate-git-dir=separate_dir.git \\\n+\t\t$HTTPD_URL/smart/server client &&\n+\tcd separate_dir.git &&\n+\trm objects/pack/*.idx &&\n+\techo -n \"$HTTPD_URL/server/\" > RESUMABLE &&\n+\techo \".git/$(find objects/pack/*.pack)\" >> RESUMABLE &&\n+\techo \"pack\" >> RESUMABLE &&\n+\tmv $(find objects/pack/*.pack) $(find objects/pack/*.pack).tmp &&\n+\tsed -i \"2,$ d\" $(find objects/pack/*.pack.tmp) &&\n+\tcd ../client &&\n+\trm * &&\n+\tcd .. &&\n+\tgit clone --resume client &&\n+\trm -r client separate_dir.git\n+'\n+\n+test_expect_success 'clone resume works with marked repo (git dir, split)' '\n+\tgit clone --separate-git-dir=separate_dir.git \\\n+\t\t$HTTPD_URL/smart/server client &&\n+\tcd separate_dir.git &&\n+\trm objects/pack/*.idx &&\n+\techo -n \"$HTTPD_URL/server/\" > RESUMABLE &&\n+\techo \".git/$(find objects/pack/*.pack)\" >> RESUMABLE &&\n+\techo \"pack\" >> RESUMABLE &&\n+\tmv $(find objects/pack/*.pack) $(find objects/pack/*.pack).tmp &&\n+\tsed -i \"2,$ d\" $(find objects/pack/*.pack.tmp) &&\n+\tcd ../client &&\n+\trm * &&\n+\tcd .. &&\n+\tgit clone --resume separate_dir.git &&\n+\trm -r client separate_dir.git\n+'\n+\n+test_expect_success 'prime-clone falls back unusable file' '\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tgit config --local primeclone.url $HTTPD_URL/server/.git/HEAD &&\n+\tcd - &&\n+\tgit clone $HTTPD_URL/smart/server client &&\n+\trm -r client &&\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\tcd -\n+'\n+\n+stop_httpd\n+test_done\n-- \n2.7.4\n\n"},{"id":"302072","messageId":"xmqq4m5fk1wy.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 00/11] Resumable clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-16T20:47:09Z","receivedAt":"2016-09-16T20:47:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Wern writes:\n\n> It's been a while (sent a very short patch in May), but I've\n> still been working on the resumable clone feature and checking up on\n> the mailing list for any updates. After submitting the prime-clone\n> service alone, I figured implementing the whole thing would be the best\n> way to understand the full scope of the problem (this is my first real\n> contribution here, and learning while working on such an involved\n> feature has not been easy). \n\nIt may not have been easy but I hope it has been a fun journey for\nyou ;-)\n\n> On the client side, the transport_prime_clone and\n> transport_download_primer APIs are built to be more robust (i.e. read\n> messages without dying due to protocol errors), so that git clone can\n> always try them without being dependent on the capability output of\n> git-upload-pack. transport_download_primer is dependent on the success\n> of transport_prime_clone, but transport_prime_clone is always run on an\n> initial clone. Part of achieving this robustness involves adding\n> *_gentle functions to pkt_line, so that prime_clone can fail silently\n> without dying.\n\nOK.\n\n> Right now, a manually resumable directory is left behind only if the\n> *client* is interrupted while a new junk mode, JUNK_LEAVE_RESUMABLE,\n> is set (right before the download). For an initial clone, if the\n> connection fails after automatic resuming, the client erases the\n> partial resources and falls through to a normal clone. However, once a\n> resumable directory is left behind by the program, it is NEVER\n> deleted/abandoned after it is continued with --resume.\n\nSounds like you made a sensible design decision here.\n\n> \t- When running with ssh and a password, the credentials are\n> \t prompted for twice. I don't know if there is a way to\n> \t preserve credentials between executions. I couldn't find any\n> \t examples in git's source.\n\nWe leave credentail reuse to keyring services like ssh-agent.\n\n"},{"id":"302074","messageId":"xmqqzin7in2c.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"1473984742-12516-2-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 01/11] Resumable clone: create service git-prime-clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-16T20:53:15Z","receivedAt":"2016-09-16T20:53:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Wern writes:\n\n> Create git-prime-clone, a program to be executed on the server that\n> returns the location and type of static resource to download before\n> performing the rest of a clone.\n>\n> Additionally, as this executable's location will be configurable (see:\n> upload-pack and receive-pack), add the program to\n> BINDIR_PROGRAMS_NEED_X, in addition to the usual builtin places. Add\n> git-prime-clone executable to gitignore, as well\n>\n> Signed-off-by: Kevin Wern \n> ---\n\nI wonder if we even need a separate service like this.\n\nWouldn't a new protocol capability that is advertised from\nupload-pack sufficient to tell the \"git clone\" that it can\nand should consider priming from this static resource?\n\n> +static void prime_clone(void)\n> +{\n> +\tif (!enabled) {\n> +\t\tfprintf(stderr, _(\"prime-clone not enabled\\n\"));\n> +\t}\n> +\telse if (url && filetype){\n> +\t\tpacket_write(1, \"%s %s\\n\", filetype, url);\n> +\t}\n> +\telse if (url || filetype) {\n> +\t\tif (filetype)\n> +\t\t\tfprintf(stderr, _(\"prime-clone not properly \"\n> +\t\t\t\t\t \"configured: missing url\\n\"));\n> +\t\telse if (url)\n> +\t\t\tfprintf(stderr, _(\"prime-clone not properly \"\n> +\t\t\t\t\t \"configured: missing filetype\\n\"));\n> +\t}\n> +\tpacket_flush(1);\n> +}\n\nTwo minor comments:\n\n - For whom are you going to localize these strings? This program\n is running on the server side and we do not know the locale\n preferred by the end-user who is sitting on the other end of the\n connection, no?\n\n - Turn \"}\\n\\s+else \" into \"} else \", please.\n\n"},{"id":"302079","messageId":"xmqqoa3nij5z.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"1473984742-12516-4-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 03/11] pkt-line: create gentle packet_read_line functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-16T22:17:28Z","receivedAt":"2016-09-16T22:17:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Wern writes:\n\n> \t/* And complain if we didn't get enough bytes to satisfy the read. */\n> \tif (ret < size) {\n> -\t\tif (options & PACKET_READ_GENTLE_ON_EOF)\n> +\t\tif (options & (PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ALL))\n> \t\t\treturn -1;\n\nThe name _ALL suggested to me that there may be multiple \"under this\ncondition, be gentle\", \"under that condition, be gentle\", and _ALL\nis used as a catch-all \"under any condition, be gentle\". If you\ndefined _ALL symbol to have all GENTLE bits on, this line could have\nbecome\n\n\tif (options & PACKET_READ_GENTLE_ALL)\n\n> @@ -205,15 +209,23 @@ int packet_read(int fd, char **src_buf, size_t *src_len,\n> \tif (ret < 0)\n> \t\treturn ret;\n> \tlen = packet_length(linelen);\n> -\tif (len < 0)\n> +\tif (len < 0) {\n> +\t\tif (options & PACKET_READ_GENTLE_ALL)\n> +\t\t\treturn -1;\n\nOn the other hand, however, you do want to die here when only\nGENTLE_ON_EOF is set.\n\nTaking the above two observations together, I'd have to say that\n_ALL is probably a misnomer. I agree with a need for a flag with\nthe behaviour you defined in this patch, though.\n\n> \t\tdie(\"protocol error: bad line length character: %.4s\", linelen);\n\n> static char *packet_read_line_generic(int fd,\n> \t\t\t\t char **src, size_t *src_len,\n> -\t\t\t\t int *dst_len)\n> +\t\t\t\t int *dst_len, int flags)\n\nThe original one is called options, not flags, and it would be\neasier to follow if it is consistently called options, instead of\nrequiring the reader to keep track of \"ah, it is called flags here\nbut the callee renames it to options\".\n\n> +/*\n> + * Same as packet_read_line, but does not die on any errors (uses\n> + * PACKET_READ_GENTLE_ALL).\n> + */\n> +char *packet_read_line_gentle(int fd, int *len_p);\n> +\n> +/*\n> + * Same as packet_read_line_buf, but does not die on any errors (uses\n> + * PACKET_READ_GENTLE_ALL).\n> + */\n> +char *packet_read_line_buf_gentle(char **src_buf, size_t *src_len, int *size);\n\nI think most if not all \"do the same thing as do_something() but\nreport errors instead of dying\" variant of functions are named\ndo_something_gently(), not do_something_gentle().\n\n"},{"id":"302085","messageId":"xmqqfuozihuv.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"1473984742-12516-8-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 07/11] Resumable clone: add resumable download to http/curl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-16T22:45:44Z","receivedAt":"2016-09-16T22:45:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Wern writes:\n\n> +int http_download_primer(const char *url, const char *out_file)\n> +{\n> +\tint ret = 0, try_count = HTTP_TRY_COUNT;\n> +\tstruct http_get_options options = {0};\n> +\toptions.progress = 1;\n> +\n> +\tif (file_exists(out_file)) {\n> +\t\tfprintf(stderr,\n> +\t\t\t\"File already downloaded: '%s', skipping...\\n\",\n> +\t\t\tout_file);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tdo {\n> +\t\tif (try_count != HTTP_TRY_COUNT) {\n> +\t\t\tfprintf(stderr, \"Connection interrupted for some \"\n> +\t\t\t\t\"reason, retrying (%d attempts left)\\n\",\n> +\t\t\t\ttry_count);\n> +\t\t\tstruct timeval time = {10, 0}; // 1s\n\nWe do not use // comment.\n\n> +\t\t\tselect(0, NULL, NULL, NULL, &time);\n> +\t\t}\n> +\t\tret = http_get_file(url, out_file, &options);\n\nI didn't realize that http_get_file() -> http_request() codepath,\nwhen it is the output file, already can do the \"ftell and request\nthe reminder\". Very nice.\n\n> @@ -1136,7 +1138,10 @@ static int handle_curl_result(struct slot_results *results)\n> \t\t\t\tcurl_easy_strerror(results->curl_result),\n> \t\t\t\tsizeof(curl_errorstr));\n> #endif\n> -\t\treturn HTTP_ERROR;\n> +\t\tif (results->http_code >= 400)\n> +\t\t\treturn HTTP_ERROR;\n> +\t\telse\n> +\t\t\treturn HTTP_ERROR_RESUMABLE;\n> \t}\n> }\n\nHmm, is \"anything below 400\" a good definition of resumable errors?\n"},{"id":"302089","messageId":"xmqq37kzigvf.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"1473984742-12516-11-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-16T23:07:00Z","receivedAt":"2016-09-16T23:07:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Wern writes:\n\n> Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child\n> process.\n>\n> This will be used by git clone when calling index-pack on a downloaded\n> packfile.\n\nIf it is just one caller, would't it make more sense for that caller\nset no_stdout explicitly itself?\n"},{"id":"302090","messageId":"xmqqy42rh156.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"1473984742-12516-12-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 11/11] Resumable clone: implement primer logic in git-clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-16T23:32:05Z","receivedAt":"2016-09-16T23:32:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Wern writes:\n\n> Use transport_download_primer and transport_prime_clone in git clone.\n> This only supports a fully connected packfile.\n>\n> transport_prime_clone and transport_download_primer are executed\n> completely independent of transport_(get|fetch)_remote_refs, et al.\n> transport_download_primer is executed based on the existence of an\n> alt_resource. The idea is that the \"prime clone\" execution should be\n> able to attempt retrieving an alternate resource without dying, as\n> opposed to depending on the result of upload pack's \"capabilities\" to\n> indicate whether or not the client can attempt it.\n>\n> If a resumable resource is available, execute a codepath with the\n> following modular components:\n> - downloading resource to a specific directory\n> - using the resource (for pack, indexing and generating the bundle\n> file)\n> - cleaning up the resource (if the download or use fails)\n> - cleaning up the resource (if the download or use succeeds)\n>\n> If resume is interrupted on the client side, the alternate resource\n> info is written to the RESUMABLE file in the git directory.\n>\n> On resume, the required info is extracted by parsing the created\n> config file, and that info is used to determine the work and git\n> directories. If these cannot be determined, the program exits.\n> The writing of the refspec and determination of the initial git\n> directories are skipped, along with transport_prime_clone.\n>\n> The main purpose of this series of patches is to flesh out a codepath\n> for automatic resuming, manual resuming, and leaving a resumable\n> directory on exit--the logic for when to do these still needs more\n> work.\n>\n> Signed-off-by: Kevin Wern \n> ---\n> Documentation/git-clone.txt | 16 ++\n> builtin/clone.c | 590 +++++++++++++++++++++++++++++++++++++-------\n> t/t9904-git-prime-clone.sh | 181 ++++++++++++++\n> 3 files changed, 698 insertions(+), 89 deletions(-)\n> create mode 100755 t/t9904-git-prime-clone.sh\n>\n> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n> index b7c467a..5934bb6 100644\n> --- a/Documentation/git-clone.txt\n> +++ b/Documentation/git-clone.txt\n> @@ -16,6 +16,7 @@ SYNOPSIS\n> \t [--depth ] [--[no-]single-branch]\n> \t [--recursive | --recurse-submodules] [--] \n> \t []\n> +'git clone --resume '\n> \n> DESCRIPTION\n> -----------\n> @@ -172,6 +173,12 @@ objects from the source repository into a pack in the cloned repository.\n> \tvia ssh, this specifies a non-default path for the command\n> \trun on the other end.\n> \n> +--prime-clone ::\n> +-p ::\n\nNot many other options have single letter shorthand. Is it expected\nthat it is worth to let this option squat on a short-and-sweet \"-p\",\nperhaps because it is so frequently used?\n\n> +--resume::\n> +\tResume a partially cloned repo in a \"resumable\" state. This\n> +\tcan only be specified with a single local directory ( +\tdir>). This is incompatible with all other options.\n> +\n> +::\n> +\tThe directory of the partial clone. This could be either the\n> +\twork directory or the git directory.\n\nI think these should be described this way:\n\n --resume ::\n description if what resume option does and how resumable_dir\n is used by the option.\n\nin a single bullet point.\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 9ac6c01..d9a13dc 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -8,7 +8,9 @@\n> * Clone a repository into a different directory that does not yet exist.\n> */\n> \n> +#include \"cache.h\"\n> #include \"builtin.h\"\n\nI do not think you need to include cache.h if you are including\nbuiltin.h; Documentation/CodingGuidelines says:\n\n - The first #include in C files, except in platform specific compat/\n implementations, must be either \"git-compat-util.h\", \"cache.h\" or\n \"builtin.h\". You do not have to include more than one of these.\n\n> @@ -40,17 +42,20 @@ static const char * const builtin_clone_usage[] = {\n> \n> static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n> static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n> +static int option_resume;\n> static char *option_template, *option_depth;\n> -static char *option_origin = NULL;\n> +static const char *option_origin = NULL;\n\nIs this change related to anything you are doing here?\n\nIf you are fixing things while at it, please don't ;-) If you really\nwant to, please also remove \" = NULL\", from this line and also from\nthe next line. Also do not add \" = NULL\" at the end of alt_res.\n\n> static char *option_branch = NULL;\n> ...\n> +static const struct alt_resource *alt_res = NULL;\n\n> +static char *get_filename(const char *dir)\n> +{\n> +\tchar *dir_copy = xstrdup(dir);\n> +\tstrip_trailing_slashes(dir_copy);\n> +\tchar *filename, *final = NULL;\n> +\n> +\tfilename = find_last_dir_sep(dir);\n> +\n> +\tif (filename && *(++filename))\n> +\t\tfinal = xstrdup(filename);\n> +\n> +\tfree(dir_copy);\n> +\treturn final;\n> +}\n\nHmph, don't we have our own basename(3) lookalike that knows about\ndir-sep already?\n\n> @@ -451,6 +475,7 @@ static const char *junk_work_tree;\n> static const char *junk_git_dir;\n> static enum {\n> \tJUNK_LEAVE_NONE,\n> +\tJUNK_LEAVE_RESUMABLE,\n> \tJUNK_LEAVE_REPO,\n> \tJUNK_LEAVE_ALL\n> } junk_mode = JUNK_LEAVE_NONE;\n> @@ -460,6 +485,29 @@ N_(\"Clone succeeded, but checkout failed.\\n\"\n> \"You can inspect what was checked out with 'git status'\\n\"\n> \"and retry the checkout with 'git checkout -f HEAD'\\n\");\n> \n> +static const char junk_leave_resumable_msg[] =\n> +N_(\"Clone interrupted while copying resumable resource.\\n\"\n> + \"Try using 'git clone --resume ',\\n\"\n> + \"where is either the new working \\n\"\n> + \"directory or git directory.\\n\\n\"\n> + \"If this does not succeed, it could be because the\\n\"\n> + \"resource has been moved, corrupted, or changed.\\n\"\n> + \"If this is the case, you should remove \\n\"\n> + \"and run the original command.\\n\");\n> +\n> +static void write_resumable_resource()\n> +{\n> +\tconst char *filename = git_path_resumable();\n> +\tstruct strbuf content = STRBUF_INIT;\n> +\tstrbuf_addf(&content, \"%s\\n%s\\n\", alt_res->url, alt_res->filetype);\n> +\tint fd = open(filename, O_WRONLY | O_CREAT, 0666);\n> +\tif (fd < 0)\n> +\t\tdie_errno(_(\"Could not open '%s' for writing\"), filename);\n> +\tif (write_in_full(fd, content.buf, content.len) != content.len)\n> +\t\tdie_errno(_(\"Could not write to '%s'\"), filename);\n> +\tclose(fd);\n> +}\n>\n> static void remove_junk(void)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> @@ -467,7 +515,11 @@ static void remove_junk(void)\n> \tswitch (junk_mode) {\n> \tcase JUNK_LEAVE_REPO:\n> \t\twarning(\"%s\", _(junk_leave_repo_msg));\n> -\t\t/* fall-through */\n> +\t\treturn;\n> +\tcase JUNK_LEAVE_RESUMABLE:\n> +\t\twrite_resumable_resource();\n> +\t\twarning(\"%s\", _(junk_leave_resumable_msg));\n> +\t\treturn;\n\nNice.\n\n> @@ -562,7 +614,7 @@ static void write_remote_refs(const struct ref *local_refs)\n> \t\tdie(\"%s\", err.buf);\n> \n> \tfor (r = local_refs; r; r = r->next) {\n> -\t\tif (!r->peer_ref)\n> +\t\tif (!r->peer_ref || ref_exists(r->peer_ref->name))\n> \t\t\tcontinue;\n> \t\tif (ref_transaction_create(t, r->peer_ref->name, r->old_oid.hash,\n> \t\t\t\t\t 0, NULL, &err))\n\nWhat is this change about?\n\n\n> @@ -820,11 +872,296 @@ static void dissociate_from_references(void)\n> \tfree(alternates);\n> }\n> \n> +static int do_index_pack(const char *in_pack_file, const char *out_idx_file)\n> +{\n> +\tconst char *argv[] = { \"index-pack\", \"--clone-bundle\", \"-v\",\n> +\t\t\t \"--check-self-contained-and-connected\", \"-o\",\n> +\t\t\t out_idx_file, in_pack_file, NULL };\n> +\treturn run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDOUT);\n> +}\n\nThat looks vaguely familiar ;-)\n\n> +static const char *setup_and_index_pack(const char *filename)\n> +{\n> +\tconst char *primer_idx_path = NULL, *primer_bndl_path = NULL;\n> +\tprimer_idx_path = replace_extension(filename, \".pack\", \".idx\");\n> +\tprimer_bndl_path = replace_extension(filename, \".pack\", \".bndl\");\n> +\n> +\tif (!(primer_idx_path && primer_bndl_path)) {\n> +\t\twarning(\"invalid pack filename '%s', falling back to full \"\n> +\t\t\t\"clone\", filename);\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\tif (!file_exists(primer_bndl_path)) {\n> +\t\tif (do_index_pack(filename, primer_idx_path)) {\n> +\t\t\twarning(\"could not index primer pack, falling back to \"\n> +\t\t\t\t\"full clone\");\n> +\t\t\treturn NULL;\n> +\t\t}\n> +\t}\n\nCan it be another (undetected) failure mode that .bndl somehow\nalready existed, but not .idx, leaving the resulting object store in\nan incosistent state? Can do_index_pack() fail and leave .bndl\nbehind to get you into such a state?\n\n> +static int write_bundle_refs(const char *bundle_filename)\n> +{\n> +\tstruct ref_transaction *t;\n> +\tstruct bundle_header history_tips;\n> +\tconst char *temp_ref_base = \"resume\";\n> +\tstruct strbuf err = STRBUF_INIT;\n> +\tint i;\n> +\n> +\tinit_bundle_header(&history_tips, bundle_filename);\n> +\tread_bundle_header(&history_tips);\n> +\n> +\tt = ref_transaction_begin(&err);\n> +\tfor (i = 0; i < history_tips.references.nr; i++) {\n> +\t\tstruct strbuf ref_name = STRBUF_INIT;\n> +\t\tstrbuf_addf(&ref_name, \"refs/temp/%s/%s/temp-%s\",\n> +\t\t\t option_origin, temp_ref_base,\n> +\t\t\t sha1_to_hex(history_tips.references.list[i].sha1));\n\nCan we do this without polluting refs/temp/ namespace?\n\nI am imagining that you are first fetching the .pack file from\nsideways when primer service is available, running index-pack on it\nto produce the bundle, and the step after that is to run \"git fetch\"\nagainst the original remote to fill the gap between the bit-stale\nhistory you got in the bundle and the reality that has progressed\nsince the primer pack was made, and you need a way to tell to the\nother end that you already have the history leading to these refs\nwhen you run \"git fetch\". I think a bit better way to do so is to\nsend these has \".have\" while you run the \"fetch\".\n\nWouldn't it do if you add the \"--advertise-bundle-tips=\"\noption to \"git fetch\", move the code to read the bundle header to\nit, and point the bundle's filename with the option when you spawn\n\"git fetch\"?\n"},{"id":"302118","messageId":"alpine.DEB.2.20.1609182118380.129229@virtualbox","threadId":"44095","inReplyTo":"xmqq37kzigvf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-18T19:22:47Z","receivedAt":"2016-09-18T19:23:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 16 Sep 2016, Junio C Hamano wrote:\n\n> Kevin Wern writes:\n> \n> > Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child\n> > process.\n> >\n> > This will be used by git clone when calling index-pack on a downloaded\n> > packfile.\n> \n> If it is just one caller, would't it make more sense for that caller\n> set no_stdout explicitly itself?\n\nTaking a step back, maybe it is not such a good idea to swallow the output\nin all cases, including the error cases?\n\nMaybe the best course of action is to hide stdout/stderr by default but\nshow it in case of a non-zero exit code, i.e. using\nhttps://public-inbox.org/git/6383b7afcdeb6c999862aa32ba437997f2dd3d4e.1472633606.git.johannes.schindelin@gmx.de/ ?\n\nCiao,\nDscho\n"},{"id":"302133","messageId":"CACsJy8ARu_PkduP-McnE26OFEgr_YAkLtmfaxwYX7i9m47mJmg@mail.gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-3-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 02/11] Resumable clone: add prime-clone endpoints","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-19T13:15:00Z","receivedAt":"2016-09-19T13:15:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 16, 2016 at 7:12 AM, Kevin Wern wrote:\n> static struct daemon_service daemon_service[] = {\n> { \"upload-archive\", \"uploadarch\", upload_archive, 0, 1 },\n> { \"upload-pack\", \"uploadpack\", upload_pack, 1, 1 },\n> { \"receive-pack\", \"receivepack\", receive_pack, 0, 1 },\n> + { \"prime-clone\", \"primeclone\", prime_clone, 0, 1 },\n> };\n\nI guess this is why you chose to implement a new command in 01/11,\nsimpler to be called from http-backend?\n\n> + // prime-clone does not need --stateless-rpc and\n> + // --advertise-refs options. Maybe it will in the future, but\n> + // until then it seems best to do this instead of adding\n> + // \"dummy\" options.\n\nStick to /* .. */\n\n> + if (strcmp(svc->name, \"prime-clone\") != 0) {\n> + argv_array_pushl(&argv, \"--stateless-rpc\",\n> + \"--advertise-refs\", NULL);\n> + }\n\nWe also have an exception for select_getanyfile() below. I think it's\ntime we add a function callback in struct rpc_service to run each\nservice the way they want. Then prime-clone won't need an exception\n(neither does select_anyfile, mostly)\n-- \nDuy\n"},{"id":"302136","messageId":"CACsJy8CnQPPhwbhwG2jSzrCUJKin9+zoB5qjHb1AP0G+pEOnqQ@mail.gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-10-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 09/11] path: add resumable marker","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-19T13:24:34Z","receivedAt":"2016-09-19T13:25:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 16, 2016 at 7:12 AM, Kevin Wern wrote:\n> Create function to get gitdir file RESUMABLE.\n\nA very good opportunity to explain what this file is or will be (and\nwhether its content matters or just its existence). Either as commit\nmessage, or even better in Documentation/gitrepository-layout.txt.\n-- \nDuy\n"},{"id":"302138","messageId":"CACsJy8C+0M8o15E+iVX+f=izO2CtK7U642NC=XXaG-g41wYAUg@mail.gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-5-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 04/11] Resumable clone: add prime-clone to remote-curl","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-19T13:52:34Z","receivedAt":"2016-09-19T13:53:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 16, 2016 at 7:12 AM, Kevin Wern wrote:\n> +static void prime_clone(void)\n> +{\n> + char *result, *result_full, *line;\n> + size_t result_len;\n> + int err = 0, one_successful = 0;\n> +\n> + if (request_service(\"git-prime-clone\", &result_full, &result,\n> + &result_len, HTTP_ERROR_GENTLE)) {\n> + while (line = packet_read_line_buf_gentle(&result, &result_len,\n> + NULL)) {\n> + char *space = strchr(line ,' ');\n> +\n> + // We will eventually support multiple resources, so\n> + // always parse the whole message\n> + if (err)\n> + continue;\n> + if (!space || strchr(space + 1, ' ')) {\n> + if (options.verbosity > 1)\n> + fprintf(stderr, \"prime clone \"\n> + \"protocol error: got '%s'\\n\",\n> + line);\n> + printf(\"error\\n\");\n> + err = 1;\n> + continue;\n> + }\n> +\n> + one_successful = 1;\n> + printf(\"%s\\n\", line);\n\nA brief overview for this service in\nDocumentation/technical/http-protocol.txt (and maybe\nDocumentation/gitremote-helpers.txt as well) would be great help. It's\na bit hard to follow because at this point I don't know anything about\nthe server side (and on top of that I was confused between http\nsend/receive vs transport send/receive, but this is my fault).\n\n> + }\n> + if (!one_successful && options.verbosity > 1)\n> + fprintf(stderr, \"did not get required components for \"\n> + \"alternate resource\\n\");\n> + }\n> +\n> + printf(\"\\n\");\n> + fflush(stdout);\n> + free(result_full);\n> +}\n-- \nDuy\n"},{"id":"302139","messageId":"CACsJy8B1bbKBhg1ke4u6PV3k4FWz-bhBPyN2X=mV2Z2=8Mhy=A@mail.gmail.com","threadId":"44095","inReplyTo":"1473984742-12516-12-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 11/11] Resumable clone: implement primer logic in git-clone","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-19T14:04:40Z","receivedAt":"2016-09-19T14:05:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 16, 2016 at 7:12 AM, Kevin Wern wrote:\n> builtin/clone.c | 590 +++++++++++++++++++++++++++++++++++++-------\n\nArgh.. this is too big for my brain at this hour. It might be easier\nto follow if you separate out some code move (I think I've seen some,\nnot sure). I'll try to have another look when I find time. But it's\ngreat to hear from you again, the pleasant surprise in my inbox today,\nas I thought we lost you ;-) There's hope for resumable clone maybe\nbefore 2018 again.\n-- \nDuy\n"},{"id":"302153","messageId":"xmqqmvj3g68q.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"CACsJy8B1bbKBhg1ke4u6PV3k4FWz-bhBPyN2X=mV2Z2=8Mhy=A@mail.gmail.com","subject":"Re: [PATCH 11/11] Resumable clone: implement primer logic in git-clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-19T17:16:21Z","receivedAt":"2016-09-19T17:16:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Fri, Sep 16, 2016 at 7:12 AM, Kevin Wern wrote:\n>> builtin/clone.c | 590 +++++++++++++++++++++++++++++++++++++-------\n>\n> Argh.. this is too big for my brain at this hour. It might be easier\n> to follow if you separate out some code move (I think I've seen some,\n> not sure). I'll try to have another look when I find time. But it's\n> great to hear from you again, the pleasant surprise in my inbox today,\n> as I thought we lost you ;-) There's hope for resumable clone maybe\n> before 2018 again.\n\nI had a similar thought.\n\nWhat \"git clone\" (WITHOUT Kevin's update) should have been was to be\nas close to\n\n * Parse command line arguments;\n\n * Create a new repository and go into it; this step would\n require us to have parsed the command line for --template,\n , --separate-git-dir, etc.\n\n * Talk to the remote and do get_remote_heads() aka ls-remote\n output;\n\n * Decide what fetch refspec to use, which alternate object store\n to borrow from; this step would require us to have parsed the\n command line for --reference, --mirror, --origin, etc;\n\n * Issue \"git fetch\" with the refspec determined above; this step\n would require us to have parsed the command line for --depth, etc.\n\n * Run \"git checkout -b\" to create an initial checkout; this step\n would require us to have parsed the command line for --branch,\n etc.\n\nand the current code Kevin is basing his work on is not quite in\nthat shape. This round of the patches may be RFC so it may be OK\nbut the ready-for-review work may need to do the refactoring to get\nit close to the above shape as a preparatory step before doing\nanything else. Once that is done, Kevin's series (other than the\npart that acutally does the resumable static file download) will\nbecome a very easily understood \"Ah, we know where to prime the well\nfrom, so let's do that first\" step inserted immediately before the\n\"Issue 'git fetch'\" step.\n\n"},{"id":"302775","messageId":"20160927215143.GA32622@starla","threadId":"44095","inReplyTo":"1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com","subject":"Re: [PATCH 00/11] Resumable clone","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-27T21:51:43Z","receivedAt":"2016-09-27T21:51:49Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Kevin Wern wrote:\n> Hey, all,\n> \n> It's been a while (sent a very short patch in May), but I've\n> still been working on the resumable clone feature and checking up on\n> the mailing list for any updates. After submitting the prime-clone\n> service alone, I figured implementing the whole thing would be the best\n> way to understand the full scope of the problem (this is my first real\n> contribution here, and learning while working on such an involved\n> feature has not been easy). \n\nThank you for working on this. I'm hugely interested in this\nfeature as both a cheapskate sysadmin and as a client with\nunreliable connectivity (and I've barely been connected this month).\n\n> This is a functional implementation handling a direct http/ftp URI to a\n> single, fully connected packfile (i.e. the link is a direct path to the\n> file, not a prefix or guess). My hope is that this acts as a bare\n> minimum cross-section spanning the full requirments that can expand in\n> width as more cases are added (.info file, split bundle, daemon\n> download service). This is certainly not perfect, but I think it at\n> least prototypes each component involved in the workflow.\n> \n> This patch series is based on jc/bundle, because the logic to find the\n> tips of a pack's history already exists there (I call index-pack\n> --clone-bundle on the downloaded file, and read the file to write the\n> references to a temporary directory). If I need to re-implement this\n> logic or base it on another branch, let me know. For ease of pulling\n> and testing, I included the branch here:\n> \n> https://github.com/kevinwern/git/tree/feature/prime-clone\n\nAm I correct this imposes no additional storage burden for servers?\n\n(unlike the current .bundle dance used by kernel.org:\n https://www.kernel.org/cloning-linux-from-a-bundle.html )\n\nThat would be great!\n\n> Although there are a few changes internally from the last patch,\n> the \"alternate resource\" url to download is configured on the\n> server side in exactly the same way:\n> \n> [primeclone]\n> \turl = http://location/pack-$NAME.pack\n> \tfiletype = pack\n\nIf unconfigured, I wonder if a primeclone pack can be inferred by\nthe existence of a pack bitmap (or merely being the biggest+oldest\npack for dumb HTTP).\n\n> The prime-clone service simply outputs the components as:\n> \n> ####url filetype\n> 0000\n> \n> On the client side, the transport_prime_clone and\n> transport_download_primer APIs are built to be more robust (i.e. read\n> messages without dying due to protocol errors), so that git clone can\n> always try them without being dependent on the capability output of\n> git-upload-pack. transport_download_primer is dependent on the success\n> of transport_prime_clone, but transport_prime_clone is always run on an\n> initial clone. Part of achieving this robustness involves adding\n> *_gentle functions to pkt_line, so that prime_clone can fail silently\n> without dying.\n> \n> The transport_download_primer function uses a resumable download,\n> which is applicable to both automatic and manual resuming. Automatic\n> is programmatically reconnecting to the resource after being\n> interrupted (up to a set number of times). Manual is using a newly\n> taught --resume option on the command line:\n> \n> git clone --resume \n\nI think calling \"git fetch\" should resume, actually.\nIt would reduce the learning curve and seems natural to me:\n\"fetch\" is jabout grabbing whatever else appeared since the\nlast clone/fetch happened.\n\n> Right now, a manually resumable directory is left behind only if the\n> *client* is interrupted while a new junk mode, JUNK_LEAVE_RESUMABLE,\n> is set (right before the download). For an initial clone, if the\n> connection fails after automatic resuming, the client erases the\n> partial resources and falls through to a normal clone. However, once a\n> resumable directory is left behind by the program, it is NEVER\n> deleted/abandoned after it is continued with --resume.\n\nI'm not sure if erasing partial resources should ever be done\nautomatically. Perhaps a note to the user explaining the\nsituation and potential ways to correct/resume it.\n\n> I think determining when a resource is \"unsalvageable\" should be more\n> nuanced. Especially in a case where a connection is perpetually poor\n> and the user wishes to resume over a long period of time. The timeout\n> logic itself *definitely* needs more nuance than \"repeat 5 times\", such\n> as expanding wait times and using earlier successes when deciding to\n> try again. Right now, I think the most important part of this patch is\n> that these two paths (falling through after a failed download, exiting\n> to be manually resumed later) exist.\n> \n> Off the top of my head, outstanding issues/TODOs inlcude:\n> \t- The above issue of determining when to fall through, when to\n> \t reattempt, and when to write the resumable info and exit\n> \t in git clone.\n\nMy current (initial) reaction is: you're overthinking this.\n\nI think it's less surprising to a user to always write resumable\ninfo and let them know how to resume (or abort); rather than\ntrying to second-guess their intent.\n\nGoing by the zero-one-infinity rule, I'd probably attempt an\nauto-retry once on socket errors before saving state and bailing\nwith instructions on how to resume.\n\nIf they hit Ctrl-C manually, then just tell them they can\neither resume or \"rm -r\" the directory.\n\n> \t- Creating git-daemon service to download a resumable resource.\n> \t Pretty straightforward, I think, especially if\n> \t http.getanyfile already exists. This falls more under\n> \t \"haven't gotten to yet\" than dilemma.\n\nI think this could be handled natively by git-daemon for\ntrickling data to slow clients in the existing event loop (and\nexpanded to use epoll/kqueue). Similar to how X-Sendfile works\nwith (Apache|lighttpd) or X-Accel in nginx.\n\nThis would be cheaper than wasting a process (or thread) to\ntrickle to low-bandwidth clients. But this may be an\noptimization we defer until we've ironed out other parts.\n\n> \t- Logic for git clone to determine when a full clone would\n> \t be superior, such as when a clone is local or a reference is\n> \t given.\n> \t- Configuring prime-clone for multiple resources, in two\n> \t dimensions: (a) resources to choose from (e.g. fall back to\n> \t a second resource if the first one doesn't work) and (b)\n> \t resources to be downloaded together or in sequence (e.g.\n> \t download http://host/this, then http://host/that). Maybe\n> \t prime-clone could also handle client preferences in terms of\n> \t filetype or protocol. For this, I just have to re-read a few\n> \t discussions about the filetypes we use to see if there are\n> \t any outliers that aren't representable in this way. I think\n> \t this is another \"haven't gotten to yet\".\n\nPerhaps using the existing http-alternates (and automatic\nprimeclone pack inference I wrote about above) can be done.\n\n\n\n> \t- Creating the logic to guess a packfile, and append that to a\n> \t prefix specified by the admin. Additionally, allowing the\n> \t admin to use a custom script to use their own logic to\n> \t output the URL.\n\nYes :) Though I'm not sure if the custom script is necessary.\n"},{"id":"302777","messageId":"xmqqshslkndk.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"20160927215143.GA32622@starla","subject":"Re: [PATCH 00/11] Resumable clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-27T22:07:35Z","receivedAt":"2016-09-27T22:07:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n>> [primeclone]\n>> \turl = http://location/pack-$NAME.pack\n>> \tfiletype = pack\n>\n> If unconfigured, I wonder if a primeclone pack can be inferred by\n> the existence of a pack bitmap (or merely being the biggest+oldest\n> pack for dumb HTTP).\n\nThat would probably be a nice heuristics but it is unclear who\nshould find that out at runtime. The downloading side would not\nhave a visiblity into directory listing.\n\n>> git clone --resume \n>\n> I think calling \"git fetch\" should resume, actually.\n> It would reduce the learning curve and seems natural to me:\n> \"fetch\" is jabout grabbing whatever else appeared since the\n> last clone/fetch happened.\n\nI hate say this but it sounds to me like a terrible idea. At that\npoint when you need to resume, there is not even ref for \"fetch\" to\nbase its incremental work off of. It is better to keep the knowledge\nof this \"priming\" dance inside \"clone\". Hopefully the original \"clone\"\nwhose connection was disconnected in the middle would automatically\nattempt resuming and \"clone --resume\" would not be as often as needed.\n"},{"id":"302788","messageId":"20160928044005.GA3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"xmqqzin7in2c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 01/11] Resumable clone: create service git-prime-clone","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T04:40:05Z","receivedAt":"2016-09-28T04:40:16Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Fri, Sep 16, 2016 at 01:53:15PM -0700, Junio C Hamano wrote:\n> Kevin Wern writes:\n> \n> > Create git-prime-clone, a program to be executed on the server that\n> > returns the location and type of static resource to download before\n> > performing the rest of a clone.\n> >\n> > Additionally, as this executable's location will be configurable (see:\n> > upload-pack and receive-pack), add the program to\n> > BINDIR_PROGRAMS_NEED_X, in addition to the usual builtin places. Add\n> > git-prime-clone executable to gitignore, as well\n> >\n> > Signed-off-by: Kevin Wern \n> > ---\n> \n> I wonder if we even need a separate service like this.\n> \n> Wouldn't a new protocol capability that is advertised from\n> upload-pack sufficient to tell the \"git clone\" that it can\n> and should consider priming from this static resource?\n\nThe short answer is yes, it could be done that way. Both methods--extending\nupload-pack and creating a new service--were suggested in different\ndiscussions.\n\nHowever, my thought was to implement the separate service because:\n\t- It is much easier for an admin trying the feature to sanity check the\n\t output of an executable, compared to passing messages to upload-pack.\n\t- In the other scenario, upload-pack might get too expansive in size\n\t and scope--not only codewise, but in terms of config namespace if\n\t \"uploadpack\" concerns too many things that are only tangentially\n\t related (the properties of the primer resource).\n\t- The transport_prime_clone API can be called independent of other\n\t transport API functions, which might prove useful when revisiting or\n\t refactoring code.\n\t- You favored the creation of a service in our original discussion [1].\n\t I'm not sure if your reasoning was similar to mine.\n\nIt definitely was a tight decision--for me, ultimately weighing the value added\nin usability (point 1) against the need for a \"failsafe\" implementation. All\nthe other points are more speculative, IMO, but the first was strong enough for\nme.\n\nWhat do you think?\n\n[1] http://www.spinics.net/lists/git/msg269992.html\n\n> Two minor comments:\n> \n> - For whom are you going to localize these strings? This program\n> is running on the server side and we do not know the locale\n> preferred by the end-user who is sitting on the other end of the\n> connection, no?\n> \n> - Turn \"}\\n\\s+else \" into \"} else \", please.\n\nThese are fair points. Changing for the revised version.\n\n- Kevin\n"},{"id":"302789","messageId":"20160928044259.GB3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"xmqqoa3nij5z.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 03/11] pkt-line: create gentle packet_read_line functions","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T04:42:59Z","receivedAt":"2016-09-28T04:43:08Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Fri, Sep 16, 2016 at 03:17:28PM -0700, Junio C Hamano wrote:\n> Kevin Wern writes:\n> \n> > \t/* And complain if we didn't get enough bytes to satisfy the read. */\n> > \tif (ret < size) {\n> > -\t\tif (options & PACKET_READ_GENTLE_ON_EOF)\n> > +\t\tif (options & (PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ALL))\n> > \t\t\treturn -1;\n> \n> The name _ALL suggested to me that there may be multiple \"under this\n> condition, be gentle\", \"under that condition, be gentle\", and _ALL\n> is used as a catch-all \"under any condition, be gentle\". If you\n> defined _ALL symbol to have all GENTLE bits on, this line could have\n> become\n> \n> \tif (options & PACKET_READ_GENTLE_ALL)\n> \n> > @@ -205,15 +209,23 @@ int packet_read(int fd, char **src_buf, size_t *src_len,\n> > \tif (ret < 0)\n> > \t\treturn ret;\n> > \tlen = packet_length(linelen);\n> > -\tif (len < 0)\n> > +\tif (len < 0) {\n> > +\t\tif (options & PACKET_READ_GENTLE_ALL)\n> > +\t\t\treturn -1;\n> \n> On the other hand, however, you do want to die here when only\n> GENTLE_ON_EOF is set.\n> \n> Taking the above two observations together, I'd have to say that\n> _ALL is probably a misnomer. I agree with a need for a flag with\n> the behaviour you defined in this patch, though.\n\nOK, my thought is either:\n\t- Come up with a name for a flag, or flags, for the other cases (to\n\t check in the function, i.e. PACKET_READ_GENTLE_INVALID), and still\n\t pass in PACKET_READ_GENTLE_ALL, which is all those bits on plus\n\t *_GENTLE_ON_EOF.\n\t- Come up with a better name for this single flag, like\n\t PACKET_READ_DONT_DIE ... only better.\n\nWhat would you suggest here?\n"},{"id":"302790","messageId":"20160928044349.GC3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"CACsJy8ARu_PkduP-McnE26OFEgr_YAkLtmfaxwYX7i9m47mJmg@mail.gmail.com","subject":"Re: [PATCH 02/11] Resumable clone: add prime-clone endpoints","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T04:43:49Z","receivedAt":"2016-09-28T04:43:57Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Mon, Sep 19, 2016 at 08:15:00PM +0700, Duy Nguyen wrote:\n> We also have an exception for select_getanyfile() below. I think it's\n> time we add a function callback in struct rpc_service to run each\n> service the way they want. Then prime-clone won't need an exception\n> (neither does select_anyfile, mostly)\n> -- \n> Duy\n\nGreat idea! I'll definitely do that if we decide to go with the service I\nimplemented.\n"},{"id":"302791","messageId":"20160928044428.GD3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"CACsJy8B1bbKBhg1ke4u6PV3k4FWz-bhBPyN2X=mV2Z2=8Mhy=A@mail.gmail.com","subject":"Re: [PATCH 11/11] Resumable clone: implement primer logic in git-clone","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T04:44:28Z","receivedAt":"2016-09-28T04:44:36Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Mon, Sep 19, 2016 at 09:04:40PM +0700, Duy Nguyen wrote:\n> On Fri, Sep 16, 2016 at 7:12 AM, Kevin Wern wrote:\n> > builtin/clone.c | 590 +++++++++++++++++++++++++++++++++++++-------\n> \n> Argh.. this is too big for my brain at this hour. It might be easier\n> to follow if you separate out some code move (I think I've seen some,\n> not sure). I'll try to have another look when I find time. But it's\n> great to hear from you again, the pleasant surprise in my inbox today,\n> as I thought we lost you ;-) There's hope for resumable clone maybe\n> before 2018 again.\n\nSorry, I didn't mean to hurt anyone haha. I probably should have broken it down\ninto the bare process (priming from static resource, with no options), and the\nresume option.\n\nThanks so much for your feedback! :)\n"},{"id":"302792","messageId":"20160928044622.GE3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"xmqq37kzigvf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T04:46:22Z","receivedAt":"2016-09-28T04:46:29Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Fri, Sep 16, 2016 at 04:07:00PM -0700, Junio C Hamano wrote:\n> Kevin Wern writes:\n> \n> > Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child\n> > process.\n> >\n> > This will be used by git clone when calling index-pack on a downloaded\n> > packfile.\n> \n> If it is just one caller, would't it make more sense for that caller\n> set no_stdout explicitly itself?\n\nI based the calling code in do_index_pack on dissociate_from_references, which\nuses run_command_v_opt, so it never occured to me to do that. I thought it was\njust good, uniform style and encapsulation. Like how transport's methods and\ninternals aren't really intended to be changed or accessed--unless it's through\nthe APIs we create.\n\nHowever, I don't feel very strongly about this, so I'm okay with this change.\n"},{"id":"302794","messageId":"20160928054951.GF3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"xmqqy42rh156.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 11/11] Resumable clone: implement primer logic in git-clone","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T05:49:51Z","receivedAt":"2016-09-28T05:50:01Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Fri, Sep 16, 2016 at 04:32:05PM -0700, Junio C Hamano wrote:\n> > -----------\n> > @@ -172,6 +173,12 @@ objects from the source repository into a pack in the cloned repository.\n> > \tvia ssh, this specifies a non-default path for the command\n> > \trun on the other end.\n> > \n> > +--prime-clone ::\n> > +-p ::\n> \n> Not many other options have single letter shorthand. Is it expected\n> that it is worth to let this option squat on a short-and-sweet \"-p\",\n> perhaps because it is so frequently used?\n\nI based my decision more on precedent than value--the \"--upload-pack\" option\nhad the shorthand \"-u\".\n\n> > @@ -40,17 +42,20 @@ static const char * const builtin_clone_usage[] = {\n> > \n> > static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n> > static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n> > +static int option_resume;\n> > static char *option_template, *option_depth;\n> > -static char *option_origin = NULL;\n> > +static const char *option_origin = NULL;\n> \n> Is this change related to anything you are doing here?\n> \n> If you are fixing things while at it, please don't ;-) If you really\n> want to, please also remove \" = NULL\", from this line and also from\n> the next line. Also do not add \" = NULL\" at the end of alt_res.\n> \n\nYes, it was because remote_config uses all const char* strings, and .name gets\nassigned to option_origin when it pulls the previous info from config. I looked\nto see if option_origin's underlying string is ever modified in a way that\nmakes it ineligible to be const, and it wasn't.\n\nSo, I should remove the NULLs here? And then initalize alt_res to NULL in\ncmd_clone?\n\n> > static char *option_branch = NULL;\n> > ...\n> > +static const struct alt_resource *alt_res = NULL;\n> \n> > +static char *get_filename(const char *dir)\n> > +{\n> > +\tchar *dir_copy = xstrdup(dir);\n> > +\tstrip_trailing_slashes(dir_copy);\n> > +\tchar *filename, *final = NULL;\n> > +\n> > +\tfilename = find_last_dir_sep(dir);\n> > +\n> > +\tif (filename && *(++filename))\n> > +\t\tfinal = xstrdup(filename);\n> > +\n> > +\tfree(dir_copy);\n> > +\treturn final;\n> > +}\n> \n> Hmph, don't we have our own basename(3) lookalike that knows about\n> dir-sep already?\n\nWhoops, did not catch that. Thanks!\n\n> > @@ -562,7 +614,7 @@ static void write_remote_refs(const struct ref *local_refs)\n> > \t\tdie(\"%s\", err.buf);\n> > \n> > \tfor (r = local_refs; r; r = r->next) {\n> > -\t\tif (!r->peer_ref)\n> > +\t\tif (!r->peer_ref || ref_exists(r->peer_ref->name))\n> > \t\t\tcontinue;\n> > \t\tif (ref_transaction_create(t, r->peer_ref->name, r->old_oid.hash,\n> > \t\t\t\t\t 0, NULL, &err))\n> \n> What is this change about?\n\nBecause resumable clone is supposed to handle halting, I included the\npossibility of halting after the final fetch happened and remote refs were\nupdated (basically anything after resumable download happens). This change\nbasically says \"If the remote ref was not previously written, then write it.\"\nBecause the write is all-or-nothing, this means the logic should either write\nall of the intended references, or none of them (because they were written\nin the previous invocation).\n\n> > +static const char *setup_and_index_pack(const char *filename)\n> > +{\n> > +\tconst char *primer_idx_path = NULL, *primer_bndl_path = NULL;\n> > +\tprimer_idx_path = replace_extension(filename, \".pack\", \".idx\");\n> > +\tprimer_bndl_path = replace_extension(filename, \".pack\", \".bndl\");\n> > +\n> > +\tif (!(primer_idx_path && primer_bndl_path)) {\n> > +\t\twarning(\"invalid pack filename '%s', falling back to full \"\n> > +\t\t\t\"clone\", filename);\n> > +\t\treturn NULL;\n> > +\t}\n> > +\n> > +\tif (!file_exists(primer_bndl_path)) {\n> > +\t\tif (do_index_pack(filename, primer_idx_path)) {\n> > +\t\t\twarning(\"could not index primer pack, falling back to \"\n> > +\t\t\t\t\"full clone\");\n> > +\t\t\treturn NULL;\n> > +\t\t}\n> > +\t}\n> \n> Can it be another (undetected) failure mode that .bndl somehow\n> already existed, but not .idx, leaving the resulting object store in\n> an incosistent state? Can do_index_pack() fail and leave .bndl\n> behind to get you into such a state?\n\nI don't think so, based on looking at builtin/index-pack.c. write_bundle_file()\nhappens at the very end of final(), which is long after the process created the\nresulting .idx, or has died in the event one is not created. There also is no\nautomatic cleanup of .idx after that if the process dies somehow after .bndl\nis written.\n\n> > +static int write_bundle_refs(const char *bundle_filename)\n> > +{\n> > +\tstruct ref_transaction *t;\n> > +\tstruct bundle_header history_tips;\n> > +\tconst char *temp_ref_base = \"resume\";\n> > +\tstruct strbuf err = STRBUF_INIT;\n> > +\tint i;\n> > +\n> > +\tinit_bundle_header(&history_tips, bundle_filename);\n> > +\tread_bundle_header(&history_tips);\n> > +\n> > +\tt = ref_transaction_begin(&err);\n> > +\tfor (i = 0; i < history_tips.references.nr; i++) {\n> > +\t\tstruct strbuf ref_name = STRBUF_INIT;\n> > +\t\tstrbuf_addf(&ref_name, \"refs/temp/%s/%s/temp-%s\",\n> > +\t\t\t option_origin, temp_ref_base,\n> > +\t\t\t sha1_to_hex(history_tips.references.list[i].sha1));\n> \n> Can we do this without polluting refs/temp/ namespace?\n> \n> I am imagining that you are first fetching the .pack file from\n> sideways when primer service is available, running index-pack on it\n> to produce the bundle, and the step after that is to run \"git fetch\"\n> against the original remote to fill the gap between the bit-stale\n> history you got in the bundle and the reality that has progressed\n> since the primer pack was made, and you need a way to tell to the\n> other end that you already have the history leading to these refs\n> when you run \"git fetch\". I think a bit better way to do so is to\n> send these has \".have\" while you run the \"fetch\".\n> \n> Wouldn't it do if you add the \"--advertise-bundle-tips=\"\n> option to \"git fetch\", move the code to read the bundle header to\n> it, and point the bundle's filename with the option when you spawn\n> \"git fetch\"?\n\nMy implementation is definitely a minimum working model. I was hoping to\nmove to something cleaner.\n\nIs this new option preferable to leveraging the \"--reference\" option you\nmentioned in the earlier discussion? I thought that was a clean solution,\nespecially because it uses an existing option. Additionally, would there be\nany use for this new fetch option outside of cloning? If so, I could see the\nvalue--otherwise, knowing that we want to keep as much resume-specific\nknowledge inside clone as possible, \"--reference\" with a .bndl seems better.\n\n- Kevin\n"},{"id":"302796","messageId":"20160928064156.GG3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"xmqqfuozihuv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 07/11] Resumable clone: add resumable download to http/curl","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T06:41:56Z","receivedAt":"2016-09-28T06:42:04Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Fri, Sep 16, 2016 at 03:45:44PM -0700, Junio C Hamano wrote:\n> > @@ -1136,7 +1138,10 @@ static int handle_curl_result(struct slot_results *results)\n> > \t\t\t\tcurl_easy_strerror(results->curl_result),\n> > \t\t\t\tsizeof(curl_errorstr));\n> > #endif\n> > -\t\treturn HTTP_ERROR;\n> > +\t\tif (results->http_code >= 400)\n> > +\t\t\treturn HTTP_ERROR;\n> > +\t\telse\n> > +\t\t\treturn HTTP_ERROR_RESUMABLE;\n> > \t}\n> > }\n> \n> Hmm, is \"anything below 400\" a good definition of resumable errors?\n\nDefinitely a rough definition, but I think it covers the majority of cases. I\nbased this on the fact that my example resumable errors (disconnections during\ndownload) either had results->http_code set to 206 or 0, and that http errors\ngenerally indicate some underlying issue that can't be solved with a repeat\nrequest (there are plenty of exceptions, such as 503 or 504 errors that happen\nintermittently). I thought that was enough to be a placeholder in the model. I\ndid consider something like \"has (the file changed size or http_code been\n206, some indication of success happened) in the last [x] attempts,\" but I\nquestion the precision of something like that, especially before exploring\nother tried-and-true solutions.\n\nThis patch is definitely the sketchiest part of my series, IMO. The concept\nisn't fleshed out enough, even theoretically--containing only the element of\nretrying with some very basic components to those decisions. I think that this\nwill ultimately require fishing through other examples of resumable downloads\n(which was very painful to read when I tried). I know curl has a --retry option,\nbut doesn't include it in libcurl...harumph. I'll probably try to read that\nimplementation and chromium's resumable download code again.\n\nI'll get back to you with a better plan here once I read through a few\nexamples.\n\n- Kevin\n"},{"id":"302797","messageId":"20160928064520.GH3762@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"CACsJy8C+0M8o15E+iVX+f=izO2CtK7U642NC=XXaG-g41wYAUg@mail.gmail.com","subject":"Re: [PATCH 04/11] Resumable clone: add prime-clone to remote-curl","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T06:45:20Z","receivedAt":"2016-09-28T06:45:28Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Mon, Sep 19, 2016 at 08:52:34PM +0700, Duy Nguyen wrote:\n> \n> A brief overview for this service in\n> Documentation/technical/http-protocol.txt (and maybe\n> Documentation/gitremote-helpers.txt as well) would be great help. It's\n> a bit hard to follow because at this point I don't know anything about\n> the server side (and on top of that I was confused between http\n> send/receive vs transport send/receive, but this is my fault).\n> \n\nI figured I would miss something in this vein. So many things to cover!\n\nThanks again for reading.\n\n- Kevin\n"},{"id":"302817","messageId":"xmqqy42cj5g1.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"xmqqshslkndk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 00/11] Resumable clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-28T17:32:30Z","receivedAt":"2016-09-28T17:32:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>>> git clone --resume \n>>\n>> I think calling \"git fetch\" should resume, actually.\n>> It would reduce the learning curve and seems natural to me:\n>> \"fetch\" is jabout grabbing whatever else appeared since the\n>> last clone/fetch happened.\n>\n> I hate say this but it sounds to me like a terrible idea. At that\n> point when you need to resume, there is not even ref for \"fetch\" to\n> base its incremental work off of. It is better to keep the knowledge\n> of this \"priming\" dance inside \"clone\". Hopefully the original \"clone\"\n> whose connection was disconnected in the middle would automatically\n> attempt resuming and \"clone --resume\" would not be as often as needed.\n\nAfter sleeping on this, I want to take the above back.\n\nI think teaching \"git fetch\" about the \"resume\" part makes tons of\nsense.\n\nWhat \"git clone\" should have been was:\n\n * Parse command line arguments;\n\n * Create a new repository and go into it; this step would\n require us to have parsed the command line for --template,\n , --separate-git-dir, etc.\n\n * Talk to the remote and do get_remote_heads() aka ls-remote\n output;\n\n * Decide what fetch refspec to use, which alternate object store\n to borrow from; this step would require us to have parsed the\n command line for --reference, --mirror, --origin, etc;\n\n --- we'll insert something new here ---\n\n * Issue \"git fetch\" with the refspec determined above; this step\n would require us to have parsed the command line for --depth, etc.\n\n * Run \"git checkout -b\" to create an initial checkout; this step\n would require us to have parsed the command line for --branch,\n etc.\n\nEven though the current code conceptually does the above, these\nsteps are not cleanly separated as such. I think our update to gain\n\"resumable clone\" feature on the client side need to start by\nrefactoring the current code, before learning \"resumable clone\", to\nlook like the above.\n\nOnce we do that, we can insert an extra step before the step that\nruns \"git fetch\" to optionally [*1*] grab the extra piece of\ninformation Kevin's \"prime-clone\" service produces [*2*], and store\nit in the \"new repository\" somewhere [*3*].\n\nAnd then, as you suggested, an updated \"git fetch\" can be taught to\nnotice the priming information left by the previous step, and use it\nto attempt to download the pack until success, and to index that\npack to learn the tips that can be used as \".have\" entries in the\nrequest. From the original server's point of view, this fetch\nrequest would \"want\" the same set of objects, but would appear as\nan incremental update.\n\nOf course, the final step that happens in \"git clone\", i.e. the\ninitial checkout, needs to be done somehow, if your user decides to\nresume with \"git fetch\", as \"git fetch\" _never_ touches the working\ntree. So for that purpose, the primary end-user facing interface\nmay still have to be \"git clone --resume \". That would\nprobably skip all four steps in the above sequence, the new\n\"download priming information\" step and go directly to the step that\nruns \"git fetch\".\n\nI do agree that is a much better design, and the crucial design\ndecision that makes it a better design is your making \"git fetch\"\naware of this \"ah, we have the instruction left in this repository\nhow to prime its object store\" information.\n\nThanks.\n\n\n[Footnotes]\n\n*1* It is debatable if it would be an overall win to use the \"first\n prime by grabbing a large packfile\" clone if we are doing\n shallow or single-branch clone, hence \"optionally\". It is\n important to notice that we already have enough information to\n base the decision at this point in the above sequence.\n\n*2* As I said, I do not think it needs to be a separate new service,\n and I suspect it may be a better design to carry it over the\n protocol extension. At this point in the above sequence, we\n have done an equivalent of ls-remote and if we designed a\n protocol extension to carry the information we should already\n have it. If we use a separate new service, we can of course\n make a separate connection to ask about \"prime-clone\"\n information. The way this piece of information is transmitted\n is of secondary importance.\n\n*3* In addition to the \"prime-clone\" information, we may need to\n store some information that is only known to \"clone\" (perhaps\n because it was given from the command line) to help the final\n \"checkout -b\" step to know what to checkout around here, in case\n the next \"fetch\" step is interrupted and killed.\n\n\n"},{"id":"302819","messageId":"xmqqponnkiz7.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"20160928044622.GE3762@kwern-HP-Pavilion-dv5-Notebook-PC","subject":"Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-28T17:54:52Z","receivedAt":"2016-09-28T17:55:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Wern writes:\n\n> On Fri, Sep 16, 2016 at 04:07:00PM -0700, Junio C Hamano wrote:\n>> Kevin Wern writes:\n>> \n>> > Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child\n>> > process.\n>> >\n>> > This will be used by git clone when calling index-pack on a downloaded\n>> > packfile.\n>> \n>> If it is just one caller, would't it make more sense for that caller\n>> set no_stdout explicitly itself?\n>\n> I based the calling code in do_index_pack on dissociate_from_references, which\n> uses run_command_v_opt, so it never occured to me to do that. I thought it was\n> just good, uniform style and encapsulation. Like how transport's methods and\n> internals aren't really intended to be changed or accessed--unless it's through\n> the APIs we create.\n>\n> However, I don't feel very strongly about this, so I'm okay with this change.\n\nI am neutral and with no opinion. I may have offered a solution to\na problem that did not exist.\n\nI just got an impression that you were apologetic for having to add\nthis option that is otherwise useless and tried to suggest a simpler\nsolution that does not involve such an addition.\n"},{"id":"302823","messageId":"20160928180615.GA3942@kwern-HP-Pavilion-dv5-Notebook-PC","threadId":"44095","inReplyTo":"xmqqponnkiz7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT","fromName":"Kevin Wern","fromEmail":"kevin.m.wern@gmail.com","sentAt":"2016-09-28T18:06:15Z","receivedAt":"2016-09-28T18:06:42Z","isPatch":true,"sender":{"key":"kevin.m.wern@gmail.com","avatar":null},"body":"On Wed, Sep 28, 2016 at 10:54:52AM -0700, Junio C Hamano wrote:\n> \n> I just got an impression that you were apologetic for having to add\n> this option that is otherwise useless and tried to suggest a simpler\n> solution that does not involve such an addition.\n\nSorry, to be clear, I meant I was ok with your suggestion. That's what I meant\nby 'this change.'\n"},{"id":"302824","messageId":"xmqqd1jnkhpv.fsf@gitster.mtv.corp.google.com","threadId":"44095","inReplyTo":"xmqqy42cj5g1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 00/11] Resumable clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-28T18:22:04Z","receivedAt":"2016-09-28T18:26:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n> What \"git clone\" should have been was:\n>\n> * Parse command line arguments;\n>\n> * Create a new repository and go into it; this step would\n> require us to have parsed the command line for --template,\n> , --separate-git-dir, etc.\n>\n> * Talk to the remote and do get_remote_heads() aka ls-remote\n> output;\n>\n> * Decide what fetch refspec to use, which alternate object store\n> to borrow from; this step would require us to have parsed the\n> command line for --reference, --mirror, --origin, etc;\n>\n> --- we'll insert something new here ---\n>\n> * Issue \"git fetch\" with the refspec determined above; this step\n> would require us to have parsed the command line for --depth, etc.\n>\n> * Run \"git checkout -b\" to create an initial checkout; this step\n> would require us to have parsed the command line for --branch,\n> etc.\n>\n> Even though the current code conceptually does the above, these\n> steps are not cleanly separated as such. I think our update to gain\n> \"resumable clone\" feature on the client side need to start by\n> refactoring the current code, before learning \"resumable clone\", to\n> look like the above.\n>\n> Once we do that, we can insert an extra step before the step that\n> runs \"git fetch\" to optionally [*1*] grab the extra piece of\n> information Kevin's \"prime-clone\" service produces [*2*], and store\n> it in the \"new repository\" somewhere [*3*].\n>\n> And then, as you suggested, an updated \"git fetch\" can be taught to\n> notice the priming information left by the previous step, and use it\n> to attempt to download the pack until success, and to index that\n> pack to learn the tips that can be used as \".have\" entries in the\n> request. From the original server's point of view, this fetch\n> request would \"want\" the same set of objects, but would appear as\n> an incremental update.\n\nThinking about this even more, it probably makes even more sense to\nmove the new \"learn prime info and store it in repository somewhere,\nso that later re-invocation of 'git fetch' can take advantage of it\"\nstep _into_ \"git fetch\". That would allow \"git fetch\" in a freshly\ncreated empty repository take advantage of this feature for free.\n\nThe step that \"git clone\" internally drives \"git fetch\" would not\nactually be done by spawning a separate process with run_command()\nbecause we would want to reuse the connection we already have with\nthe server when \"git clone\" first talked to it to learn \"ls-remote\"\nequivalent (i.e. transport_get_remote_refs()). I wonder if we can\ndo without this early \"ls-remote\"; that would further simplify\nthings by allowing us to just spawn \"git fetch\" internally.\n\n"},{"id":"302837","messageId":"20160928204627.GA27878@starla","threadId":"44095","inReplyTo":"xmqqshslkndk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 00/11] Resumable clone","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-28T20:46:27Z","receivedAt":"2016-09-28T20:46:32Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> >> [primeclone]\n> >> \turl = http://location/pack-$NAME.pack\n> >> \tfiletype = pack\n> >\n> > If unconfigured, I wonder if a primeclone pack can be inferred by\n> > the existence of a pack bitmap (or merely being the biggest+oldest\n> > pack for dumb HTTP).\n> \n> That would probably be a nice heuristics but it is unclear who\n> should find that out at runtime. The downloading side would not\n> have a visiblity into directory listing.\n\nI think making a bunch of HEAD requests based on the contents of\n$GIT_DIR/objects/info/packs wouldn't be too expensive on either\nend, especially when HTTP/1.1 persistent connections + pipelining\nmay be used.\n"}]}