{"thread":{"id":"58220","subject":"[PATCH 0/5] Bundle URIs II: git clone --bundle-uri","startedAt":"2022-07-25T20:34:44Z","lastAt":"2022-08-30T13:33:50Z","messageCount":36,"participants":["Derrick Stolee via GitGitGadget","Josh Steadmon","Ævar Arnfjörð Bjarmason","Derrick Stolee","Junio C Hamano","Teng Long"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"459913","messageId":"pull.1300.git.1658781277.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":null,"subject":"[PATCH 0/5] Bundle URIs II: git clone --bundle-uri","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-25T20:34:32Z","receivedAt":"2022-07-25T20:34:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This is the second series building the bundle URI feature as discussed in\nthe previous series that added the design document [1]. This series does not\nmodify the design document, so the patches are independent and can be\napplied to the latest 'master'.\n\n[1]\nhttps://lore.kernel.org/git/pull.1248.v3.git.1658757188.gitgitgadget@gmail.com\n\nThis series brings in just enough logic that we can bootstrap clones from a\nsingle bundle using git clone --bundle-uri=.\n\n * Patch 1 adds a 'get' capability to 'git remote-https' which allows\n downloading the contents of a URI to a local file.\n * Patch 2 creates basic file-copy logic within a new bundle-uri.c file. It\n is not used until patch 3.\n * Patch 3 creates the git clone --bundle-uri= option, allowing Git to\n bootstrap a clone from a bundle, but get the remaining objects from the\n origin URL. (As of this patch, it only accepts a filename.)\n * Patch 4 extends the git clone --bundle-uri= option to allow file://\n and https:// URIs.\n * Patch 5 is a CLI helper to avoid using --bundle-uri and --depth at the\n same time in git clone.\n\nAs outlined in [1], the next steps after this are:\n\n 1. Allow parsing a bundle list as a config file at the given URI. The\n key-value format is unified with the protocol v2 verb (coming in (3)).\n [2]\n 2. Implement the protocol v2 verb, re-using the bundle list logic from (2).\n Use this to auto-discover bundle URIs during 'git clone' (behind a\n config option). [3]\n 3. Implement the 'creationToken' heuristic, allowing incremental 'git\n fetch' commands to download a bundle list from a configured URI, and\n only download bundles that are new based on the creation token values.\n [4]\n\nI have prepared some of this work as pull requests on my personal fork so\ncurious readers can look ahead to where we are going:\n\n[2] https://github.com/derrickstolee/git/pull/20\n\n[3] https://github.com/derrickstolee/git/pull/21\n\n[4] https://github.com/derrickstolee/git/pull/22\n\nNote: this series includes some code pulled out of the first series [1], and\nin particular the git fetch --bundle-uri= option is removed. The\nintention was to replace that with git bundle fetch , but that conflicts\nwith the work to refactor how subcommands are parsed. The git bundle fetch\nsubcommand could be added later for maximum flexibility on the client side,\nbut we can also move forward without it.\n\nThanks, -Stolee\n\nP.S. Here is the range-diff compared to v2 of the previous bundle URI\nseries:\n\n1: d444042dc4dcc < -: ------------- docs: document bundle URI standard\n2: 0a2cf60437f78 ! 1: 40808e92afb7b remote-curl: add 'get' capability\n @@ remote-curl.c: static void parse_fetch(struct strbuf *buf)\n \n +static void parse_get(struct strbuf *buf)\n +{\n -+\tstruct http_get_options opts = { 0 };\n +\tstruct strbuf url = STRBUF_INIT;\n +\tstruct strbuf path = STRBUF_INIT;\n +\tconst char *p, *space;\n @@ remote-curl.c: static void parse_fetch(struct strbuf *buf)\n +\tstrbuf_add(&url, p, space - p);\n +\tstrbuf_addstr(&path, space + 1);\n +\n -+\tif (http_get_file(url.buf, path.buf, &opts))\n ++\tif (http_get_file(url.buf, path.buf, NULL))\n +\t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n +\n +\tstrbuf_release(&url);\n @@ t/t5557-http-get.sh (new)\n +\tget $url file1\n +\tEOF\n +\n -+\ttest_must_fail git remote-http $url $url err &&\n ++\ttest_must_fail git remote-http $url err &&\n +\ttest_path_is_missing file1 &&\n +\tgrep \"failed to download file at URL\" err &&\n +\trm file1.temp\n @@ t/t5557-http-get.sh (new)\n +\n +\tEOF\n +\n -+\tGIT_TRACE2_PERF=1 git remote-http $url $url no_private_update = 1;\n - \t\t} else if (starts_with(capname, \"object-format\")) {\n - \t\t\tdata->object_format = 1;\n -+\t\t} else if (!strcmp(capname, \"get\")) {\n -+\t\t\tdata->get = 1;\n - \t\t} else if (mandatory) {\n - \t\t\tdie(_(\"unknown mandatory capability %s; this remote \"\n - \t\t\t \"helper probably needs newer version of Git\"),\n3: abec47564fd9c ! 2: 7d3159f0d9a29 bundle-uri: create basic file-copy logic\n @@ Commit message\n file, not a bundle list. Bundle lists will be implemented in a future\n change.\n \n + Note that the discovery of a temporary filename is slightly racy because\n + the odb_mkstemp() relies on the temporary file not existing. With the\n + current implementation being limited to file copies, we could replace\n + the copy_file() with copy_fd(). The tricky part comes in future changes\n + that send the filename to 'git remote-https' and its 'get' capability.\n + At that point, we need the file descriptor closed _and_ the file\n + unlinked. If we were to keep the file descriptor open for the sake of\n + normal file copies, then we would pollute the rest of the code for\n + little benefit. This is especially the case because we expect that most\n + bundle URI use will be based on HTTPS instead of file copies.\n +\n Signed-off-by: Derrick Stolee \n \n ## Makefile ##\n @@ bundle-uri.c (new)\n +#include \"refs.h\"\n +#include \"run-command.h\"\n +\n -+static void find_temp_filename(struct strbuf *name)\n ++static int find_temp_filename(struct strbuf *name)\n +{\n +\tint fd;\n +\t/*\n @@ bundle-uri.c (new)\n +\t * racy, but unlikely to collide.\n +\t */\n +\tfd = odb_mkstemp(name, \"bundles/tmp_uri_XXXXXX\");\n -+\tif (fd < 0)\n -+\t\tdie(_(\"failed to create temporary file\"));\n ++\tif (fd < 0) {\n ++\t\twarning(_(\"failed to create temporary file\"));\n ++\t\treturn -1;\n ++\t}\n ++\n +\tclose(fd);\n +\tunlink(name->buf);\n ++\treturn 0;\n +}\n +\n -+static int copy_uri_to_file(const char *uri, const char *file)\n ++static int copy_uri_to_file(const char *file, const char *uri)\n +{\n -+\t/* Copy as a file */\n -+\treturn copy_file(file, uri, 0444);\n ++\t/* File-based URIs only for now. */\n ++\treturn copy_file(file, uri, 0);\n +}\n +\n +static int unbundle_from_file(struct repository *r, const char *file)\n @@ bundle-uri.c (new)\n +\tint result = 0;\n +\tint bundle_fd;\n +\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n -+\tstruct strvec extra_index_pack_args = STRVEC_INIT;\n +\tstruct string_list_item *refname;\n +\tstruct strbuf bundle_ref = STRBUF_INIT;\n +\tsize_t bundle_prefix_len;\n @@ bundle-uri.c (new)\n +\tif ((bundle_fd = read_bundle_header(file, &header)) < 0)\n +\t\treturn 1;\n +\n -+\tif ((result = unbundle(r, &header, bundle_fd, &extra_index_pack_args)))\n ++\tif ((result = unbundle(r, &header, bundle_fd, NULL)))\n +\t\treturn 1;\n +\n +\t/*\n @@ bundle-uri.c (new)\n +\tint result = 0;\n +\tstruct strbuf filename = STRBUF_INIT;\n +\n -+\tfind_temp_filename(&filename);\n -+\tif ((result = copy_uri_to_file(uri, filename.buf)))\n ++\tif ((result = find_temp_filename(&filename)))\n ++\t\tgoto cleanup;\n ++\n ++\tif ((result = copy_uri_to_file(filename.buf, uri))) {\n ++\t\twarning(_(\"failed to download bundle from URI '%s'\"), uri);\n +\t\tgoto cleanup;\n ++\t}\n +\n -+\tif ((result = !is_bundle(filename.buf, 0)))\n ++\tif ((result = !is_bundle(filename.buf, 0))) {\n ++\t\twarning(_(\"file at URI '%s' is not a bundle\"), uri);\n +\t\tgoto cleanup;\n ++\t}\n +\n -+\tif ((result = unbundle_from_file(r, filename.buf)))\n ++\tif ((result = unbundle_from_file(r, filename.buf))) {\n ++\t\twarning(_(\"failed to unbundle bundle from URI '%s'\"), uri);\n +\t\tgoto cleanup;\n ++\t}\n +\n +cleanup:\n +\tunlink(filename.buf);\n4: f6255ec518857 < -: ------------- fetch: add --bundle-uri option\n-: ------------- > 3: 29e645a54ba7f clone: add --bundle-uri option\n5: bfbd11b48bf1b ! 4: f6bc3177332e8 bundle-uri: add support for http(s):// and file://\n @@ Metadata\n ## Commit message ##\n bundle-uri: add support for http(s):// and file://\n \n - The previous change created the 'git fetch --bundle-uri=' option.\n + The previous change created the 'git clone --bundle-uri=' option.\n Currently, must be a filename.\n \n Update copy_uri_to_file() to first inspect the URI for an HTTP(S) prefix\n @@ Commit message\n Signed-off-by: Derrick Stolee \n \n ## bundle-uri.c ##\n -@@ bundle-uri.c: static void find_temp_filename(struct strbuf *name)\n - \tunlink(name->buf);\n +@@ bundle-uri.c: static int find_temp_filename(struct strbuf *name)\n + \treturn 0;\n }\n \n -+static int download_https_uri_to_file(const char *uri, const char *file)\n -+{\n +-static int copy_uri_to_file(const char *file, const char *uri)\n ++static int download_https_uri_to_file(const char *file, const char *uri)\n + {\n +-\t/* File-based URIs only for now. */\n +-\treturn copy_file(file, uri, 0);\n +\tint result = 0;\n +\tstruct child_process cp = CHILD_PROCESS_INIT;\n +\tFILE *child_in = NULL, *child_out = NULL;\n @@ bundle-uri.c: static void find_temp_filename(struct strbuf *name)\n +\treturn result;\n +}\n +\n - static int copy_uri_to_file(const char *uri, const char *file)\n - {\n ++static int copy_uri_to_file(const char *filename, const char *uri)\n ++{\n +\tconst char *out;\n ++\n +\tif (skip_prefix(uri, \"https:\", &out) ||\n +\t skip_prefix(uri, \"http:\", &out))\n -+\t\treturn download_https_uri_to_file(uri, file);\n ++\t\treturn download_https_uri_to_file(filename, uri);\n +\n +\tif (!skip_prefix(uri, \"file://\", &out))\n +\t\tout = uri;\n +\n - \t/* Copy as a file */\n --\treturn copy_file(file, uri, 0444);\n -+\treturn !!copy_file(file, out, 0);\n ++\t/* Copy as a file */\n ++\treturn copy_file(filename, out, 0);\n }\n \n static int unbundle_from_file(struct repository *r, const char *file)\n \n - ## t/t5558-fetch-bundle-uri.sh ##\n -@@ t/t5558-fetch-bundle-uri.sh: test_expect_success 'fetch file bundle' '\n + ## t/t5558-clone-bundle-uri.sh ##\n +@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone with path bundle' '\n test_cmp expect actual\n '\n \n -+test_expect_success 'fetch file:// bundle' '\n -+\tgit init fetch-file &&\n -+\tgit -C fetch-file fetch --bundle-uri=\"file://$(pwd)/fetch-from/B.bundle\" &&\n -+\tgit -C fetch-file rev-parse refs/bundles/topic >actual &&\n -+\tgit -C fetch-from rev-parse topic >expect &&\n ++test_expect_success 'clone with file:// bundle' '\n ++\tgit clone --bundle-uri=\"file://$(pwd)/clone-from/B.bundle\" \\\n ++\t\tclone-from clone-file &&\n ++\tgit -C clone-file rev-parse refs/bundles/topic >actual &&\n ++\tgit -C clone-from rev-parse topic >expect &&\n +\ttest_cmp expect actual\n +'\n +\n @@ t/t5558-fetch-bundle-uri.sh: test_expect_success 'fetch file bundle' '\n +start_httpd\n +\n +test_expect_success 'fail to fetch from non-existent HTTP URL' '\n -+\ttest_must_fail git fetch --bundle-uri=\"$HTTPD_URL/does-not-exist\" 2>err &&\n ++\ttest_when_finished rm -rf test &&\n ++\tgit clone --bundle-uri=\"$HTTPD_URL/does-not-exist\" . test 2>err &&\n +\tgrep \"failed to download bundle from URI\" err\n +'\n +\n +test_expect_success 'fail to fetch from non-bundle HTTP URL' '\n ++\ttest_when_finished rm -rf test &&\n +\techo bogus >\"$HTTPD_DOCUMENT_ROOT_PATH/bogus\" &&\n -+\ttest_must_fail git fetch --bundle-uri=\"$HTTPD_URL/bogus\" 2>err &&\n ++\tgit clone --bundle-uri=\"$HTTPD_URL/bogus\" . test 2>err &&\n +\tgrep \"is not a bundle\" err\n +'\n +\n -+test_expect_success 'fetch HTTP bundle' '\n -+\tcp fetch-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n -+\tgit init fetch-http &&\n -+\tgit -C fetch-http fetch --bundle-uri=\"$HTTPD_URL/B.bundle\" &&\n -+\tgit -C fetch-http rev-parse refs/bundles/topic >actual &&\n -+\tgit -C fetch-from rev-parse topic >expect &&\n -+\ttest_cmp expect actual\n ++test_expect_success 'clone HTTP bundle' '\n ++\tcp clone-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n ++\n ++\tgit clone --no-local --mirror clone-from \\\n ++\t\t\"$HTTPD_DOCUMENT_ROOT_PATH/fetch.git\" &&\n ++\n ++\tgit clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n ++\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http &&\n ++\tgit -C clone-http rev-parse refs/bundles/topic >actual &&\n ++\tgit -C clone-from rev-parse topic >expect &&\n ++\ttest_cmp expect actual &&\n ++\n ++\ttest_config -C clone-http log.excludedecoration refs/bundle/\n +'\n +\n +# Do not add tests here unless they use the HTTP server, as they will\n6: a217e9a0640b4 < -: ------------- fetch: add 'refs/bundle/' to log.excludeDecoration\n-: ------------- > 5: e823b168ab725 clone: --bundle-uri cannot be combined with --depth\n\n\nDerrick Stolee (5):\n remote-curl: add 'get' capability\n bundle-uri: create basic file-copy logic\n clone: add --bundle-uri option\n bundle-uri: add support for http(s):// and file://\n clone: --bundle-uri cannot be combined with --depth\n\n Documentation/git-clone.txt | 7 ++\n Documentation/gitremote-helpers.txt | 9 ++\n Makefile | 1 +\n builtin/clone.c | 18 +++\n bundle-uri.c | 168 ++++++++++++++++++++++++++++\n bundle-uri.h | 14 +++\n remote-curl.c | 32 ++++++\n t/t5557-http-get.sh | 37 ++++++\n t/t5558-clone-bundle-uri.sh | 81 ++++++++++++++\n t/t5606-clone-options.sh | 8 ++\n 10 files changed, 375 insertions(+)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n create mode 100755 t/t5557-http-get.sh\n create mode 100755 t/t5558-clone-bundle-uri.sh\n\n\nbase-commit: e72d93e88cb20b06e88e6e7d81bd1dc4effe453f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1300%2Fderrickstolee%2Fbundle-redo%2Fclone-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1300/derrickstolee/bundle-redo/clone-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1300\n-- \ngitgitgadget\n"},{"id":"459914","messageId":"40808e92afb7bcf3e8e9b4b53d5e30b5e17816f8.1658781277.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.git.1658781277.gitgitgadget@gmail.com","subject":"[PATCH 1/5] remote-curl: add 'get' capability","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-25T20:34:33Z","receivedAt":"2022-07-25T20:34:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nA future change will want a way to download a file over HTTP(S) using\nthe simplest of download mechanisms. We do not want to assume that the\nserver on the other side understands anything about the Git protocol but\ncould be a simple static web server.\n\nCreate the new 'get' capability for the remote helpers which advertises\nthat the 'get' command is avalable. A caller can send a line containing\n'get ' to download the file at into the file at\n.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/gitremote-helpers.txt | 9 +++++++\n remote-curl.c | 32 +++++++++++++++++++++++++\n t/t5557-http-get.sh | 37 +++++++++++++++++++++++++++++\n 3 files changed, 78 insertions(+)\n create mode 100755 t/t5557-http-get.sh\n\ndiff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\nindex 6f1e269ae43..ed8da428c98 100644\n--- a/Documentation/gitremote-helpers.txt\n+++ b/Documentation/gitremote-helpers.txt\n@@ -168,6 +168,9 @@ Supported commands: 'list', 'import'.\n \tCan guarantee that when a clone is requested, the received\n \tpack is self contained and is connected.\n \n+'get'::\n+\tCan use the 'get' command to download a file from a given URI.\n+\n If a helper advertises 'connect', Git will use it if possible and\n fall back to another capability if the helper requests so when\n connecting (see the 'connect' command under COMMANDS).\n@@ -418,6 +421,12 @@ Supported if the helper has the \"connect\" capability.\n +\n Supported if the helper has the \"stateless-connect\" capability.\n \n+'get' ::\n+\tDownloads the file from the given `` to the given ``. If\n+\t`.temp` exists, then Git assumes that the `.temp` file is a\n+\tpartial download from a previous attempt and will resume the\n+\tdownload from that position.\n+\n If a fatal error occurs, the program writes the error message to\n stderr and exits. The caller should expect that a suitable error\n message has been printed if the child closes the connection without\ndiff --git a/remote-curl.c b/remote-curl.c\nindex b8758757ece..73fbdbddd84 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1286,6 +1286,33 @@ static void parse_fetch(struct strbuf *buf)\n \tstrbuf_reset(buf);\n }\n \n+static void parse_get(struct strbuf *buf)\n+{\n+\tstruct strbuf url = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tconst char *p, *space;\n+\n+\tif (!skip_prefix(buf->buf, \"get \", &p))\n+\t\tdie(_(\"http transport does not support %s\"), buf->buf);\n+\n+\tspace = strchr(p, ' ');\n+\n+\tif (!space)\n+\t\tdie(_(\"protocol error: expected ' ', missing space\"));\n+\n+\tstrbuf_add(&url, p, space - p);\n+\tstrbuf_addstr(&path, space + 1);\n+\n+\tif (http_get_file(url.buf, path.buf, NULL))\n+\t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n+\n+\tstrbuf_release(&url);\n+\tstrbuf_release(&path);\n+\tprintf(\"\\n\");\n+\tfflush(stdout);\n+\tstrbuf_reset(buf);\n+}\n+\n static int push_dav(int nr_spec, const char **specs)\n {\n \tstruct child_process child = CHILD_PROCESS_INIT;\n@@ -1564,9 +1591,14 @@ int cmd_main(int argc, const char **argv)\n \t\t\t\tprintf(\"unsupported\\n\");\n \t\t\tfflush(stdout);\n \n+\t\t} else if (skip_prefix(buf.buf, \"get \", &arg)) {\n+\t\t\tparse_get(&buf);\n+\t\t\tfflush(stdout);\n+\n \t\t} else if (!strcmp(buf.buf, \"capabilities\")) {\n \t\t\tprintf(\"stateless-connect\\n\");\n \t\t\tprintf(\"fetch\\n\");\n+\t\t\tprintf(\"get\\n\");\n \t\t\tprintf(\"option\\n\");\n \t\t\tprintf(\"push\\n\");\n \t\t\tprintf(\"check-connectivity\\n\");\ndiff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\nnew file mode 100755\nindex 00000000000..50b7dbcf957\n--- /dev/null\n+++ b/t/t5557-http-get.sh\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+\n+test_description='test downloading a file by URL'\n+\n+. ./test-lib.sh\n+\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'get by URL: 404' '\n+\turl=\"$HTTPD_URL/none.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\tget $url file1\n+\tEOF\n+\n+\ttest_must_fail git remote-http $url err &&\n+\ttest_path_is_missing file1 &&\n+\tgrep \"failed to download file at URL\" err &&\n+\trm file1.temp\n+'\n+\n+test_expect_success 'get by URL: 200' '\n+\techo data >\"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" &&\n+\n+\turl=\"$HTTPD_URL/exists.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\tget $url file2\n+\n+\tEOF\n+\n+\tGIT_TRACE2_PERF=1 git remote-http $url \n\nBefore implementing a way to fetch bundles into a repository, create the\nbasic logic. Assume that the URI is actually a file path. Future logic\nwill make this more careful to other protocols.\n\nFor now, we also only succeed if the content at the URI is a bundle\nfile, not a bundle list. Bundle lists will be implemented in a future\nchange.\n\nNote that the discovery of a temporary filename is slightly racy because\nthe odb_mkstemp() relies on the temporary file not existing. With the\ncurrent implementation being limited to file copies, we could replace\nthe copy_file() with copy_fd(). The tricky part comes in future changes\nthat send the filename to 'git remote-https' and its 'get' capability.\nAt that point, we need the file descriptor closed _and_ the file\nunlinked. If we were to keep the file descriptor open for the sake of\nnormal file copies, then we would pollute the rest of the code for\nlittle benefit. This is especially the case because we expect that most\nbundle URI use will be based on HTTPS instead of file copies.\n\nSigned-off-by: Derrick Stolee \n---\n Makefile | 1 +\n bundle-uri.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++\n bundle-uri.h | 14 +++++++\n 3 files changed, 119 insertions(+)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n\ndiff --git a/Makefile b/Makefile\nindex 1624471badc..7d5f48069ea 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -906,6 +906,7 @@ LIB_OBJS += blob.o\n LIB_OBJS += bloom.o\n LIB_OBJS += branch.o\n LIB_OBJS += bulk-checkin.o\n+LIB_OBJS += bundle-uri.o\n LIB_OBJS += bundle.o\n LIB_OBJS += cache-tree.o\n LIB_OBJS += cbtree.o\ndiff --git a/bundle-uri.c b/bundle-uri.c\nnew file mode 100644\nindex 00000000000..b35babc36aa\n--- /dev/null\n+++ b/bundle-uri.c\n@@ -0,0 +1,104 @@\n+#include \"cache.h\"\n+#include \"bundle-uri.h\"\n+#include \"bundle.h\"\n+#include \"object-store.h\"\n+#include \"refs.h\"\n+#include \"run-command.h\"\n+\n+static int find_temp_filename(struct strbuf *name)\n+{\n+\tint fd;\n+\t/*\n+\t * Find a temporary filename that is available. This is briefly\n+\t * racy, but unlikely to collide.\n+\t */\n+\tfd = odb_mkstemp(name, \"bundles/tmp_uri_XXXXXX\");\n+\tif (fd < 0) {\n+\t\twarning(_(\"failed to create temporary file\"));\n+\t\treturn -1;\n+\t}\n+\n+\tclose(fd);\n+\tunlink(name->buf);\n+\treturn 0;\n+}\n+\n+static int copy_uri_to_file(const char *file, const char *uri)\n+{\n+\t/* File-based URIs only for now. */\n+\treturn copy_file(file, uri, 0);\n+}\n+\n+static int unbundle_from_file(struct repository *r, const char *file)\n+{\n+\tint result = 0;\n+\tint bundle_fd;\n+\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n+\tstruct string_list_item *refname;\n+\tstruct strbuf bundle_ref = STRBUF_INIT;\n+\tsize_t bundle_prefix_len;\n+\n+\tif ((bundle_fd = read_bundle_header(file, &header)) < 0)\n+\t\treturn 1;\n+\n+\tif ((result = unbundle(r, &header, bundle_fd, NULL)))\n+\t\treturn 1;\n+\n+\t/*\n+\t * Convert all refs/heads/ from the bundle into refs/bundles/\n+\t * in the local repository.\n+\t */\n+\tstrbuf_addstr(&bundle_ref, \"refs/bundles/\");\n+\tbundle_prefix_len = bundle_ref.len;\n+\n+\tfor_each_string_list_item(refname, &header.references) {\n+\t\tstruct object_id *oid = refname->util;\n+\t\tstruct object_id old_oid;\n+\t\tconst char *branch_name;\n+\t\tint has_old;\n+\n+\t\tif (!skip_prefix(refname->string, \"refs/heads/\", &branch_name))\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_setlen(&bundle_ref, bundle_prefix_len);\n+\t\tstrbuf_addstr(&bundle_ref, branch_name);\n+\n+\t\thas_old = !read_ref(bundle_ref.buf, &old_oid);\n+\t\tupdate_ref(\"fetched bundle\", bundle_ref.buf, oid,\n+\t\t\t has_old ? &old_oid : NULL,\n+\t\t\t REF_SKIP_OID_VERIFICATION,\n+\t\t\t UPDATE_REFS_MSG_ON_ERR);\n+\t}\n+\n+\tbundle_header_release(&header);\n+\treturn result;\n+}\n+\n+int fetch_bundle_uri(struct repository *r, const char *uri)\n+{\n+\tint result = 0;\n+\tstruct strbuf filename = STRBUF_INIT;\n+\n+\tif ((result = find_temp_filename(&filename)))\n+\t\tgoto cleanup;\n+\n+\tif ((result = copy_uri_to_file(filename.buf, uri))) {\n+\t\twarning(_(\"failed to download bundle from URI '%s'\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif ((result = !is_bundle(filename.buf, 0))) {\n+\t\twarning(_(\"file at URI '%s' is not a bundle\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif ((result = unbundle_from_file(r, filename.buf))) {\n+\t\twarning(_(\"failed to unbundle bundle from URI '%s'\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+cleanup:\n+\tunlink(filename.buf);\n+\tstrbuf_release(&filename);\n+\treturn result;\n+}\ndiff --git a/bundle-uri.h b/bundle-uri.h\nnew file mode 100644\nindex 00000000000..8a152f1ef14\n--- /dev/null\n+++ b/bundle-uri.h\n@@ -0,0 +1,14 @@\n+#ifndef BUNDLE_URI_H\n+#define BUNDLE_URI_H\n+\n+struct repository;\n+\n+/**\n+ * Fetch data from the given 'uri' and unbundle the bundle data found\n+ * based on that information.\n+ *\n+ * Returns non-zero if no bundle information is found at the given 'uri'.\n+ */\n+int fetch_bundle_uri(struct repository *r, const char *uri);\n+\n+#endif\n-- \ngitgitgadget\n\n"},{"id":"459916","messageId":"29e645a54ba7f6f5d2bcd0590d0ba747ef18fef6.1658781277.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.git.1658781277.gitgitgadget@gmail.com","subject":"[PATCH 3/5] clone: add --bundle-uri option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-25T20:34:35Z","receivedAt":"2022-07-25T20:34:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nCloning a remote repository is one of the most expensive operations in\nGit. The server can spend a lot of CPU time generating a pack-file for\nthe client's request. The amount of data can clog the network for a long\ntime, and the Git protocol is not resumable. For users with poor network\nconnections or are located far away from the origin server, this can be\nespecially painful.\n\nAdd a new '--bundle-uri' option to 'git clone' to bootstrap a clone from\na bundle. If the user is aware of a bundle server, then they can tell\nGit to bootstrap the new repository with these bundles before fetching\nthe remaining objects from the origin server.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-clone.txt | 6 ++++++\n builtin/clone.c | 15 +++++++++++++++\n t/t5558-clone-bundle-uri.sh | 36 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+)\n create mode 100755 t/t5558-clone-bundle-uri.sh\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 632bd1348ea..60fedf7eb5e 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -323,6 +323,12 @@ or `--mirror` is given)\n \tfor `host.xz:foo/.git`). Cloning into an existing directory\n \tis only allowed if the directory is empty.\n \n+--bundle-uri=::\n+\tBefore fetching from the remote, fetch a bundle from the given\n+\t`` and unbundle the data into the local repository. The refs\n+\tin the bundle will be stored under the hidden `refs/bundle/*`\n+\tnamespace.\n+\n :git-clone: 1\n include::urls.txt[]\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex c4ff4643ecd..4224d562758 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -34,6 +34,7 @@\n #include \"list-objects-filter-options.h\"\n #include \"hook.h\"\n #include \"bundle.h\"\n+#include \"bundle-uri.h\"\n \n /*\n * Overall FIXMEs:\n@@ -77,6 +78,7 @@ static int option_filter_submodules = -1; /* unspecified */\n static int config_filter_submodules = -1; /* unspecified */\n static struct string_list server_options = STRING_LIST_INIT_NODUP;\n static int option_remote_submodules;\n+static const char *bundle_uri;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -160,6 +162,8 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"any cloned submodules will use their remote-tracking branch\")),\n \tOPT_BOOL(0, \"sparse\", &option_sparse_checkout,\n \t\t N_(\"initialize sparse-checkout file to include only files at root\")),\n+\tOPT_STRING(0, \"bundle-uri\", &bundle_uri,\n+\t\t N_(\"uri\"), N_(\"a URI for downloading bundles before fetching from origin remote\")),\n \tOPT_END()\n };\n \n@@ -1232,6 +1236,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (transport->smart_options && !deepen && !filter_options.choice)\n \t\ttransport->smart_options->check_self_contained_and_connected = 1;\n \n+\t/*\n+\t * Before fetching from the remote, download and install bundle\n+\t * data from the --bundle-uri option.\n+\t */\n+\tif (bundle_uri) {\n+\t\t/* At this point, we need the_repository to match the cloned repo. */\n+\t\trepo_init(the_repository, git_dir, work_tree);\n+\t\tif (fetch_bundle_uri(the_repository, bundle_uri))\n+\t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n+\t\t\t\tbundle_uri);\n+\t}\n \n \tstrvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n \trefspec_ref_prefixes(&remote->fetch,\ndiff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh\nnew file mode 100755\nindex 00000000000..f709bcb729c\n--- /dev/null\n+++ b/t/t5558-clone-bundle-uri.sh\n@@ -0,0 +1,36 @@\n+#!/bin/sh\n+\n+test_description='test fetching bundles with --bundle-uri'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'fail to clone from non-existent file' '\n+\ttest_when_finished rm -rf test &&\n+\tgit clone --bundle-uri=\"$(pwd)/does-not-exist\" . test 2>err &&\n+\tgrep \"failed to download bundle from URI\" err\n+'\n+\n+test_expect_success 'fail to clone from non-bundle file' '\n+\ttest_when_finished rm -rf test &&\n+\techo bogus >bogus &&\n+\tgit clone --bundle-uri=\"$(pwd)/bogus\" . test 2>err &&\n+\tgrep \"is not a bundle\" err\n+'\n+\n+test_expect_success 'create bundle' '\n+\tgit init clone-from &&\n+\tgit -C clone-from checkout -b topic &&\n+\ttest_commit -C clone-from A &&\n+\ttest_commit -C clone-from B &&\n+\tgit -C clone-from bundle create B.bundle topic\n+'\n+\n+test_expect_success 'clone with path bundle' '\n+\tgit clone --bundle-uri=\"clone-from/B.bundle\" \\\n+\t\tclone-from clone-path &&\n+\tgit -C clone-path rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"459917","messageId":"e823b168ab7254a0c56e549b240cce9b70d5a2f4.1658781277.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.git.1658781277.gitgitgadget@gmail.com","subject":"[PATCH 5/5] clone: --bundle-uri cannot be combined with --depth","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-25T20:34:37Z","receivedAt":"2022-07-25T20:34:58Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nA previous change added the '--bundle-uri' option, but did not check\nif the --depth parameter was included. Since bundles are not compatible\nwith shallow clones, provide an error message to the user who is\nattempting this combination.\n\nI am leaving this as its own change, separate from the one that\nimplements '--bundle-uri', because this is more of an advisory for the\nuser. There is nothing wrong with bootstrapping with bundles and then\nfetching a shallow clone. However, that is likely going to involve too\nmuch work for the client _and_ the server. The client will download all\nof this bundle information containing the full history of the\nrepository only to ignore most of it. The server will get a shallow\nfetch request, but with a list of haves that might cause a more painful\ncomputation of that shallow pack-file.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-clone.txt | 3 ++-\n builtin/clone.c | 3 +++\n t/t5606-clone-options.sh | 8 ++++++++\n 3 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 60fedf7eb5e..d032d971dd7 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -327,7 +327,8 @@ or `--mirror` is given)\n \tBefore fetching from the remote, fetch a bundle from the given\n \t`` and unbundle the data into the local repository. The refs\n \tin the bundle will be stored under the hidden `refs/bundle/*`\n-\tnamespace.\n+\tnamespace. This option is incompatible with `--depth`,\n+\t`--shallow-since`, and `--shallow-exclude`.\n \n :git-clone: 1\n include::urls.txt[]\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 4224d562758..4463789680b 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -937,6 +937,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_no_checkout = 1;\n \t}\n \n+\tif (bundle_uri && deepen)\n+\t\tdie(_(\"--bundle-uri is incompatible with --depth, --shallow-since, and --shallow-exclude\"));\n+\n \trepo_name = argv[0];\n \n \tpath = get_repo_path(repo_name, &is_bundle);\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 8f676d6b0c0..f6bb02ab947 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -58,6 +58,14 @@ test_expect_success 'disallows --bare with --separate-git-dir' '\n \n '\n \n+test_expect_success 'disallows --bundle-uri with shallow options' '\n+\tfor option in --depth=1 --shallow-since=01-01-2000 --shallow-exclude=HEAD\n+\tdo\n+\t\ttest_must_fail git clone --bundle-uri=bundle $option from to 2>err &&\n+\t\tgrep \"bundle-uri is incompatible\" err || return 1\n+\tdone\n+'\n+\n test_expect_success 'reject cloning shallow repository' '\n \ttest_when_finished \"rm -rf repo\" &&\n \ttest_must_fail git clone --reject-shallow shallow-repo out 2>err &&\n-- \ngitgitgadget\n"},{"id":"459918","messageId":"f6bc3177332e8608d0ba300669853bbad01c3266.1658781277.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.git.1658781277.gitgitgadget@gmail.com","subject":"[PATCH 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-25T20:34:36Z","receivedAt":"2022-07-25T20:35:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe previous change created the 'git clone --bundle-uri=' option.\nCurrently, must be a filename.\n\nUpdate copy_uri_to_file() to first inspect the URI for an HTTP(S) prefix\nand use git-remote-https as the way to download the data at that URI.\nOtherwise, check to see if file:// is present and modify the prefix\naccordingly.\n\nSigned-off-by: Derrick Stolee \n---\n bundle-uri.c | 70 +++++++++++++++++++++++++++++++++++--\n t/t5558-clone-bundle-uri.sh | 45 ++++++++++++++++++++++++\n 2 files changed, 112 insertions(+), 3 deletions(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex b35babc36aa..7086382b186 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -23,10 +23,74 @@ static int find_temp_filename(struct strbuf *name)\n \treturn 0;\n }\n \n-static int copy_uri_to_file(const char *file, const char *uri)\n+static int download_https_uri_to_file(const char *file, const char *uri)\n {\n-\t/* File-based URIs only for now. */\n-\treturn copy_file(file, uri, 0);\n+\tint result = 0;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tFILE *child_in = NULL, *child_out = NULL;\n+\tstruct strbuf line = STRBUF_INIT;\n+\tint found_get = 0;\n+\n+\tstrvec_pushl(&cp.args, \"git-remote-https\", \"origin\", uri, NULL);\n+\tcp.in = -1;\n+\tcp.out = -1;\n+\n+\tif (start_command(&cp))\n+\t\treturn 1;\n+\n+\tchild_in = fdopen(cp.in, \"w\");\n+\tif (!child_in) {\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tchild_out = fdopen(cp.out, \"r\");\n+\tif (!child_out) {\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tfprintf(child_in, \"capabilities\\n\");\n+\tfflush(child_in);\n+\n+\twhile (!strbuf_getline(&line, child_out)) {\n+\t\tif (!line.len)\n+\t\t\tbreak;\n+\t\tif (!strcmp(line.buf, \"get\"))\n+\t\t\tfound_get = 1;\n+\t}\n+\tstrbuf_release(&line);\n+\n+\tif (!found_get) {\n+\t\tresult = error(_(\"insufficient capabilities\"));\n+\t\tgoto cleanup;\n+\t}\n+\n+\tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n+\n+cleanup:\n+\tif (child_in)\n+\t\tfclose(child_in);\n+\tif (finish_command(&cp))\n+\t\treturn 1;\n+\tif (child_out)\n+\t\tfclose(child_out);\n+\treturn result;\n+}\n+\n+static int copy_uri_to_file(const char *filename, const char *uri)\n+{\n+\tconst char *out;\n+\n+\tif (skip_prefix(uri, \"https:\", &out) ||\n+\t skip_prefix(uri, \"http:\", &out))\n+\t\treturn download_https_uri_to_file(filename, uri);\n+\n+\tif (!skip_prefix(uri, \"file://\", &out))\n+\t\tout = uri;\n+\n+\t/* Copy as a file */\n+\treturn copy_file(filename, out, 0);\n }\n \n static int unbundle_from_file(struct repository *r, const char *file)\ndiff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh\nindex f709bcb729c..ad666a2d28a 100755\n--- a/t/t5558-clone-bundle-uri.sh\n+++ b/t/t5558-clone-bundle-uri.sh\n@@ -33,4 +33,49 @@ test_expect_success 'clone with path bundle' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'clone with file:// bundle' '\n+\tgit clone --bundle-uri=\"file://$(pwd)/clone-from/B.bundle\" \\\n+\t\tclone-from clone-file &&\n+\tgit -C clone-file rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+#########################################################################\n+# HTTP tests begin here\n+\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'fail to fetch from non-existent HTTP URL' '\n+\ttest_when_finished rm -rf test &&\n+\tgit clone --bundle-uri=\"$HTTPD_URL/does-not-exist\" . test 2>err &&\n+\tgrep \"failed to download bundle from URI\" err\n+'\n+\n+test_expect_success 'fail to fetch from non-bundle HTTP URL' '\n+\ttest_when_finished rm -rf test &&\n+\techo bogus >\"$HTTPD_DOCUMENT_ROOT_PATH/bogus\" &&\n+\tgit clone --bundle-uri=\"$HTTPD_URL/bogus\" . test 2>err &&\n+\tgrep \"is not a bundle\" err\n+'\n+\n+test_expect_success 'clone HTTP bundle' '\n+\tcp clone-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n+\n+\tgit clone --no-local --mirror clone-from \\\n+\t\t\"$HTTPD_DOCUMENT_ROOT_PATH/fetch.git\" &&\n+\n+\tgit clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n+\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http &&\n+\tgit -C clone-http rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_config -C clone-http log.excludedecoration refs/bundle/\n+'\n+\n+# Do not add tests here unless they use the HTTP server, as they will\n+# not run unless the HTTP dependencies exist.\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"460040","messageId":"YuG3cuFEvhL+wKs/@google.com","threadId":"58220","inReplyTo":"40808e92afb7bcf3e8e9b4b53d5e30b5e17816f8.1658781277.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/5] remote-curl: add 'get' capability","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2022-07-27T22:08:50Z","receivedAt":"2022-07-27T22:09:28Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2022.07.25 20:34, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> A future change will want a way to download a file over HTTP(S) using\n> the simplest of download mechanisms. We do not want to assume that the\n> server on the other side understands anything about the Git protocol but\n> could be a simple static web server.\n> \n> Create the new 'get' capability for the remote helpers which advertises\n> that the 'get' command is avalable. A caller can send a line containing\n> 'get ' to download the file at into the file at\n> .\n> \n> Signed-off-by: Derrick Stolee \n> ---\n> Documentation/gitremote-helpers.txt | 9 +++++++\n> remote-curl.c | 32 +++++++++++++++++++++++++\n> t/t5557-http-get.sh | 37 +++++++++++++++++++++++++++++\n> 3 files changed, 78 insertions(+)\n> create mode 100755 t/t5557-http-get.sh\n> \n> diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\n> index 6f1e269ae43..ed8da428c98 100644\n> --- a/Documentation/gitremote-helpers.txt\n> +++ b/Documentation/gitremote-helpers.txt\n> @@ -168,6 +168,9 @@ Supported commands: 'list', 'import'.\n> \tCan guarantee that when a clone is requested, the received\n> \tpack is self contained and is connected.\n> \n> +'get'::\n> +\tCan use the 'get' command to download a file from a given URI.\n> +\n> If a helper advertises 'connect', Git will use it if possible and\n> fall back to another capability if the helper requests so when\n> connecting (see the 'connect' command under COMMANDS).\n> @@ -418,6 +421,12 @@ Supported if the helper has the \"connect\" capability.\n> +\n> Supported if the helper has the \"stateless-connect\" capability.\n> \n> +'get' ::\n> +\tDownloads the file from the given `` to the given ``. If\n> +\t`.temp` exists, then Git assumes that the `.temp` file is a\n> +\tpartial download from a previous attempt and will resume the\n> +\tdownload from that position.\n> +\n> If a fatal error occurs, the program writes the error message to\n> stderr and exits. The caller should expect that a suitable error\n> message has been printed if the child closes the connection without\n> diff --git a/remote-curl.c b/remote-curl.c\n> index b8758757ece..73fbdbddd84 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -1286,6 +1286,33 @@ static void parse_fetch(struct strbuf *buf)\n> \tstrbuf_reset(buf);\n> }\n> \n> +static void parse_get(struct strbuf *buf)\n> +{\n> +\tstruct strbuf url = STRBUF_INIT;\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tconst char *p, *space;\n> +\n> +\tif (!skip_prefix(buf->buf, \"get \", &p))\n> +\t\tdie(_(\"http transport does not support %s\"), buf->buf);\n\nNit: since we're already calling skip_prefix(...) in cmd_main() below,\ncan we just pass the suffix to parse_get() and avoid having to skip the\nprefix twice?\n\n\n> +\n> +\tspace = strchr(p, ' ');\n> +\n> +\tif (!space)\n> +\t\tdie(_(\"protocol error: expected ' ', missing space\"));\n> +\n> +\tstrbuf_add(&url, p, space - p);\n> +\tstrbuf_addstr(&path, space + 1);\n> +\n> +\tif (http_get_file(url.buf, path.buf, NULL))\n> +\t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n> +\n> +\tstrbuf_release(&url);\n> +\tstrbuf_release(&path);\n> +\tprintf(\"\\n\");\n> +\tfflush(stdout);\n> +\tstrbuf_reset(buf);\n> +}\n> +\n> static int push_dav(int nr_spec, const char **specs)\n> {\n> \tstruct child_process child = CHILD_PROCESS_INIT;\n> @@ -1564,9 +1591,14 @@ int cmd_main(int argc, const char **argv)\n> \t\t\t\tprintf(\"unsupported\\n\");\n> \t\t\tfflush(stdout);\n> \n> +\t\t} else if (skip_prefix(buf.buf, \"get \", &arg)) {\n> +\t\t\tparse_get(&buf);\n> +\t\t\tfflush(stdout);\n> +\n> \t\t} else if (!strcmp(buf.buf, \"capabilities\")) {\n> \t\t\tprintf(\"stateless-connect\\n\");\n> \t\t\tprintf(\"fetch\\n\");\n> +\t\t\tprintf(\"get\\n\");\n> \t\t\tprintf(\"option\\n\");\n> \t\t\tprintf(\"push\\n\");\n> \t\t\tprintf(\"check-connectivity\\n\");\n> diff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\n> new file mode 100755\n> index 00000000000..50b7dbcf957\n> --- /dev/null\n> +++ b/t/t5557-http-get.sh\n> @@ -0,0 +1,37 @@\n> +#!/bin/sh\n> +\n> +test_description='test downloading a file by URL'\n> +\n> +. ./test-lib.sh\n> +\n> +. \"$TEST_DIRECTORY\"/lib-httpd.sh\n> +start_httpd\n> +\n> +test_expect_success 'get by URL: 404' '\n> +\turl=\"$HTTPD_URL/none.txt\" &&\n> +\tcat >input <<-EOF &&\n> +\tcapabilities\n> +\tget $url file1\n> +\tEOF\n> +\n> +\ttest_must_fail git remote-http $url err &&\n> +\ttest_path_is_missing file1 &&\n> +\tgrep \"failed to download file at URL\" err &&\n> +\trm file1.temp\n> +'\n> +\n> +test_expect_success 'get by URL: 200' '\n> +\techo data >\"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" &&\n> +\n> +\turl=\"$HTTPD_URL/exists.txt\" &&\n> +\tcat >input <<-EOF &&\n> +\tcapabilities\n> +\tget $url file2\n> +\n> +\tEOF\n> +\n> +\tGIT_TRACE2_PERF=1 git remote-http $url +\ttest_cmp \"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" file2\n> +'\n> +\n> +test_done\n> -- \n> gitgitgadget\n> \n"},{"id":"460041","messageId":"YuG3h4AcZjyILvdK@google.com","threadId":"58220","inReplyTo":"7d3159f0d9a29a04b5856982f787f4eddfb91b53.1658781277.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/5] bundle-uri: create basic file-copy logic","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2022-07-27T22:09:11Z","receivedAt":"2022-07-27T22:10:02Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2022.07.25 20:34, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> Before implementing a way to fetch bundles into a repository, create the\n> basic logic. Assume that the URI is actually a file path. Future logic\n> will make this more careful to other protocols.\n> \n> For now, we also only succeed if the content at the URI is a bundle\n> file, not a bundle list. Bundle lists will be implemented in a future\n> change.\n> \n> Note that the discovery of a temporary filename is slightly racy because\n> the odb_mkstemp() relies on the temporary file not existing. With the\n> current implementation being limited to file copies, we could replace\n> the copy_file() with copy_fd(). The tricky part comes in future changes\n> that send the filename to 'git remote-https' and its 'get' capability.\n\n> At that point, we need the file descriptor closed _and_ the file\n> unlinked.\n\nAhh, it looks like this was the point I missed in my previous review.\nIIUC, we need the file unlinked because http_get_file() will eventually\ncall finalize_object_file() to move a tempfile to the final object name,\nand that will fail if we have an empty file already in place.\n\n\n> If we were to keep the file descriptor open for the sake of\n> normal file copies, then we would pollute the rest of the code for\n> little benefit. This is especially the case because we expect that most\n> bundle URI use will be based on HTTPS instead of file copies.\n> \n> Signed-off-by: Derrick Stolee \n> ---\n> Makefile | 1 +\n> bundle-uri.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++\n> bundle-uri.h | 14 +++++++\n> 3 files changed, 119 insertions(+)\n> create mode 100644 bundle-uri.c\n> create mode 100644 bundle-uri.h\n> \n> diff --git a/Makefile b/Makefile\n> index 1624471badc..7d5f48069ea 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -906,6 +906,7 @@ LIB_OBJS += blob.o\n> LIB_OBJS += bloom.o\n> LIB_OBJS += branch.o\n> LIB_OBJS += bulk-checkin.o\n> +LIB_OBJS += bundle-uri.o\n> LIB_OBJS += bundle.o\n> LIB_OBJS += cache-tree.o\n> LIB_OBJS += cbtree.o\n> diff --git a/bundle-uri.c b/bundle-uri.c\n> new file mode 100644\n> index 00000000000..b35babc36aa\n> --- /dev/null\n> +++ b/bundle-uri.c\n> @@ -0,0 +1,104 @@\n> +#include \"cache.h\"\n> +#include \"bundle-uri.h\"\n> +#include \"bundle.h\"\n> +#include \"object-store.h\"\n> +#include \"refs.h\"\n> +#include \"run-command.h\"\n> +\n> +static int find_temp_filename(struct strbuf *name)\n> +{\n> +\tint fd;\n> +\t/*\n> +\t * Find a temporary filename that is available. This is briefly\n> +\t * racy, but unlikely to collide.\n> +\t */\n> +\tfd = odb_mkstemp(name, \"bundles/tmp_uri_XXXXXX\");\n> +\tif (fd < 0) {\n> +\t\twarning(_(\"failed to create temporary file\"));\n> +\t\treturn -1;\n> +\t}\n> +\n> +\tclose(fd);\n> +\tunlink(name->buf);\n> +\treturn 0;\n> +}\n> +\n> +static int copy_uri_to_file(const char *file, const char *uri)\n> +{\n> +\t/* File-based URIs only for now. */\n> +\treturn copy_file(file, uri, 0);\n> +}\n> +\n> +static int unbundle_from_file(struct repository *r, const char *file)\n> +{\n> +\tint result = 0;\n> +\tint bundle_fd;\n> +\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n> +\tstruct string_list_item *refname;\n> +\tstruct strbuf bundle_ref = STRBUF_INIT;\n> +\tsize_t bundle_prefix_len;\n> +\n> +\tif ((bundle_fd = read_bundle_header(file, &header)) < 0)\n> +\t\treturn 1;\n> +\n> +\tif ((result = unbundle(r, &header, bundle_fd, NULL)))\n> +\t\treturn 1;\n> +\n> +\t/*\n> +\t * Convert all refs/heads/ from the bundle into refs/bundles/\n> +\t * in the local repository.\n> +\t */\n> +\tstrbuf_addstr(&bundle_ref, \"refs/bundles/\");\n> +\tbundle_prefix_len = bundle_ref.len;\n> +\n> +\tfor_each_string_list_item(refname, &header.references) {\n> +\t\tstruct object_id *oid = refname->util;\n> +\t\tstruct object_id old_oid;\n> +\t\tconst char *branch_name;\n> +\t\tint has_old;\n> +\n> +\t\tif (!skip_prefix(refname->string, \"refs/heads/\", &branch_name))\n> +\t\t\tcontinue;\n> +\n> +\t\tstrbuf_setlen(&bundle_ref, bundle_prefix_len);\n> +\t\tstrbuf_addstr(&bundle_ref, branch_name);\n> +\n> +\t\thas_old = !read_ref(bundle_ref.buf, &old_oid);\n> +\t\tupdate_ref(\"fetched bundle\", bundle_ref.buf, oid,\n> +\t\t\t has_old ? &old_oid : NULL,\n> +\t\t\t REF_SKIP_OID_VERIFICATION,\n> +\t\t\t UPDATE_REFS_MSG_ON_ERR);\n> +\t}\n> +\n> +\tbundle_header_release(&header);\n\nWe still also need to release bundle_ref here, right?\n\n\n> +\treturn result;\n> +}\n> +\n> +int fetch_bundle_uri(struct repository *r, const char *uri)\n> +{\n> +\tint result = 0;\n> +\tstruct strbuf filename = STRBUF_INIT;\n> +\n> +\tif ((result = find_temp_filename(&filename)))\n> +\t\tgoto cleanup;\n> +\n> +\tif ((result = copy_uri_to_file(filename.buf, uri))) {\n> +\t\twarning(_(\"failed to download bundle from URI '%s'\"), uri);\n> +\t\tgoto cleanup;\n> +\t}\n> +\n> +\tif ((result = !is_bundle(filename.buf, 0))) {\n> +\t\twarning(_(\"file at URI '%s' is not a bundle\"), uri);\n> +\t\tgoto cleanup;\n> +\t}\n> +\n> +\tif ((result = unbundle_from_file(r, filename.buf))) {\n> +\t\twarning(_(\"failed to unbundle bundle from URI '%s'\"), uri);\n> +\t\tgoto cleanup;\n> +\t}\n> +\n> +cleanup:\n> +\tunlink(filename.buf);\n> +\tstrbuf_release(&filename);\n> +\treturn result;\n> +}\n> diff --git a/bundle-uri.h b/bundle-uri.h\n> new file mode 100644\n> index 00000000000..8a152f1ef14\n> --- /dev/null\n> +++ b/bundle-uri.h\n> @@ -0,0 +1,14 @@\n> +#ifndef BUNDLE_URI_H\n> +#define BUNDLE_URI_H\n> +\n> +struct repository;\n> +\n> +/**\n> + * Fetch data from the given 'uri' and unbundle the bundle data found\n> + * based on that information.\n> + *\n> + * Returns non-zero if no bundle information is found at the given 'uri'.\n> + */\n> +int fetch_bundle_uri(struct repository *r, const char *uri);\n> +\n> +#endif\n> -- \n> gitgitgadget\n> \n"},{"id":"460042","messageId":"YuG3miyCKtnv3Na/@google.com","threadId":"58220","inReplyTo":"f6bc3177332e8608d0ba300669853bbad01c3266.1658781277.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2022-07-27T22:09:30Z","receivedAt":"2022-07-27T22:11:34Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2022.07.25 20:34, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> The previous change created the 'git clone --bundle-uri=' option.\n> Currently, must be a filename.\n> \n> Update copy_uri_to_file() to first inspect the URI for an HTTP(S) prefix\n> and use git-remote-https as the way to download the data at that URI.\n> Otherwise, check to see if file:// is present and modify the prefix\n> accordingly.\n> \n> Signed-off-by: Derrick Stolee \n> ---\n> bundle-uri.c | 70 +++++++++++++++++++++++++++++++++++--\n> t/t5558-clone-bundle-uri.sh | 45 ++++++++++++++++++++++++\n> 2 files changed, 112 insertions(+), 3 deletions(-)\n> \n> diff --git a/bundle-uri.c b/bundle-uri.c\n> index b35babc36aa..7086382b186 100644\n> --- a/bundle-uri.c\n> +++ b/bundle-uri.c\n> @@ -23,10 +23,74 @@ static int find_temp_filename(struct strbuf *name)\n> \treturn 0;\n> }\n> \n> -static int copy_uri_to_file(const char *file, const char *uri)\n> +static int download_https_uri_to_file(const char *file, const char *uri)\n> {\n> -\t/* File-based URIs only for now. */\n> -\treturn copy_file(file, uri, 0);\n> +\tint result = 0;\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tFILE *child_in = NULL, *child_out = NULL;\n> +\tstruct strbuf line = STRBUF_INIT;\n> +\tint found_get = 0;\n> +\n> +\tstrvec_pushl(&cp.args, \"git-remote-https\", \"origin\", uri, NULL);\n> +\tcp.in = -1;\n> +\tcp.out = -1;\n> +\n> +\tif (start_command(&cp))\n> +\t\treturn 1;\n> +\n> +\tchild_in = fdopen(cp.in, \"w\");\n> +\tif (!child_in) {\n> +\t\tresult = 1;\n> +\t\tgoto cleanup;\n> +\t}\n> +\n> +\tchild_out = fdopen(cp.out, \"r\");\n> +\tif (!child_out) {\n> +\t\tresult = 1;\n> +\t\tgoto cleanup;\n> +\t}\n> +\n> +\tfprintf(child_in, \"capabilities\\n\");\n> +\tfflush(child_in);\n> +\n> +\twhile (!strbuf_getline(&line, child_out)) {\n> +\t\tif (!line.len)\n> +\t\t\tbreak;\n> +\t\tif (!strcmp(line.buf, \"get\"))\n> +\t\t\tfound_get = 1;\n> +\t}\n> +\tstrbuf_release(&line);\n> +\n> +\tif (!found_get) {\n> +\t\tresult = error(_(\"insufficient capabilities\"));\n> +\t\tgoto cleanup;\n> +\t}\n> +\n> +\tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n> +\n> +cleanup:\n> +\tif (child_in)\n> +\t\tfclose(child_in);\n> +\tif (finish_command(&cp))\n> +\t\treturn 1;\n> +\tif (child_out)\n> +\t\tfclose(child_out);\n> +\treturn result;\n> +}\n> +\n> +static int copy_uri_to_file(const char *filename, const char *uri)\n> +{\n> +\tconst char *out;\n> +\n> +\tif (skip_prefix(uri, \"https:\", &out) ||\n> +\t skip_prefix(uri, \"http:\", &out))\n> +\t\treturn download_https_uri_to_file(filename, uri);\n\nSince we're not using \"out\" here, should we replace the skip_prefix()s\nwith starts_with(), or perhaps even istarts_with()?\n\n\n> +\tif (!skip_prefix(uri, \"file://\", &out))\n> +\t\tout = uri;\n> +\n> +\t/* Copy as a file */\n> +\treturn copy_file(filename, out, 0);\n> }\n> \n> static int unbundle_from_file(struct repository *r, const char *file)\n> diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh\n> index f709bcb729c..ad666a2d28a 100755\n> --- a/t/t5558-clone-bundle-uri.sh\n> +++ b/t/t5558-clone-bundle-uri.sh\n> @@ -33,4 +33,49 @@ test_expect_success 'clone with path bundle' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'clone with file:// bundle' '\n> +\tgit clone --bundle-uri=\"file://$(pwd)/clone-from/B.bundle\" \\\n> +\t\tclone-from clone-file &&\n> +\tgit -C clone-file rev-parse refs/bundles/topic >actual &&\n> +\tgit -C clone-from rev-parse topic >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +#########################################################################\n> +# HTTP tests begin here\n> +\n> +. \"$TEST_DIRECTORY\"/lib-httpd.sh\n> +start_httpd\n> +\n> +test_expect_success 'fail to fetch from non-existent HTTP URL' '\n> +\ttest_when_finished rm -rf test &&\n> +\tgit clone --bundle-uri=\"$HTTPD_URL/does-not-exist\" . test 2>err &&\n> +\tgrep \"failed to download bundle from URI\" err\n> +'\n> +\n> +test_expect_success 'fail to fetch from non-bundle HTTP URL' '\n> +\ttest_when_finished rm -rf test &&\n> +\techo bogus >\"$HTTPD_DOCUMENT_ROOT_PATH/bogus\" &&\n> +\tgit clone --bundle-uri=\"$HTTPD_URL/bogus\" . test 2>err &&\n> +\tgrep \"is not a bundle\" err\n> +'\n> +\n> +test_expect_success 'clone HTTP bundle' '\n> +\tcp clone-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n> +\n> +\tgit clone --no-local --mirror clone-from \\\n> +\t\t\"$HTTPD_DOCUMENT_ROOT_PATH/fetch.git\" &&\n> +\n> +\tgit clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n> +\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http &&\n> +\tgit -C clone-http rev-parse refs/bundles/topic >actual &&\n> +\tgit -C clone-from rev-parse topic >expect &&\n> +\ttest_cmp expect actual &&\n> +\n> +\ttest_config -C clone-http log.excludedecoration refs/bundle/\n> +'\n> +\n> +# Do not add tests here unless they use the HTTP server, as they will\n> +# not run unless the HTTP dependencies exist.\n> +\n> test_done\n> -- \n> gitgitgadget\n> \n"},{"id":"460043","messageId":"YuG4ICBFNbLPR9Iv@google.com","threadId":"58220","inReplyTo":"pull.1300.git.1658781277.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] Bundle URIs II: git clone --bundle-uri","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2022-07-27T22:11:44Z","receivedAt":"2022-07-27T22:15:12Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2022.07.25 20:34, Derrick Stolee via GitGitGadget wrote:\n> This is the second series building the bundle URI feature as discussed in\n> the previous series that added the design document [1]. This series does not\n> modify the design document, so the patches are independent and can be\n> applied to the latest 'master'.\n> \n> [1]\n> https://lore.kernel.org/git/pull.1248.v3.git.1658757188.gitgitgadget@gmail.com\n> \n> This series brings in just enough logic that we can bootstrap clones from a\n> single bundle using git clone --bundle-uri=.\n> \n> * Patch 1 adds a 'get' capability to 'git remote-https' which allows\n> downloading the contents of a URI to a local file.\n> * Patch 2 creates basic file-copy logic within a new bundle-uri.c file. It\n> is not used until patch 3.\n> * Patch 3 creates the git clone --bundle-uri= option, allowing Git to\n> bootstrap a clone from a bundle, but get the remaining objects from the\n> origin URL. (As of this patch, it only accepts a filename.)\n> * Patch 4 extends the git clone --bundle-uri= option to allow file://\n> and https:// URIs.\n> * Patch 5 is a CLI helper to avoid using --bundle-uri and --depth at the\n> same time in git clone.\n> \n> As outlined in [1], the next steps after this are:\n> \n> 1. Allow parsing a bundle list as a config file at the given URI. The\n> key-value format is unified with the protocol v2 verb (coming in (3)).\n> [2]\n> 2. Implement the protocol v2 verb, re-using the bundle list logic from (2).\n> Use this to auto-discover bundle URIs during 'git clone' (behind a\n> config option). [3]\n> 3. Implement the 'creationToken' heuristic, allowing incremental 'git\n> fetch' commands to download a bundle list from a configured URI, and\n> only download bundles that are new based on the creation token values.\n> [4]\n> \n> I have prepared some of this work as pull requests on my personal fork so\n> curious readers can look ahead to where we are going:\n> \n> [2] https://github.com/derrickstolee/git/pull/20\n> \n> [3] https://github.com/derrickstolee/git/pull/21\n> \n> [4] https://github.com/derrickstolee/git/pull/22\n> \n> Note: this series includes some code pulled out of the first series [1], and\n> in particular the git fetch --bundle-uri= option is removed. The\n> intention was to replace that with git bundle fetch , but that conflicts\n> with the work to refactor how subcommands are parsed. The git bundle fetch\n> subcommand could be added later for maximum flexibility on the client side,\n> but we can also move forward without it.\n> \n> Thanks, -Stolee\n> \n> P.S. Here is the range-diff compared to v2 of the previous bundle URI\n> series:\n> \n> 1: d444042dc4dcc < -: ------------- docs: document bundle URI standard\n> 2: 0a2cf60437f78 ! 1: 40808e92afb7b remote-curl: add 'get' capability\n> @@ remote-curl.c: static void parse_fetch(struct strbuf *buf)\n> \n> +static void parse_get(struct strbuf *buf)\n> +{\n> -+\tstruct http_get_options opts = { 0 };\n> +\tstruct strbuf url = STRBUF_INIT;\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tconst char *p, *space;\n> @@ remote-curl.c: static void parse_fetch(struct strbuf *buf)\n> +\tstrbuf_add(&url, p, space - p);\n> +\tstrbuf_addstr(&path, space + 1);\n> +\n> -+\tif (http_get_file(url.buf, path.buf, &opts))\n> ++\tif (http_get_file(url.buf, path.buf, NULL))\n> +\t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n> +\n> +\tstrbuf_release(&url);\n> @@ t/t5557-http-get.sh (new)\n> +\tget $url file1\n> +\tEOF\n> +\n> -+\ttest_must_fail git remote-http $url $url err &&\n> ++\ttest_must_fail git remote-http $url err &&\n> +\ttest_path_is_missing file1 &&\n> +\tgrep \"failed to download file at URL\" err &&\n> +\trm file1.temp\n> @@ t/t5557-http-get.sh (new)\n> +\n> +\tEOF\n> +\n> -+\tGIT_TRACE2_PERF=1 git remote-http $url $url ++\tGIT_TRACE2_PERF=1 git remote-http $url +\ttest_cmp \"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" file2\n> +'\n> +\n> +test_done\n> -\n> - ## transport-helper.c ##\n> -@@ transport-helper.c: struct helper_data {\n> - \t\tcheck_connectivity : 1,\n> - \t\tno_disconnect_req : 1,\n> - \t\tno_private_update : 1,\n> --\t\tobject_format : 1;\n> -+\t\tobject_format : 1,\n> -+\t\tget : 1;\n> - \n> - \t/*\n> - \t * As an optimization, the transport code may invoke fetch before\n> -@@ transport-helper.c: static struct child_process *get_helper(struct transport *transport)\n> - \t\t\tdata->no_private_update = 1;\n> - \t\t} else if (starts_with(capname, \"object-format\")) {\n> - \t\t\tdata->object_format = 1;\n> -+\t\t} else if (!strcmp(capname, \"get\")) {\n> -+\t\t\tdata->get = 1;\n> - \t\t} else if (mandatory) {\n> - \t\t\tdie(_(\"unknown mandatory capability %s; this remote \"\n> - \t\t\t \"helper probably needs newer version of Git\"),\n> 3: abec47564fd9c ! 2: 7d3159f0d9a29 bundle-uri: create basic file-copy logic\n> @@ Commit message\n> file, not a bundle list. Bundle lists will be implemented in a future\n> change.\n> \n> + Note that the discovery of a temporary filename is slightly racy because\n> + the odb_mkstemp() relies on the temporary file not existing. With the\n> + current implementation being limited to file copies, we could replace\n> + the copy_file() with copy_fd(). The tricky part comes in future changes\n> + that send the filename to 'git remote-https' and its 'get' capability.\n> + At that point, we need the file descriptor closed _and_ the file\n> + unlinked. If we were to keep the file descriptor open for the sake of\n> + normal file copies, then we would pollute the rest of the code for\n> + little benefit. This is especially the case because we expect that most\n> + bundle URI use will be based on HTTPS instead of file copies.\n> +\n> Signed-off-by: Derrick Stolee \n> \n> ## Makefile ##\n> @@ bundle-uri.c (new)\n> +#include \"refs.h\"\n> +#include \"run-command.h\"\n> +\n> -+static void find_temp_filename(struct strbuf *name)\n> ++static int find_temp_filename(struct strbuf *name)\n> +{\n> +\tint fd;\n> +\t/*\n> @@ bundle-uri.c (new)\n> +\t * racy, but unlikely to collide.\n> +\t */\n> +\tfd = odb_mkstemp(name, \"bundles/tmp_uri_XXXXXX\");\n> -+\tif (fd < 0)\n> -+\t\tdie(_(\"failed to create temporary file\"));\n> ++\tif (fd < 0) {\n> ++\t\twarning(_(\"failed to create temporary file\"));\n> ++\t\treturn -1;\n> ++\t}\n> ++\n> +\tclose(fd);\n> +\tunlink(name->buf);\n> ++\treturn 0;\n> +}\n> +\n> -+static int copy_uri_to_file(const char *uri, const char *file)\n> ++static int copy_uri_to_file(const char *file, const char *uri)\n> +{\n> -+\t/* Copy as a file */\n> -+\treturn copy_file(file, uri, 0444);\n> ++\t/* File-based URIs only for now. */\n> ++\treturn copy_file(file, uri, 0);\n> +}\n> +\n> +static int unbundle_from_file(struct repository *r, const char *file)\n> @@ bundle-uri.c (new)\n> +\tint result = 0;\n> +\tint bundle_fd;\n> +\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n> -+\tstruct strvec extra_index_pack_args = STRVEC_INIT;\n> +\tstruct string_list_item *refname;\n> +\tstruct strbuf bundle_ref = STRBUF_INIT;\n> +\tsize_t bundle_prefix_len;\n> @@ bundle-uri.c (new)\n> +\tif ((bundle_fd = read_bundle_header(file, &header)) < 0)\n> +\t\treturn 1;\n> +\n> -+\tif ((result = unbundle(r, &header, bundle_fd, &extra_index_pack_args)))\n> ++\tif ((result = unbundle(r, &header, bundle_fd, NULL)))\n> +\t\treturn 1;\n> +\n> +\t/*\n> @@ bundle-uri.c (new)\n> +\tint result = 0;\n> +\tstruct strbuf filename = STRBUF_INIT;\n> +\n> -+\tfind_temp_filename(&filename);\n> -+\tif ((result = copy_uri_to_file(uri, filename.buf)))\n> ++\tif ((result = find_temp_filename(&filename)))\n> ++\t\tgoto cleanup;\n> ++\n> ++\tif ((result = copy_uri_to_file(filename.buf, uri))) {\n> ++\t\twarning(_(\"failed to download bundle from URI '%s'\"), uri);\n> +\t\tgoto cleanup;\n> ++\t}\n> +\n> -+\tif ((result = !is_bundle(filename.buf, 0)))\n> ++\tif ((result = !is_bundle(filename.buf, 0))) {\n> ++\t\twarning(_(\"file at URI '%s' is not a bundle\"), uri);\n> +\t\tgoto cleanup;\n> ++\t}\n> +\n> -+\tif ((result = unbundle_from_file(r, filename.buf)))\n> ++\tif ((result = unbundle_from_file(r, filename.buf))) {\n> ++\t\twarning(_(\"failed to unbundle bundle from URI '%s'\"), uri);\n> +\t\tgoto cleanup;\n> ++\t}\n> +\n> +cleanup:\n> +\tunlink(filename.buf);\n> 4: f6255ec518857 < -: ------------- fetch: add --bundle-uri option\n> -: ------------- > 3: 29e645a54ba7f clone: add --bundle-uri option\n> 5: bfbd11b48bf1b ! 4: f6bc3177332e8 bundle-uri: add support for http(s):// and file://\n> @@ Metadata\n> ## Commit message ##\n> bundle-uri: add support for http(s):// and file://\n> \n> - The previous change created the 'git fetch --bundle-uri=' option.\n> + The previous change created the 'git clone --bundle-uri=' option.\n> Currently, must be a filename.\n> \n> Update copy_uri_to_file() to first inspect the URI for an HTTP(S) prefix\n> @@ Commit message\n> Signed-off-by: Derrick Stolee \n> \n> ## bundle-uri.c ##\n> -@@ bundle-uri.c: static void find_temp_filename(struct strbuf *name)\n> - \tunlink(name->buf);\n> +@@ bundle-uri.c: static int find_temp_filename(struct strbuf *name)\n> + \treturn 0;\n> }\n> \n> -+static int download_https_uri_to_file(const char *uri, const char *file)\n> -+{\n> +-static int copy_uri_to_file(const char *file, const char *uri)\n> ++static int download_https_uri_to_file(const char *file, const char *uri)\n> + {\n> +-\t/* File-based URIs only for now. */\n> +-\treturn copy_file(file, uri, 0);\n> +\tint result = 0;\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tFILE *child_in = NULL, *child_out = NULL;\n> @@ bundle-uri.c: static void find_temp_filename(struct strbuf *name)\n> +\treturn result;\n> +}\n> +\n> - static int copy_uri_to_file(const char *uri, const char *file)\n> - {\n> ++static int copy_uri_to_file(const char *filename, const char *uri)\n> ++{\n> +\tconst char *out;\n> ++\n> +\tif (skip_prefix(uri, \"https:\", &out) ||\n> +\t skip_prefix(uri, \"http:\", &out))\n> -+\t\treturn download_https_uri_to_file(uri, file);\n> ++\t\treturn download_https_uri_to_file(filename, uri);\n> +\n> +\tif (!skip_prefix(uri, \"file://\", &out))\n> +\t\tout = uri;\n> +\n> - \t/* Copy as a file */\n> --\treturn copy_file(file, uri, 0444);\n> -+\treturn !!copy_file(file, out, 0);\n> ++\t/* Copy as a file */\n> ++\treturn copy_file(filename, out, 0);\n> }\n> \n> static int unbundle_from_file(struct repository *r, const char *file)\n> \n> - ## t/t5558-fetch-bundle-uri.sh ##\n> -@@ t/t5558-fetch-bundle-uri.sh: test_expect_success 'fetch file bundle' '\n> + ## t/t5558-clone-bundle-uri.sh ##\n> +@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone with path bundle' '\n> test_cmp expect actual\n> '\n> \n> -+test_expect_success 'fetch file:// bundle' '\n> -+\tgit init fetch-file &&\n> -+\tgit -C fetch-file fetch --bundle-uri=\"file://$(pwd)/fetch-from/B.bundle\" &&\n> -+\tgit -C fetch-file rev-parse refs/bundles/topic >actual &&\n> -+\tgit -C fetch-from rev-parse topic >expect &&\n> ++test_expect_success 'clone with file:// bundle' '\n> ++\tgit clone --bundle-uri=\"file://$(pwd)/clone-from/B.bundle\" \\\n> ++\t\tclone-from clone-file &&\n> ++\tgit -C clone-file rev-parse refs/bundles/topic >actual &&\n> ++\tgit -C clone-from rev-parse topic >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> @@ t/t5558-fetch-bundle-uri.sh: test_expect_success 'fetch file bundle' '\n> +start_httpd\n> +\n> +test_expect_success 'fail to fetch from non-existent HTTP URL' '\n> -+\ttest_must_fail git fetch --bundle-uri=\"$HTTPD_URL/does-not-exist\" 2>err &&\n> ++\ttest_when_finished rm -rf test &&\n> ++\tgit clone --bundle-uri=\"$HTTPD_URL/does-not-exist\" . test 2>err &&\n> +\tgrep \"failed to download bundle from URI\" err\n> +'\n> +\n> +test_expect_success 'fail to fetch from non-bundle HTTP URL' '\n> ++\ttest_when_finished rm -rf test &&\n> +\techo bogus >\"$HTTPD_DOCUMENT_ROOT_PATH/bogus\" &&\n> -+\ttest_must_fail git fetch --bundle-uri=\"$HTTPD_URL/bogus\" 2>err &&\n> ++\tgit clone --bundle-uri=\"$HTTPD_URL/bogus\" . test 2>err &&\n> +\tgrep \"is not a bundle\" err\n> +'\n> +\n> -+test_expect_success 'fetch HTTP bundle' '\n> -+\tcp fetch-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n> -+\tgit init fetch-http &&\n> -+\tgit -C fetch-http fetch --bundle-uri=\"$HTTPD_URL/B.bundle\" &&\n> -+\tgit -C fetch-http rev-parse refs/bundles/topic >actual &&\n> -+\tgit -C fetch-from rev-parse topic >expect &&\n> -+\ttest_cmp expect actual\n> ++test_expect_success 'clone HTTP bundle' '\n> ++\tcp clone-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n> ++\n> ++\tgit clone --no-local --mirror clone-from \\\n> ++\t\t\"$HTTPD_DOCUMENT_ROOT_PATH/fetch.git\" &&\n> ++\n> ++\tgit clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n> ++\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http &&\n> ++\tgit -C clone-http rev-parse refs/bundles/topic >actual &&\n> ++\tgit -C clone-from rev-parse topic >expect &&\n> ++\ttest_cmp expect actual &&\n> ++\n> ++\ttest_config -C clone-http log.excludedecoration refs/bundle/\n> +'\n> +\n> +# Do not add tests here unless they use the HTTP server, as they will\n> 6: a217e9a0640b4 < -: ------------- fetch: add 'refs/bundle/' to log.excludeDecoration\n> -: ------------- > 5: e823b168ab725 clone: --bundle-uri cannot be combined with --depth\n> \n> \n> Derrick Stolee (5):\n> remote-curl: add 'get' capability\n> bundle-uri: create basic file-copy logic\n> clone: add --bundle-uri option\n> bundle-uri: add support for http(s):// and file://\n> clone: --bundle-uri cannot be combined with --depth\n> \n> Documentation/git-clone.txt | 7 ++\n> Documentation/gitremote-helpers.txt | 9 ++\n> Makefile | 1 +\n> builtin/clone.c | 18 +++\n> bundle-uri.c | 168 ++++++++++++++++++++++++++++\n> bundle-uri.h | 14 +++\n> remote-curl.c | 32 ++++++\n> t/t5557-http-get.sh | 37 ++++++\n> t/t5558-clone-bundle-uri.sh | 81 ++++++++++++++\n> t/t5606-clone-options.sh | 8 ++\n> 10 files changed, 375 insertions(+)\n> create mode 100644 bundle-uri.c\n> create mode 100644 bundle-uri.h\n> create mode 100755 t/t5557-http-get.sh\n> create mode 100755 t/t5558-clone-bundle-uri.sh\n> \n> \n> base-commit: e72d93e88cb20b06e88e6e7d81bd1dc4effe453f\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1300%2Fderrickstolee%2Fbundle-redo%2Fclone-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1300/derrickstolee/bundle-redo/clone-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1300\n> -- \n> gitgitgadget\n\nLooks good to me, with just one small fix and a couple of optional\nnitpicks. Thaks for the series!\n\nReviewed-by: Josh Steadmon \n"},{"id":"460044","messageId":"220728.86a68ukvg6.gmgdl@evledraar.gmail.com","threadId":"58220","inReplyTo":"40808e92afb7bcf3e8e9b4b53d5e30b5e17816f8.1658781277.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/5] remote-curl: add 'get' capability","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-27T23:00:44Z","receivedAt":"2022-07-27T23:05:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jul 25 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> [...]\n\nAdd a:\n\n\tTEST_PASSES_SANITIZE_LEAK=true\n\nBefore:\n\n> +. ./test-lib.sh\n> +\n> +. \"$TEST_DIRECTORY\"/lib-httpd.sh\n> +start_httpd\n> +\n> +test_expect_success 'get by URL: 404' '\n> +\turl=\"$HTTPD_URL/none.txt\" &&\n> +\tcat >input <<-EOF &&\n> +\tcapabilities\n> +\tget $url file1\n> +\tEOF\n> +\n> +\ttest_must_fail git remote-http $url err &&\n> +\ttest_path_is_missing file1 &&\n> +\tgrep \"failed to download file at URL\" err &&\n> +\trm file1.temp\n\nThis should presumably be a :\n\n\ttest_when_finished \"rm file.temp\"\n\nBefore the \"test_must_fail\", otherwise we'll leave this around if\ne.g. the \"grep\" in the &&-chain fails, but we surely want to clean this\nup in that case..\n\n> +'\n> +\n> +test_expect_success 'get by URL: 200' '\n> +\techo data >\"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" &&\n> +\n> +\turl=\"$HTTPD_URL/exists.txt\" &&\n> +\tcat >input <<-EOF &&\n> +\tcapabilities\n> +\tget $url file2\n> +\n> +\tEOF\n> +\n> +\tGIT_TRACE2_PERF=1 git remote-http $url \n> On Mon, Jul 25 2022, Derrick Stolee via GitGitGadget wrote:\n> \n>> From: Derrick Stolee \n>> [...]\n> \n> Add a:\n> \n> \tTEST_PASSES_SANITIZE_LEAK=true\n\nI trust this is important for your series that flags tests\nthat _could_ pass with this. Thanks.\n\n>> +. ./test-lib.sh\n>> +\n>> +. \"$TEST_DIRECTORY\"/lib-httpd.sh\n>> +start_httpd\n>> +\n>> +test_expect_success 'get by URL: 404' '\n>> +\turl=\"$HTTPD_URL/none.txt\" &&\n>> +\tcat >input <<-EOF &&\n>> +\tcapabilities\n>> +\tget $url file1\n>> +\tEOF\n>> +\n>> +\ttest_must_fail git remote-http $url err &&\n>> +\ttest_path_is_missing file1 &&\n>> +\tgrep \"failed to download file at URL\" err &&\n>> +\trm file1.temp\n> \n> This should presumably be a :\n> \n> \ttest_when_finished \"rm file.temp\"\n\nYes. Thanks.\n\n>> +'\n>> +\n>> +test_expect_success 'get by URL: 200' '\n>> +\techo data >\"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" &&\n>> +\n>> +\turl=\"$HTTPD_URL/exists.txt\" &&\n>> +\tcat >input <<-EOF &&\n>> +\tcapabilities\n>> +\tget $url file2\n>> +\n>> +\tEOF\n>> +\n>> +\tGIT_TRACE2_PERF=1 git remote-http $url \n> Is this ad-hoc debugging that snuck into the test? Nothing here needs\n> this GIT_TRACE2_PERF...\n\nGood eye. Removed.\n\nThanks,\n-Stolee\n"},{"id":"460350","messageId":"3d5aa1ff-3629-b5d9-28a0-a0e204a6cab0@github.com","threadId":"58220","inReplyTo":"YuG3h4AcZjyILvdK@google.com","subject":"Re: [PATCH 2/5] bundle-uri: create basic file-copy logic","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-08-01T13:58:02Z","receivedAt":"2022-08-01T13:58:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/27/2022 6:09 PM, Josh Steadmon wrote:\n> On 2022.07.25 20:34, Derrick Stolee via GitGitGadget wrote:\n>> From: Derrick Stolee \n>>\n>> Before implementing a way to fetch bundles into a repository, create the\n>> basic logic. Assume that the URI is actually a file path. Future logic\n>> will make this more careful to other protocols.\n>>\n>> For now, we also only succeed if the content at the URI is a bundle\n>> file, not a bundle list. Bundle lists will be implemented in a future\n>> change.\n>>\n>> Note that the discovery of a temporary filename is slightly racy because\n>> the odb_mkstemp() relies on the temporary file not existing. With the\n>> current implementation being limited to file copies, we could replace\n>> the copy_file() with copy_fd(). The tricky part comes in future changes\n>> that send the filename to 'git remote-https' and its 'get' capability.\n> \n>> At that point, we need the file descriptor closed _and_ the file\n>> unlinked.\n> \n> Ahh, it looks like this was the point I missed in my previous review.\n> IIUC, we need the file unlinked because http_get_file() will eventually\n> call finalize_object_file() to move a tempfile to the final object name,\n> and that will fail if we have an empty file already in place.\n\nYes, and I also was not sure what would happen if the empty file existed.\nI tested it and thought allowing overwriting an existing file would be a\nbigger problem than this choice of a filename.\n\nWe also discussed options about how it would be nice to have a predictable\nfilename so we could resume downloads _across Git process failures_\ninstead of just a network failure within a single Git process. This is\nsomething to explore when creating that functionality.\n\nThanks,\n-Stolee\n"},{"id":"460351","messageId":"6621a20d-9857-2150-258f-f229cea525dd@github.com","threadId":"58220","inReplyTo":"YuG3miyCKtnv3Na/@google.com","subject":"Re: [PATCH 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-08-01T14:00:30Z","receivedAt":"2022-08-01T14:00:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/27/2022 6:09 PM, Josh Steadmon wrote:\n> On 2022.07.25 20:34, Derrick Stolee via GitGitGadget wrote:\n\n>> +static int copy_uri_to_file(const char *filename, const char *uri)\n>> +{\n>> +\tconst char *out;\n>> +\n>> +\tif (skip_prefix(uri, \"https:\", &out) ||\n>> +\t skip_prefix(uri, \"http:\", &out))\n>> +\t\treturn download_https_uri_to_file(filename, uri);\n> \n> Since we're not using \"out\" here, should we replace the skip_prefix()s\n> with starts_with(), or perhaps even istarts_with()?\n\nSounds good to me. Can't abandon \"out\" because it is used\nfor the \"file://\" prefix, but istarts_with() is better here.\n\nThanks,\n-Stolee\n"},{"id":"460352","messageId":"3ca12cea-aabf-05e8-2502-9cd4e82deb41@github.com","threadId":"58220","inReplyTo":"YuG4ICBFNbLPR9Iv@google.com","subject":"Re: [PATCH 0/5] Bundle URIs II: git clone --bundle-uri","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-08-01T14:00:57Z","receivedAt":"2022-08-01T14:01:08Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/27/2022 6:11 PM, Josh Steadmon wrote:\n\n> Looks good to me, with just one small fix and a couple of optional\n> nitpicks. Thaks for the series!\n> \n> Reviewed-by: Josh Steadmon \n\nThanks for the review!\n\n-Stolee\n"},{"id":"460355","messageId":"220801.86zggogh5j.gmgdl@evledraar.gmail.com","threadId":"58220","inReplyTo":"71afa569-7071-466f-11af-3bbf856b4bef@github.com","subject":"Re: [PATCH 1/5] remote-curl: add 'get' capability","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-01T14:39:34Z","receivedAt":"2022-08-01T14:42:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Aug 01 2022, Derrick Stolee wrote:\n\n> On 7/27/2022 7:00 PM, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Mon, Jul 25 2022, Derrick Stolee via GitGitGadget wrote:\n>> \n>>> From: Derrick Stolee \n>>> [...]\n>> \n>> Add a:\n>> \n>> \tTEST_PASSES_SANITIZE_LEAK=true\n>\n> I trust this is important for your series that flags tests\n> that _could_ pass with this. Thanks.\n\nNot really, it does add a \"check\" mode, and this is one of the few tests\ncrop up when merged with \"seen\" if run in that mode, but currentlny it's\nonly run manually, and we have other outstanding deviations.\n\nIt's just something for \"master\": We have the \"linux-leaks\" job for a\nwhile now, so it makes sense for new tests to declare if they're\nleak-free or not, so we guard against the introduction of leaks with new\ncode, if possible.\n\nI (or someone else) can always follow-up with something like[1], but as\nit's easy it makes sense to just add it in the first place, thanks.\n\n1. https://lore.kernel.org/git/patch-v3-13.15-28255ac3239-20220727T230800Z-avarab@gmail.com/\n"},{"id":"460411","messageId":"pull.1300.v2.git.1659443384.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.git.1658781277.gitgitgadget@gmail.com","subject":"[PATCH v2 0/5] Bundle URIs II: git clone --bundle-uri","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-02T12:29:39Z","receivedAt":"2022-08-02T12:29:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This is the second series building the bundle URI feature as discussed in\nthe previous series that added the design document [1]. This series does not\nmodify the design document, so the patches are independent and can be\napplied to the latest 'master'.\n\n[1]\nhttps://lore.kernel.org/git/pull.1248.v3.git.1658757188.gitgitgadget@gmail.com\n\nThis series brings in just enough logic that we can bootstrap clones from a\nsingle bundle using git clone --bundle-uri=.\n\n * Patch 1 adds a 'get' capability to 'git remote-https' which allows\n downloading the contents of a URI to a local file.\n * Patch 2 creates basic file-copy logic within a new bundle-uri.c file. It\n is not used until patch 3.\n * Patch 3 creates the git clone --bundle-uri= option, allowing Git to\n bootstrap a clone from a bundle, but get the remaining objects from the\n origin URL. (As of this patch, it only accepts a filename.)\n * Patch 4 extends the git clone --bundle-uri= option to allow file://\n and https:// URIs.\n * Patch 5 is a CLI helper to avoid using --bundle-uri and --depth at the\n same time in git clone.\n\nAs outlined in [1], the next steps after this are:\n\n 1. Allow parsing a bundle list as a config file at the given URI. The\n key-value format is unified with the protocol v2 verb (coming in (3)).\n [2]\n 2. Implement the protocol v2 verb, re-using the bundle list logic from (2).\n Use this to auto-discover bundle URIs during 'git clone' (behind a\n config option). [3]\n 3. Implement the 'creationToken' heuristic, allowing incremental 'git\n fetch' commands to download a bundle list from a configured URI, and\n only download bundles that are new based on the creation token values.\n [4]\n\nI have prepared some of this work as pull requests on my personal fork so\ncurious readers can look ahead to where we are going:\n\n[2] https://github.com/derrickstolee/git/pull/20\n\n[3] https://github.com/derrickstolee/git/pull/21\n\n[4] https://github.com/derrickstolee/git/pull/22\n\nNote: this series includes some code pulled out of the first series [1], and\nin particular the git fetch --bundle-uri= option is removed. The\nintention was to replace that with git bundle fetch , but that conflicts\nwith the work to refactor how subcommands are parsed. The git bundle fetch\nsubcommand could be added later for maximum flexibility on the client side,\nbut we can also move forward without it.\n\n\nUpdates in v2\n=============\n\n * Several typos or small tweaks. See the range-diff for details.\n\nThanks, -Stolee\n\nDerrick Stolee (5):\n remote-curl: add 'get' capability\n bundle-uri: create basic file-copy logic\n clone: add --bundle-uri option\n bundle-uri: add support for http(s):// and file://\n clone: --bundle-uri cannot be combined with --depth\n\n Documentation/git-clone.txt | 7 ++\n Documentation/gitremote-helpers.txt | 9 ++\n Makefile | 1 +\n builtin/clone.c | 18 +++\n bundle-uri.c | 168 ++++++++++++++++++++++++++++\n bundle-uri.h | 14 +++\n remote-curl.c | 28 +++++\n t/t5557-http-get.sh | 39 +++++++\n t/t5558-clone-bundle-uri.sh | 81 ++++++++++++++\n t/t5606-clone-options.sh | 8 ++\n 10 files changed, 373 insertions(+)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n create mode 100755 t/t5557-http-get.sh\n create mode 100755 t/t5558-clone-bundle-uri.sh\n\n\nbase-commit: e72d93e88cb20b06e88e6e7d81bd1dc4effe453f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1300%2Fderrickstolee%2Fbundle-redo%2Fclone-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1300/derrickstolee/bundle-redo/clone-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1300\n\nRange-diff vs v1:\n\n 1: 40808e92afb ! 1: 4df4a1d679a remote-curl: add 'get' capability\n @@ Commit message\n 'get ' to download the file at into the file at\n .\n \n + Reviewed-by: Josh Steadmon \n Signed-off-by: Derrick Stolee \n \n ## Documentation/gitremote-helpers.txt ##\n @@ remote-curl.c: static void parse_fetch(struct strbuf *buf)\n \tstrbuf_reset(buf);\n }\n \n -+static void parse_get(struct strbuf *buf)\n ++static void parse_get(const char *arg)\n +{\n +\tstruct strbuf url = STRBUF_INIT;\n +\tstruct strbuf path = STRBUF_INIT;\n -+\tconst char *p, *space;\n ++\tconst char *space;\n +\n -+\tif (!skip_prefix(buf->buf, \"get \", &p))\n -+\t\tdie(_(\"http transport does not support %s\"), buf->buf);\n -+\n -+\tspace = strchr(p, ' ');\n ++\tspace = strchr(arg, ' ');\n +\n +\tif (!space)\n +\t\tdie(_(\"protocol error: expected ' ', missing space\"));\n +\n -+\tstrbuf_add(&url, p, space - p);\n ++\tstrbuf_add(&url, arg, space - arg);\n +\tstrbuf_addstr(&path, space + 1);\n +\n +\tif (http_get_file(url.buf, path.buf, NULL))\n @@ remote-curl.c: static void parse_fetch(struct strbuf *buf)\n +\tstrbuf_release(&path);\n +\tprintf(\"\\n\");\n +\tfflush(stdout);\n -+\tstrbuf_reset(buf);\n +}\n +\n static int push_dav(int nr_spec, const char **specs)\n @@ remote-curl.c: int cmd_main(int argc, const char **argv)\n \t\t\tfflush(stdout);\n \n +\t\t} else if (skip_prefix(buf.buf, \"get \", &arg)) {\n -+\t\t\tparse_get(&buf);\n ++\t\t\tparse_get(arg);\n +\t\t\tfflush(stdout);\n +\n \t\t} else if (!strcmp(buf.buf, \"capabilities\")) {\n @@ t/t5557-http-get.sh (new)\n +start_httpd\n +\n +test_expect_success 'get by URL: 404' '\n ++\ttest_when_finished \"rm -f file.temp\" &&\n +\turl=\"$HTTPD_URL/none.txt\" &&\n +\tcat >input <<-EOF &&\n +\tcapabilities\n @@ t/t5557-http-get.sh (new)\n +\n +\ttest_must_fail git remote-http $url err &&\n +\ttest_path_is_missing file1 &&\n -+\tgrep \"failed to download file at URL\" err &&\n -+\trm file1.temp\n ++\tgrep \"failed to download file at URL\" err\n +'\n +\n +test_expect_success 'get by URL: 200' '\n @@ t/t5557-http-get.sh (new)\n +\n +\tEOF\n +\n -+\tGIT_TRACE2_PERF=1 git remote-http $url \n Signed-off-by: Derrick Stolee \n \n ## Makefile ##\n @@ bundle-uri.h (new)\n +int fetch_bundle_uri(struct repository *r, const char *uri);\n +\n +#endif\n +\n + ## t/t5557-http-get.sh ##\n +@@\n + \n + test_description='test downloading a file by URL'\n + \n ++TEST_PASSES_SANITIZE_LEAK=true\n ++\n + . ./test-lib.sh\n + \n + . \"$TEST_DIRECTORY\"/lib-httpd.sh\n 3: 29e645a54ba ! 3: 00debaf6e77 clone: add --bundle-uri option\n @@ Commit message\n Git to bootstrap the new repository with these bundles before fetching\n the remaining objects from the origin server.\n \n + Reviewed-by: Josh Steadmon \n Signed-off-by: Derrick Stolee \n \n ## Documentation/git-clone.txt ##\n 4: f6bc3177332 ! 4: e4f2dcc7a45 bundle-uri: add support for http(s):// and file://\n @@ Commit message\n Otherwise, check to see if file:// is present and modify the prefix\n accordingly.\n \n + Reviewed-by: Josh Steadmon \n Signed-off-by: Derrick Stolee \n \n ## bundle-uri.c ##\n @@ bundle-uri.c: static int find_temp_filename(struct strbuf *name)\n +{\n +\tconst char *out;\n +\n -+\tif (skip_prefix(uri, \"https:\", &out) ||\n -+\t skip_prefix(uri, \"http:\", &out))\n ++\tif (istarts_with(uri, \"https:\") ||\n ++\t istarts_with(uri, \"http:\"))\n +\t\treturn download_https_uri_to_file(filename, uri);\n +\n +\tif (!skip_prefix(uri, \"file://\", &out))\n 5: e823b168ab7 ! 5: acee1fae027 clone: --bundle-uri cannot be combined with --depth\n @@ Commit message\n fetch request, but with a list of haves that might cause a more painful\n computation of that shallow pack-file.\n \n + Reviewed-by: Josh Steadmon \n Signed-off-by: Derrick Stolee \n \n ## Documentation/git-clone.txt ##\n\n-- \ngitgitgadget\n"},{"id":"460412","messageId":"4df4a1d679a1003ccd3a4dd049bc05141831970c.1659443384.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v2.git.1659443384.gitgitgadget@gmail.com","subject":"[PATCH v2 1/5] remote-curl: add 'get' capability","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-02T12:29:40Z","receivedAt":"2022-08-02T12:29:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nA future change will want a way to download a file over HTTP(S) using\nthe simplest of download mechanisms. We do not want to assume that the\nserver on the other side understands anything about the Git protocol but\ncould be a simple static web server.\n\nCreate the new 'get' capability for the remote helpers which advertises\nthat the 'get' command is avalable. A caller can send a line containing\n'get ' to download the file at into the file at\n.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Documentation/gitremote-helpers.txt | 9 +++++++\n remote-curl.c | 28 ++++++++++++++++++++++\n t/t5557-http-get.sh | 37 +++++++++++++++++++++++++++++\n 3 files changed, 74 insertions(+)\n create mode 100755 t/t5557-http-get.sh\n\ndiff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\nindex 6f1e269ae43..ed8da428c98 100644\n--- a/Documentation/gitremote-helpers.txt\n+++ b/Documentation/gitremote-helpers.txt\n@@ -168,6 +168,9 @@ Supported commands: 'list', 'import'.\n \tCan guarantee that when a clone is requested, the received\n \tpack is self contained and is connected.\n \n+'get'::\n+\tCan use the 'get' command to download a file from a given URI.\n+\n If a helper advertises 'connect', Git will use it if possible and\n fall back to another capability if the helper requests so when\n connecting (see the 'connect' command under COMMANDS).\n@@ -418,6 +421,12 @@ Supported if the helper has the \"connect\" capability.\n +\n Supported if the helper has the \"stateless-connect\" capability.\n \n+'get' ::\n+\tDownloads the file from the given `` to the given ``. If\n+\t`.temp` exists, then Git assumes that the `.temp` file is a\n+\tpartial download from a previous attempt and will resume the\n+\tdownload from that position.\n+\n If a fatal error occurs, the program writes the error message to\n stderr and exits. The caller should expect that a suitable error\n message has been printed if the child closes the connection without\ndiff --git a/remote-curl.c b/remote-curl.c\nindex b8758757ece..72dfb8fb86a 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1286,6 +1286,29 @@ static void parse_fetch(struct strbuf *buf)\n \tstrbuf_reset(buf);\n }\n \n+static void parse_get(const char *arg)\n+{\n+\tstruct strbuf url = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tconst char *space;\n+\n+\tspace = strchr(arg, ' ');\n+\n+\tif (!space)\n+\t\tdie(_(\"protocol error: expected ' ', missing space\"));\n+\n+\tstrbuf_add(&url, arg, space - arg);\n+\tstrbuf_addstr(&path, space + 1);\n+\n+\tif (http_get_file(url.buf, path.buf, NULL))\n+\t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n+\n+\tstrbuf_release(&url);\n+\tstrbuf_release(&path);\n+\tprintf(\"\\n\");\n+\tfflush(stdout);\n+}\n+\n static int push_dav(int nr_spec, const char **specs)\n {\n \tstruct child_process child = CHILD_PROCESS_INIT;\n@@ -1564,9 +1587,14 @@ int cmd_main(int argc, const char **argv)\n \t\t\t\tprintf(\"unsupported\\n\");\n \t\t\tfflush(stdout);\n \n+\t\t} else if (skip_prefix(buf.buf, \"get \", &arg)) {\n+\t\t\tparse_get(arg);\n+\t\t\tfflush(stdout);\n+\n \t\t} else if (!strcmp(buf.buf, \"capabilities\")) {\n \t\t\tprintf(\"stateless-connect\\n\");\n \t\t\tprintf(\"fetch\\n\");\n+\t\t\tprintf(\"get\\n\");\n \t\t\tprintf(\"option\\n\");\n \t\t\tprintf(\"push\\n\");\n \t\t\tprintf(\"check-connectivity\\n\");\ndiff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\nnew file mode 100755\nindex 00000000000..09fb0bd9a8a\n--- /dev/null\n+++ b/t/t5557-http-get.sh\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+\n+test_description='test downloading a file by URL'\n+\n+. ./test-lib.sh\n+\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'get by URL: 404' '\n+\ttest_when_finished \"rm -f file.temp\" &&\n+\turl=\"$HTTPD_URL/none.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\tget $url file1\n+\tEOF\n+\n+\ttest_must_fail git remote-http $url err &&\n+\ttest_path_is_missing file1 &&\n+\tgrep \"failed to download file at URL\" err\n+'\n+\n+test_expect_success 'get by URL: 200' '\n+\techo data >\"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" &&\n+\n+\turl=\"$HTTPD_URL/exists.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\tget $url file2\n+\n+\tEOF\n+\n+\tgit remote-http $url \n\nBefore implementing a way to fetch bundles into a repository, create the\nbasic logic. Assume that the URI is actually a file path. Future logic\nwill make this more careful to other protocols.\n\nFor now, we also only succeed if the content at the URI is a bundle\nfile, not a bundle list. Bundle lists will be implemented in a future\nchange.\n\nNote that the discovery of a temporary filename is slightly racy because\nthe odb_mkstemp() relies on the temporary file not existing. With the\ncurrent implementation being limited to file copies, we could replace\nthe copy_file() with copy_fd(). The tricky part comes in future changes\nthat send the filename to 'git remote-https' and its 'get' capability.\nAt that point, we need the file descriptor closed _and_ the file\nunlinked. If we were to keep the file descriptor open for the sake of\nnormal file copies, then we would pollute the rest of the code for\nlittle benefit. This is especially the case because we expect that most\nbundle URI use will be based on HTTPS instead of file copies.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Makefile | 1 +\n bundle-uri.c | 104 ++++++++++++++++++++++++++++++++++++++++++++\n bundle-uri.h | 14 ++++++\n t/t5557-http-get.sh | 2 +\n 4 files changed, 121 insertions(+)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n\ndiff --git a/Makefile b/Makefile\nindex 1624471badc..7d5f48069ea 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -906,6 +906,7 @@ LIB_OBJS += blob.o\n LIB_OBJS += bloom.o\n LIB_OBJS += branch.o\n LIB_OBJS += bulk-checkin.o\n+LIB_OBJS += bundle-uri.o\n LIB_OBJS += bundle.o\n LIB_OBJS += cache-tree.o\n LIB_OBJS += cbtree.o\ndiff --git a/bundle-uri.c b/bundle-uri.c\nnew file mode 100644\nindex 00000000000..b35babc36aa\n--- /dev/null\n+++ b/bundle-uri.c\n@@ -0,0 +1,104 @@\n+#include \"cache.h\"\n+#include \"bundle-uri.h\"\n+#include \"bundle.h\"\n+#include \"object-store.h\"\n+#include \"refs.h\"\n+#include \"run-command.h\"\n+\n+static int find_temp_filename(struct strbuf *name)\n+{\n+\tint fd;\n+\t/*\n+\t * Find a temporary filename that is available. This is briefly\n+\t * racy, but unlikely to collide.\n+\t */\n+\tfd = odb_mkstemp(name, \"bundles/tmp_uri_XXXXXX\");\n+\tif (fd < 0) {\n+\t\twarning(_(\"failed to create temporary file\"));\n+\t\treturn -1;\n+\t}\n+\n+\tclose(fd);\n+\tunlink(name->buf);\n+\treturn 0;\n+}\n+\n+static int copy_uri_to_file(const char *file, const char *uri)\n+{\n+\t/* File-based URIs only for now. */\n+\treturn copy_file(file, uri, 0);\n+}\n+\n+static int unbundle_from_file(struct repository *r, const char *file)\n+{\n+\tint result = 0;\n+\tint bundle_fd;\n+\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n+\tstruct string_list_item *refname;\n+\tstruct strbuf bundle_ref = STRBUF_INIT;\n+\tsize_t bundle_prefix_len;\n+\n+\tif ((bundle_fd = read_bundle_header(file, &header)) < 0)\n+\t\treturn 1;\n+\n+\tif ((result = unbundle(r, &header, bundle_fd, NULL)))\n+\t\treturn 1;\n+\n+\t/*\n+\t * Convert all refs/heads/ from the bundle into refs/bundles/\n+\t * in the local repository.\n+\t */\n+\tstrbuf_addstr(&bundle_ref, \"refs/bundles/\");\n+\tbundle_prefix_len = bundle_ref.len;\n+\n+\tfor_each_string_list_item(refname, &header.references) {\n+\t\tstruct object_id *oid = refname->util;\n+\t\tstruct object_id old_oid;\n+\t\tconst char *branch_name;\n+\t\tint has_old;\n+\n+\t\tif (!skip_prefix(refname->string, \"refs/heads/\", &branch_name))\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_setlen(&bundle_ref, bundle_prefix_len);\n+\t\tstrbuf_addstr(&bundle_ref, branch_name);\n+\n+\t\thas_old = !read_ref(bundle_ref.buf, &old_oid);\n+\t\tupdate_ref(\"fetched bundle\", bundle_ref.buf, oid,\n+\t\t\t has_old ? &old_oid : NULL,\n+\t\t\t REF_SKIP_OID_VERIFICATION,\n+\t\t\t UPDATE_REFS_MSG_ON_ERR);\n+\t}\n+\n+\tbundle_header_release(&header);\n+\treturn result;\n+}\n+\n+int fetch_bundle_uri(struct repository *r, const char *uri)\n+{\n+\tint result = 0;\n+\tstruct strbuf filename = STRBUF_INIT;\n+\n+\tif ((result = find_temp_filename(&filename)))\n+\t\tgoto cleanup;\n+\n+\tif ((result = copy_uri_to_file(filename.buf, uri))) {\n+\t\twarning(_(\"failed to download bundle from URI '%s'\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif ((result = !is_bundle(filename.buf, 0))) {\n+\t\twarning(_(\"file at URI '%s' is not a bundle\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif ((result = unbundle_from_file(r, filename.buf))) {\n+\t\twarning(_(\"failed to unbundle bundle from URI '%s'\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+cleanup:\n+\tunlink(filename.buf);\n+\tstrbuf_release(&filename);\n+\treturn result;\n+}\ndiff --git a/bundle-uri.h b/bundle-uri.h\nnew file mode 100644\nindex 00000000000..8a152f1ef14\n--- /dev/null\n+++ b/bundle-uri.h\n@@ -0,0 +1,14 @@\n+#ifndef BUNDLE_URI_H\n+#define BUNDLE_URI_H\n+\n+struct repository;\n+\n+/**\n+ * Fetch data from the given 'uri' and unbundle the bundle data found\n+ * based on that information.\n+ *\n+ * Returns non-zero if no bundle information is found at the given 'uri'.\n+ */\n+int fetch_bundle_uri(struct repository *r, const char *uri);\n+\n+#endif\ndiff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\nindex 09fb0bd9a8a..76a4bbd16af 100755\n--- a/t/t5557-http-get.sh\n+++ b/t/t5557-http-get.sh\n@@ -2,6 +2,8 @@\n \n test_description='test downloading a file by URL'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n+\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n-- \ngitgitgadget\n\n"},{"id":"460414","messageId":"00debaf6e77852efe1dcad4bfda5ebd5bf590ac4.1659443384.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v2.git.1659443384.gitgitgadget@gmail.com","subject":"[PATCH v2 3/5] clone: add --bundle-uri option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-02T12:29:42Z","receivedAt":"2022-08-02T12:30:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nCloning a remote repository is one of the most expensive operations in\nGit. The server can spend a lot of CPU time generating a pack-file for\nthe client's request. The amount of data can clog the network for a long\ntime, and the Git protocol is not resumable. For users with poor network\nconnections or are located far away from the origin server, this can be\nespecially painful.\n\nAdd a new '--bundle-uri' option to 'git clone' to bootstrap a clone from\na bundle. If the user is aware of a bundle server, then they can tell\nGit to bootstrap the new repository with these bundles before fetching\nthe remaining objects from the origin server.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Documentation/git-clone.txt | 6 ++++++\n builtin/clone.c | 15 +++++++++++++++\n t/t5558-clone-bundle-uri.sh | 36 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+)\n create mode 100755 t/t5558-clone-bundle-uri.sh\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 632bd1348ea..60fedf7eb5e 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -323,6 +323,12 @@ or `--mirror` is given)\n \tfor `host.xz:foo/.git`). Cloning into an existing directory\n \tis only allowed if the directory is empty.\n \n+--bundle-uri=::\n+\tBefore fetching from the remote, fetch a bundle from the given\n+\t`` and unbundle the data into the local repository. The refs\n+\tin the bundle will be stored under the hidden `refs/bundle/*`\n+\tnamespace.\n+\n :git-clone: 1\n include::urls.txt[]\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex c4ff4643ecd..4224d562758 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -34,6 +34,7 @@\n #include \"list-objects-filter-options.h\"\n #include \"hook.h\"\n #include \"bundle.h\"\n+#include \"bundle-uri.h\"\n \n /*\n * Overall FIXMEs:\n@@ -77,6 +78,7 @@ static int option_filter_submodules = -1; /* unspecified */\n static int config_filter_submodules = -1; /* unspecified */\n static struct string_list server_options = STRING_LIST_INIT_NODUP;\n static int option_remote_submodules;\n+static const char *bundle_uri;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -160,6 +162,8 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"any cloned submodules will use their remote-tracking branch\")),\n \tOPT_BOOL(0, \"sparse\", &option_sparse_checkout,\n \t\t N_(\"initialize sparse-checkout file to include only files at root\")),\n+\tOPT_STRING(0, \"bundle-uri\", &bundle_uri,\n+\t\t N_(\"uri\"), N_(\"a URI for downloading bundles before fetching from origin remote\")),\n \tOPT_END()\n };\n \n@@ -1232,6 +1236,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (transport->smart_options && !deepen && !filter_options.choice)\n \t\ttransport->smart_options->check_self_contained_and_connected = 1;\n \n+\t/*\n+\t * Before fetching from the remote, download and install bundle\n+\t * data from the --bundle-uri option.\n+\t */\n+\tif (bundle_uri) {\n+\t\t/* At this point, we need the_repository to match the cloned repo. */\n+\t\trepo_init(the_repository, git_dir, work_tree);\n+\t\tif (fetch_bundle_uri(the_repository, bundle_uri))\n+\t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n+\t\t\t\tbundle_uri);\n+\t}\n \n \tstrvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n \trefspec_ref_prefixes(&remote->fetch,\ndiff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh\nnew file mode 100755\nindex 00000000000..f709bcb729c\n--- /dev/null\n+++ b/t/t5558-clone-bundle-uri.sh\n@@ -0,0 +1,36 @@\n+#!/bin/sh\n+\n+test_description='test fetching bundles with --bundle-uri'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'fail to clone from non-existent file' '\n+\ttest_when_finished rm -rf test &&\n+\tgit clone --bundle-uri=\"$(pwd)/does-not-exist\" . test 2>err &&\n+\tgrep \"failed to download bundle from URI\" err\n+'\n+\n+test_expect_success 'fail to clone from non-bundle file' '\n+\ttest_when_finished rm -rf test &&\n+\techo bogus >bogus &&\n+\tgit clone --bundle-uri=\"$(pwd)/bogus\" . test 2>err &&\n+\tgrep \"is not a bundle\" err\n+'\n+\n+test_expect_success 'create bundle' '\n+\tgit init clone-from &&\n+\tgit -C clone-from checkout -b topic &&\n+\ttest_commit -C clone-from A &&\n+\ttest_commit -C clone-from B &&\n+\tgit -C clone-from bundle create B.bundle topic\n+'\n+\n+test_expect_success 'clone with path bundle' '\n+\tgit clone --bundle-uri=\"clone-from/B.bundle\" \\\n+\t\tclone-from clone-path &&\n+\tgit -C clone-path rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"460415","messageId":"e4f2dcc7a45388663aeac786e5abdcf2164cfe62.1659443384.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v2.git.1659443384.gitgitgadget@gmail.com","subject":"[PATCH v2 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-02T12:29:43Z","receivedAt":"2022-08-02T12:30:05Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe previous change created the 'git clone --bundle-uri=' option.\nCurrently, must be a filename.\n\nUpdate copy_uri_to_file() to first inspect the URI for an HTTP(S) prefix\nand use git-remote-https as the way to download the data at that URI.\nOtherwise, check to see if file:// is present and modify the prefix\naccordingly.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n bundle-uri.c | 70 +++++++++++++++++++++++++++++++++++--\n t/t5558-clone-bundle-uri.sh | 45 ++++++++++++++++++++++++\n 2 files changed, 112 insertions(+), 3 deletions(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex b35babc36aa..930e995d194 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -23,10 +23,74 @@ static int find_temp_filename(struct strbuf *name)\n \treturn 0;\n }\n \n-static int copy_uri_to_file(const char *file, const char *uri)\n+static int download_https_uri_to_file(const char *file, const char *uri)\n {\n-\t/* File-based URIs only for now. */\n-\treturn copy_file(file, uri, 0);\n+\tint result = 0;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tFILE *child_in = NULL, *child_out = NULL;\n+\tstruct strbuf line = STRBUF_INIT;\n+\tint found_get = 0;\n+\n+\tstrvec_pushl(&cp.args, \"git-remote-https\", \"origin\", uri, NULL);\n+\tcp.in = -1;\n+\tcp.out = -1;\n+\n+\tif (start_command(&cp))\n+\t\treturn 1;\n+\n+\tchild_in = fdopen(cp.in, \"w\");\n+\tif (!child_in) {\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tchild_out = fdopen(cp.out, \"r\");\n+\tif (!child_out) {\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tfprintf(child_in, \"capabilities\\n\");\n+\tfflush(child_in);\n+\n+\twhile (!strbuf_getline(&line, child_out)) {\n+\t\tif (!line.len)\n+\t\t\tbreak;\n+\t\tif (!strcmp(line.buf, \"get\"))\n+\t\t\tfound_get = 1;\n+\t}\n+\tstrbuf_release(&line);\n+\n+\tif (!found_get) {\n+\t\tresult = error(_(\"insufficient capabilities\"));\n+\t\tgoto cleanup;\n+\t}\n+\n+\tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n+\n+cleanup:\n+\tif (child_in)\n+\t\tfclose(child_in);\n+\tif (finish_command(&cp))\n+\t\treturn 1;\n+\tif (child_out)\n+\t\tfclose(child_out);\n+\treturn result;\n+}\n+\n+static int copy_uri_to_file(const char *filename, const char *uri)\n+{\n+\tconst char *out;\n+\n+\tif (istarts_with(uri, \"https:\") ||\n+\t istarts_with(uri, \"http:\"))\n+\t\treturn download_https_uri_to_file(filename, uri);\n+\n+\tif (!skip_prefix(uri, \"file://\", &out))\n+\t\tout = uri;\n+\n+\t/* Copy as a file */\n+\treturn copy_file(filename, out, 0);\n }\n \n static int unbundle_from_file(struct repository *r, const char *file)\ndiff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh\nindex f709bcb729c..ad666a2d28a 100755\n--- a/t/t5558-clone-bundle-uri.sh\n+++ b/t/t5558-clone-bundle-uri.sh\n@@ -33,4 +33,49 @@ test_expect_success 'clone with path bundle' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'clone with file:// bundle' '\n+\tgit clone --bundle-uri=\"file://$(pwd)/clone-from/B.bundle\" \\\n+\t\tclone-from clone-file &&\n+\tgit -C clone-file rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+#########################################################################\n+# HTTP tests begin here\n+\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'fail to fetch from non-existent HTTP URL' '\n+\ttest_when_finished rm -rf test &&\n+\tgit clone --bundle-uri=\"$HTTPD_URL/does-not-exist\" . test 2>err &&\n+\tgrep \"failed to download bundle from URI\" err\n+'\n+\n+test_expect_success 'fail to fetch from non-bundle HTTP URL' '\n+\ttest_when_finished rm -rf test &&\n+\techo bogus >\"$HTTPD_DOCUMENT_ROOT_PATH/bogus\" &&\n+\tgit clone --bundle-uri=\"$HTTPD_URL/bogus\" . test 2>err &&\n+\tgrep \"is not a bundle\" err\n+'\n+\n+test_expect_success 'clone HTTP bundle' '\n+\tcp clone-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n+\n+\tgit clone --no-local --mirror clone-from \\\n+\t\t\"$HTTPD_DOCUMENT_ROOT_PATH/fetch.git\" &&\n+\n+\tgit clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n+\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http &&\n+\tgit -C clone-http rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_config -C clone-http log.excludedecoration refs/bundle/\n+'\n+\n+# Do not add tests here unless they use the HTTP server, as they will\n+# not run unless the HTTP dependencies exist.\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"460416","messageId":"acee1fae0274306790fe1a6b0ae8eb2ed04ec32a.1659443384.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v2.git.1659443384.gitgitgadget@gmail.com","subject":"[PATCH v2 5/5] clone: --bundle-uri cannot be combined with --depth","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-02T12:29:44Z","receivedAt":"2022-08-02T12:30:06Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nA previous change added the '--bundle-uri' option, but did not check\nif the --depth parameter was included. Since bundles are not compatible\nwith shallow clones, provide an error message to the user who is\nattempting this combination.\n\nI am leaving this as its own change, separate from the one that\nimplements '--bundle-uri', because this is more of an advisory for the\nuser. There is nothing wrong with bootstrapping with bundles and then\nfetching a shallow clone. However, that is likely going to involve too\nmuch work for the client _and_ the server. The client will download all\nof this bundle information containing the full history of the\nrepository only to ignore most of it. The server will get a shallow\nfetch request, but with a list of haves that might cause a more painful\ncomputation of that shallow pack-file.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Documentation/git-clone.txt | 3 ++-\n builtin/clone.c | 3 +++\n t/t5606-clone-options.sh | 8 ++++++++\n 3 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 60fedf7eb5e..d032d971dd7 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -327,7 +327,8 @@ or `--mirror` is given)\n \tBefore fetching from the remote, fetch a bundle from the given\n \t`` and unbundle the data into the local repository. The refs\n \tin the bundle will be stored under the hidden `refs/bundle/*`\n-\tnamespace.\n+\tnamespace. This option is incompatible with `--depth`,\n+\t`--shallow-since`, and `--shallow-exclude`.\n \n :git-clone: 1\n include::urls.txt[]\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 4224d562758..4463789680b 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -937,6 +937,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_no_checkout = 1;\n \t}\n \n+\tif (bundle_uri && deepen)\n+\t\tdie(_(\"--bundle-uri is incompatible with --depth, --shallow-since, and --shallow-exclude\"));\n+\n \trepo_name = argv[0];\n \n \tpath = get_repo_path(repo_name, &is_bundle);\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 8f676d6b0c0..f6bb02ab947 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -58,6 +58,14 @@ test_expect_success 'disallows --bare with --separate-git-dir' '\n \n '\n \n+test_expect_success 'disallows --bundle-uri with shallow options' '\n+\tfor option in --depth=1 --shallow-since=01-01-2000 --shallow-exclude=HEAD\n+\tdo\n+\t\ttest_must_fail git clone --bundle-uri=bundle $option from to 2>err &&\n+\t\tgrep \"bundle-uri is incompatible\" err || return 1\n+\tdone\n+'\n+\n test_expect_success 'reject cloning shallow repository' '\n \ttest_when_finished \"rm -rf repo\" &&\n \ttest_must_fail git clone --reject-shallow shallow-repo out 2>err &&\n-- \ngitgitgadget\n"},{"id":"460506","messageId":"xmqq8ro6fi1j.fsf@gitster.g","threadId":"58220","inReplyTo":"e4f2dcc7a45388663aeac786e5abdcf2164cfe62.1659443384.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-02T21:32:24Z","receivedAt":"2022-08-02T21:32:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> +static int copy_uri_to_file(const char *filename, const char *uri)\n> +{\n> +\tconst char *out;\n> +\n> +\tif (istarts_with(uri, \"https:\") ||\n> +\t istarts_with(uri, \"http:\"))\n\nLet's be a bit more strict to avoid mistakes and make the code\nimmediately obvious, e.g.\n\n\tif (istarts_with(uri, \"https://\") ||\n\t istarts_with(uri, \"http://\"))\n\n> +\t\treturn download_https_uri_to_file(filename, uri);\n> +\n> +\tif (!skip_prefix(uri, \"file://\", &out))\n> +\t\tout = uri;\n\nIf we are using istarts_with because URI scheme name is case\ninsensitive, shouldn't we do the same for \"file://\" URL, not\njust for \"http(s)://\" URL? IOW\n\n\tif (!skip_iprefix(uri, \"file://\", &out))\n\n> +static int download_https_uri_to_file(const char *file, const char *uri)\n> {\n> +\tint result = 0;\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tFILE *child_in = NULL, *child_out = NULL;\n> +\tstruct strbuf line = STRBUF_INIT;\n> +\tint found_get = 0;\n> +\n> +\tstrvec_pushl(&cp.args, \"git-remote-https\", \"origin\", uri, NULL);\n\nDoes \"git-remote-https\" talk to a \"http://\" URL just fine when uri\nparameter starts with \"http://\"? Would it be the same if the uri\nparameter begins with say \"Http://\"?\n\nOther than that, looks sensible.\n\n"},{"id":"460623","messageId":"9afd5eb2-44a0-6342-6006-5dbdefba9947@github.com","threadId":"58220","inReplyTo":"xmqq8ro6fi1j.fsf@gitster.g","subject":"Re: [PATCH v2 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-08-04T15:34:36Z","receivedAt":"2022-08-04T15:34:42Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/2/2022 5:32 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> +static int copy_uri_to_file(const char *filename, const char *uri)\n>> +{\n>> +\tconst char *out;\n>> +\n>> +\tif (istarts_with(uri, \"https:\") ||\n>> +\t istarts_with(uri, \"http:\"))\n> \n> Let's be a bit more strict to avoid mistakes and make the code\n> immediately obvious, e.g.\n> \n> \tif (istarts_with(uri, \"https://\") ||\n> \t istarts_with(uri, \"http://\"))\n> \n>> +\t\treturn download_https_uri_to_file(filename, uri);\n>> +\n>> +\tif (!skip_prefix(uri, \"file://\", &out))\n>> +\t\tout = uri;\n> \n> If we are using istarts_with because URI scheme name is case\n> insensitive, shouldn't we do the same for \"file://\" URL, not\n> just for \"http(s)://\" URL? IOW\n> \n> \tif (!skip_iprefix(uri, \"file://\", &out))\n\nGood ideas. Of course, we don't have a skip_iprefix(), but\nI can use \"istarts_with()\" and then manually add the length.\nIf we see more need for that in the future, we can consider\nadding it.\n\n(It's interesting that these uses in bundle-uri.c are the\nonly uses of istarts_with() that I see in the codebase.)\n\n>> +static int download_https_uri_to_file(const char *file, const char *uri)\n>> {\n>> +\tint result = 0;\n>> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n>> +\tFILE *child_in = NULL, *child_out = NULL;\n>> +\tstruct strbuf line = STRBUF_INIT;\n>> +\tint found_get = 0;\n>> +\n>> +\tstrvec_pushl(&cp.args, \"git-remote-https\", \"origin\", uri, NULL);\n> \n> Does \"git-remote-https\" talk to a \"http://\" URL just fine when uri\n> parameter starts with \"http://\"? Would it be the same if the uri\n> parameter begins with say \"Http://\"?\n\nI did a quick check of our HTTPS tests modifying the HTTPD_PROTO\nvariable in lib-httpd.sh to \"HtTP\" and we get this fun error:\n\n+ git clone --filter=blob:limit=0 HtTP://127.0.0.1:5601/smart/server client\nCloning into 'client'...\ngit: 'remote-HtTP' is not a git command. See 'git --help'.\n\nSo I guess I can keep case-sensitive comparisons here.\n\nThanks,\n-Stolee\n"},{"id":"460653","messageId":"xmqqsfmb50s0.fsf@gitster.g","threadId":"58220","inReplyTo":"9afd5eb2-44a0-6342-6006-5dbdefba9947@github.com","subject":"Re: [PATCH v2 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-04T18:19:59Z","receivedAt":"2022-08-04T18:20:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n>>> +\tif (istarts_with(uri, \"https:\") ||\n>>> +\t istarts_with(uri, \"http:\"))\n>> \n>> Let's be a bit more strict to avoid mistakes and make the code\n>> immediately obvious, e.g.\n>> \n>> \tif (istarts_with(uri, \"https://\") ||\n>> \t istarts_with(uri, \"http://\"))\n\nThis part (i.e. check for \"://\", not \":\") still\nstands, as the latter could be an scp style Git URL that goes to the\nhost whose name is . As mentioned later, s/istarts/starts/\nis probably a good thing to do here.\n\n>> Does \"git-remote-https\" talk to a \"http://\" URL just fine when uri\n>> parameter starts with \"http://\"? Would it be the same if the uri\n>> parameter begins with say \"Http://\"?\n>\n> I did a quick check of our HTTPS tests modifying the HTTPD_PROTO\n> variable in lib-httpd.sh to \"HtTP\" and we get this fun error:\n>\n> + git clone --filter=blob:limit=0 HtTP://127.0.0.1:5601/smart/server client\n> Cloning into 'client'...\n> git: 'remote-HtTP' is not a git command. See 'git --help'.\n>\n> So I guess I can keep case-sensitive comparisons here.\n\nGuarding them to lowercase-only may sound like a cop-out to purists,\nbut I think it is reasonable thing to do. The only folks that would\nbe offended by are protocol lawyers, and as your check shows, we are\ntreating case sensitively already.\n\nAn obvious alternative is to downcase the \"://\" part but I\ndo not think it is worth it; we have to do that everywhere and we\nneed to be confident that we covered all the code paths---the latter\nis expensive.\n\nThere do exist skip_iprefix() that are used in many places, like\nconvert, http, mailinfo, etc. by the way. That is a moot point as\nwe are not doing case insensitive comparison anymore.\n\nThanks.\n"},{"id":"460905","messageId":"pull.1300.v3.git.1660050703.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v2.git.1659443384.gitgitgadget@gmail.com","subject":"[PATCH v3 0/5] Bundle URIs II: git clone --bundle-uri","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-09T13:11:38Z","receivedAt":"2022-08-09T13:11:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This is the second series building the bundle URI feature as discussed in\nthe previous series that added the design document [1]. This series does not\nmodify the design document, so the patches are independent and can be\napplied to the latest 'master'.\n\n[1]\nhttps://lore.kernel.org/git/pull.1248.v3.git.1658757188.gitgitgadget@gmail.com\n\nThis series brings in just enough logic that we can bootstrap clones from a\nsingle bundle using git clone --bundle-uri=.\n\n * Patch 1 adds a 'get' capability to 'git remote-https' which allows\n downloading the contents of a URI to a local file.\n * Patch 2 creates basic file-copy logic within a new bundle-uri.c file. It\n is not used until patch 3.\n * Patch 3 creates the git clone --bundle-uri= option, allowing Git to\n bootstrap a clone from a bundle, but get the remaining objects from the\n origin URL. (As of this patch, it only accepts a filename.)\n * Patch 4 extends the git clone --bundle-uri= option to allow file://\n and https:// URIs.\n * Patch 5 is a CLI helper to avoid using --bundle-uri and --depth at the\n same time in git clone.\n\nAs outlined in [1], the next steps after this are:\n\n 1. Allow parsing a bundle list as a config file at the given URI. The\n key-value format is unified with the protocol v2 verb (coming in (3)).\n [2]\n 2. Implement the protocol v2 verb, re-using the bundle list logic from (2).\n Use this to auto-discover bundle URIs during 'git clone' (behind a\n config option). [3]\n 3. Implement the 'creationToken' heuristic, allowing incremental 'git\n fetch' commands to download a bundle list from a configured URI, and\n only download bundles that are new based on the creation token values.\n [4]\n\nI have prepared some of this work as pull requests on my personal fork so\ncurious readers can look ahead to where we are going:\n\n[2] https://github.com/derrickstolee/git/pull/20\n\n[3] https://github.com/derrickstolee/git/pull/21\n\n[4] https://github.com/derrickstolee/git/pull/22\n\nNote: this series includes some code pulled out of the first series [1], and\nin particular the git fetch --bundle-uri= option is removed. The\nintention was to replace that with git bundle fetch , but that conflicts\nwith the work to refactor how subcommands are parsed. The git bundle fetch\nsubcommand could be added later for maximum flexibility on the client side,\nbut we can also move forward without it.\n\n\nUpdates in v3\n=============\n\n * The protocol matching for \"http://\", \"https://\", and \"file://\" now\n uniformly include the \"//\" portion and are case-sensitive.\n\n\nUpdates in v2\n=============\n\n * Several typos or small tweaks. See the range-diff for details.\n\nThanks, -Stolee\n\nDerrick Stolee (5):\n remote-curl: add 'get' capability\n bundle-uri: create basic file-copy logic\n clone: add --bundle-uri option\n bundle-uri: add support for http(s):// and file://\n clone: --bundle-uri cannot be combined with --depth\n\n Documentation/git-clone.txt | 7 ++\n Documentation/gitremote-helpers.txt | 9 ++\n Makefile | 1 +\n builtin/clone.c | 18 +++\n bundle-uri.c | 168 ++++++++++++++++++++++++++++\n bundle-uri.h | 14 +++\n remote-curl.c | 28 +++++\n t/t5557-http-get.sh | 39 +++++++\n t/t5558-clone-bundle-uri.sh | 81 ++++++++++++++\n t/t5606-clone-options.sh | 8 ++\n 10 files changed, 373 insertions(+)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n create mode 100755 t/t5557-http-get.sh\n create mode 100755 t/t5558-clone-bundle-uri.sh\n\n\nbase-commit: e72d93e88cb20b06e88e6e7d81bd1dc4effe453f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1300%2Fderrickstolee%2Fbundle-redo%2Fclone-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1300/derrickstolee/bundle-redo/clone-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1300\n\nRange-diff vs v2:\n\n 1: 4df4a1d679a = 1: 4df4a1d679a remote-curl: add 'get' capability\n 2: 6a6f1a04889 = 2: 6a6f1a04889 bundle-uri: create basic file-copy logic\n 3: 00debaf6e77 = 3: 00debaf6e77 clone: add --bundle-uri option\n 4: e4f2dcc7a45 ! 4: 66a1ce40451 bundle-uri: add support for http(s):// and file://\n @@ bundle-uri.c: static int find_temp_filename(struct strbuf *name)\n +\tstruct strbuf line = STRBUF_INIT;\n +\tint found_get = 0;\n +\n -+\tstrvec_pushl(&cp.args, \"git-remote-https\", \"origin\", uri, NULL);\n ++\tstrvec_pushl(&cp.args, \"git-remote-https\", uri, NULL);\n +\tcp.in = -1;\n +\tcp.out = -1;\n +\n @@ bundle-uri.c: static int find_temp_filename(struct strbuf *name)\n +{\n +\tconst char *out;\n +\n -+\tif (istarts_with(uri, \"https:\") ||\n -+\t istarts_with(uri, \"http:\"))\n ++\tif (starts_with(uri, \"https:\") ||\n ++\t starts_with(uri, \"http:\"))\n +\t\treturn download_https_uri_to_file(filename, uri);\n +\n -+\tif (!skip_prefix(uri, \"file://\", &out))\n -+\t\tout = uri;\n ++\tif (skip_prefix(uri, \"file://\", &out))\n ++\t\turi = out;\n +\n +\t/* Copy as a file */\n -+\treturn copy_file(filename, out, 0);\n ++\treturn copy_file(filename, uri, 0);\n }\n \n static int unbundle_from_file(struct repository *r, const char *file)\n 5: acee1fae027 = 5: ed76d84c5a7 clone: --bundle-uri cannot be combined with --depth\n\n-- \ngitgitgadget\n"},{"id":"460906","messageId":"4df4a1d679a1003ccd3a4dd049bc05141831970c.1660050703.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v3.git.1660050703.gitgitgadget@gmail.com","subject":"[PATCH v3 1/5] remote-curl: add 'get' capability","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-09T13:11:39Z","receivedAt":"2022-08-09T13:11:55Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nA future change will want a way to download a file over HTTP(S) using\nthe simplest of download mechanisms. We do not want to assume that the\nserver on the other side understands anything about the Git protocol but\ncould be a simple static web server.\n\nCreate the new 'get' capability for the remote helpers which advertises\nthat the 'get' command is avalable. A caller can send a line containing\n'get ' to download the file at into the file at\n.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Documentation/gitremote-helpers.txt | 9 +++++++\n remote-curl.c | 28 ++++++++++++++++++++++\n t/t5557-http-get.sh | 37 +++++++++++++++++++++++++++++\n 3 files changed, 74 insertions(+)\n create mode 100755 t/t5557-http-get.sh\n\ndiff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\nindex 6f1e269ae43..ed8da428c98 100644\n--- a/Documentation/gitremote-helpers.txt\n+++ b/Documentation/gitremote-helpers.txt\n@@ -168,6 +168,9 @@ Supported commands: 'list', 'import'.\n \tCan guarantee that when a clone is requested, the received\n \tpack is self contained and is connected.\n \n+'get'::\n+\tCan use the 'get' command to download a file from a given URI.\n+\n If a helper advertises 'connect', Git will use it if possible and\n fall back to another capability if the helper requests so when\n connecting (see the 'connect' command under COMMANDS).\n@@ -418,6 +421,12 @@ Supported if the helper has the \"connect\" capability.\n +\n Supported if the helper has the \"stateless-connect\" capability.\n \n+'get' ::\n+\tDownloads the file from the given `` to the given ``. If\n+\t`.temp` exists, then Git assumes that the `.temp` file is a\n+\tpartial download from a previous attempt and will resume the\n+\tdownload from that position.\n+\n If a fatal error occurs, the program writes the error message to\n stderr and exits. The caller should expect that a suitable error\n message has been printed if the child closes the connection without\ndiff --git a/remote-curl.c b/remote-curl.c\nindex b8758757ece..72dfb8fb86a 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1286,6 +1286,29 @@ static void parse_fetch(struct strbuf *buf)\n \tstrbuf_reset(buf);\n }\n \n+static void parse_get(const char *arg)\n+{\n+\tstruct strbuf url = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tconst char *space;\n+\n+\tspace = strchr(arg, ' ');\n+\n+\tif (!space)\n+\t\tdie(_(\"protocol error: expected ' ', missing space\"));\n+\n+\tstrbuf_add(&url, arg, space - arg);\n+\tstrbuf_addstr(&path, space + 1);\n+\n+\tif (http_get_file(url.buf, path.buf, NULL))\n+\t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n+\n+\tstrbuf_release(&url);\n+\tstrbuf_release(&path);\n+\tprintf(\"\\n\");\n+\tfflush(stdout);\n+}\n+\n static int push_dav(int nr_spec, const char **specs)\n {\n \tstruct child_process child = CHILD_PROCESS_INIT;\n@@ -1564,9 +1587,14 @@ int cmd_main(int argc, const char **argv)\n \t\t\t\tprintf(\"unsupported\\n\");\n \t\t\tfflush(stdout);\n \n+\t\t} else if (skip_prefix(buf.buf, \"get \", &arg)) {\n+\t\t\tparse_get(arg);\n+\t\t\tfflush(stdout);\n+\n \t\t} else if (!strcmp(buf.buf, \"capabilities\")) {\n \t\t\tprintf(\"stateless-connect\\n\");\n \t\t\tprintf(\"fetch\\n\");\n+\t\t\tprintf(\"get\\n\");\n \t\t\tprintf(\"option\\n\");\n \t\t\tprintf(\"push\\n\");\n \t\t\tprintf(\"check-connectivity\\n\");\ndiff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\nnew file mode 100755\nindex 00000000000..09fb0bd9a8a\n--- /dev/null\n+++ b/t/t5557-http-get.sh\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+\n+test_description='test downloading a file by URL'\n+\n+. ./test-lib.sh\n+\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'get by URL: 404' '\n+\ttest_when_finished \"rm -f file.temp\" &&\n+\turl=\"$HTTPD_URL/none.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\tget $url file1\n+\tEOF\n+\n+\ttest_must_fail git remote-http $url err &&\n+\ttest_path_is_missing file1 &&\n+\tgrep \"failed to download file at URL\" err\n+'\n+\n+test_expect_success 'get by URL: 200' '\n+\techo data >\"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" &&\n+\n+\turl=\"$HTTPD_URL/exists.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\tget $url file2\n+\n+\tEOF\n+\n+\tgit remote-http $url \n\nBefore implementing a way to fetch bundles into a repository, create the\nbasic logic. Assume that the URI is actually a file path. Future logic\nwill make this more careful to other protocols.\n\nFor now, we also only succeed if the content at the URI is a bundle\nfile, not a bundle list. Bundle lists will be implemented in a future\nchange.\n\nNote that the discovery of a temporary filename is slightly racy because\nthe odb_mkstemp() relies on the temporary file not existing. With the\ncurrent implementation being limited to file copies, we could replace\nthe copy_file() with copy_fd(). The tricky part comes in future changes\nthat send the filename to 'git remote-https' and its 'get' capability.\nAt that point, we need the file descriptor closed _and_ the file\nunlinked. If we were to keep the file descriptor open for the sake of\nnormal file copies, then we would pollute the rest of the code for\nlittle benefit. This is especially the case because we expect that most\nbundle URI use will be based on HTTPS instead of file copies.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Makefile | 1 +\n bundle-uri.c | 104 ++++++++++++++++++++++++++++++++++++++++++++\n bundle-uri.h | 14 ++++++\n t/t5557-http-get.sh | 2 +\n 4 files changed, 121 insertions(+)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n\ndiff --git a/Makefile b/Makefile\nindex 1624471badc..7d5f48069ea 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -906,6 +906,7 @@ LIB_OBJS += blob.o\n LIB_OBJS += bloom.o\n LIB_OBJS += branch.o\n LIB_OBJS += bulk-checkin.o\n+LIB_OBJS += bundle-uri.o\n LIB_OBJS += bundle.o\n LIB_OBJS += cache-tree.o\n LIB_OBJS += cbtree.o\ndiff --git a/bundle-uri.c b/bundle-uri.c\nnew file mode 100644\nindex 00000000000..b35babc36aa\n--- /dev/null\n+++ b/bundle-uri.c\n@@ -0,0 +1,104 @@\n+#include \"cache.h\"\n+#include \"bundle-uri.h\"\n+#include \"bundle.h\"\n+#include \"object-store.h\"\n+#include \"refs.h\"\n+#include \"run-command.h\"\n+\n+static int find_temp_filename(struct strbuf *name)\n+{\n+\tint fd;\n+\t/*\n+\t * Find a temporary filename that is available. This is briefly\n+\t * racy, but unlikely to collide.\n+\t */\n+\tfd = odb_mkstemp(name, \"bundles/tmp_uri_XXXXXX\");\n+\tif (fd < 0) {\n+\t\twarning(_(\"failed to create temporary file\"));\n+\t\treturn -1;\n+\t}\n+\n+\tclose(fd);\n+\tunlink(name->buf);\n+\treturn 0;\n+}\n+\n+static int copy_uri_to_file(const char *file, const char *uri)\n+{\n+\t/* File-based URIs only for now. */\n+\treturn copy_file(file, uri, 0);\n+}\n+\n+static int unbundle_from_file(struct repository *r, const char *file)\n+{\n+\tint result = 0;\n+\tint bundle_fd;\n+\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n+\tstruct string_list_item *refname;\n+\tstruct strbuf bundle_ref = STRBUF_INIT;\n+\tsize_t bundle_prefix_len;\n+\n+\tif ((bundle_fd = read_bundle_header(file, &header)) < 0)\n+\t\treturn 1;\n+\n+\tif ((result = unbundle(r, &header, bundle_fd, NULL)))\n+\t\treturn 1;\n+\n+\t/*\n+\t * Convert all refs/heads/ from the bundle into refs/bundles/\n+\t * in the local repository.\n+\t */\n+\tstrbuf_addstr(&bundle_ref, \"refs/bundles/\");\n+\tbundle_prefix_len = bundle_ref.len;\n+\n+\tfor_each_string_list_item(refname, &header.references) {\n+\t\tstruct object_id *oid = refname->util;\n+\t\tstruct object_id old_oid;\n+\t\tconst char *branch_name;\n+\t\tint has_old;\n+\n+\t\tif (!skip_prefix(refname->string, \"refs/heads/\", &branch_name))\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_setlen(&bundle_ref, bundle_prefix_len);\n+\t\tstrbuf_addstr(&bundle_ref, branch_name);\n+\n+\t\thas_old = !read_ref(bundle_ref.buf, &old_oid);\n+\t\tupdate_ref(\"fetched bundle\", bundle_ref.buf, oid,\n+\t\t\t has_old ? &old_oid : NULL,\n+\t\t\t REF_SKIP_OID_VERIFICATION,\n+\t\t\t UPDATE_REFS_MSG_ON_ERR);\n+\t}\n+\n+\tbundle_header_release(&header);\n+\treturn result;\n+}\n+\n+int fetch_bundle_uri(struct repository *r, const char *uri)\n+{\n+\tint result = 0;\n+\tstruct strbuf filename = STRBUF_INIT;\n+\n+\tif ((result = find_temp_filename(&filename)))\n+\t\tgoto cleanup;\n+\n+\tif ((result = copy_uri_to_file(filename.buf, uri))) {\n+\t\twarning(_(\"failed to download bundle from URI '%s'\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif ((result = !is_bundle(filename.buf, 0))) {\n+\t\twarning(_(\"file at URI '%s' is not a bundle\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif ((result = unbundle_from_file(r, filename.buf))) {\n+\t\twarning(_(\"failed to unbundle bundle from URI '%s'\"), uri);\n+\t\tgoto cleanup;\n+\t}\n+\n+cleanup:\n+\tunlink(filename.buf);\n+\tstrbuf_release(&filename);\n+\treturn result;\n+}\ndiff --git a/bundle-uri.h b/bundle-uri.h\nnew file mode 100644\nindex 00000000000..8a152f1ef14\n--- /dev/null\n+++ b/bundle-uri.h\n@@ -0,0 +1,14 @@\n+#ifndef BUNDLE_URI_H\n+#define BUNDLE_URI_H\n+\n+struct repository;\n+\n+/**\n+ * Fetch data from the given 'uri' and unbundle the bundle data found\n+ * based on that information.\n+ *\n+ * Returns non-zero if no bundle information is found at the given 'uri'.\n+ */\n+int fetch_bundle_uri(struct repository *r, const char *uri);\n+\n+#endif\ndiff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\nindex 09fb0bd9a8a..76a4bbd16af 100755\n--- a/t/t5557-http-get.sh\n+++ b/t/t5557-http-get.sh\n@@ -2,6 +2,8 @@\n \n test_description='test downloading a file by URL'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n+\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n-- \ngitgitgadget\n\n"},{"id":"460908","messageId":"00debaf6e77852efe1dcad4bfda5ebd5bf590ac4.1660050704.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v3.git.1660050703.gitgitgadget@gmail.com","subject":"[PATCH v3 3/5] clone: add --bundle-uri option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-09T13:11:41Z","receivedAt":"2022-08-09T13:11:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nCloning a remote repository is one of the most expensive operations in\nGit. The server can spend a lot of CPU time generating a pack-file for\nthe client's request. The amount of data can clog the network for a long\ntime, and the Git protocol is not resumable. For users with poor network\nconnections or are located far away from the origin server, this can be\nespecially painful.\n\nAdd a new '--bundle-uri' option to 'git clone' to bootstrap a clone from\na bundle. If the user is aware of a bundle server, then they can tell\nGit to bootstrap the new repository with these bundles before fetching\nthe remaining objects from the origin server.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Documentation/git-clone.txt | 6 ++++++\n builtin/clone.c | 15 +++++++++++++++\n t/t5558-clone-bundle-uri.sh | 36 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+)\n create mode 100755 t/t5558-clone-bundle-uri.sh\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 632bd1348ea..60fedf7eb5e 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -323,6 +323,12 @@ or `--mirror` is given)\n \tfor `host.xz:foo/.git`). Cloning into an existing directory\n \tis only allowed if the directory is empty.\n \n+--bundle-uri=::\n+\tBefore fetching from the remote, fetch a bundle from the given\n+\t`` and unbundle the data into the local repository. The refs\n+\tin the bundle will be stored under the hidden `refs/bundle/*`\n+\tnamespace.\n+\n :git-clone: 1\n include::urls.txt[]\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex c4ff4643ecd..4224d562758 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -34,6 +34,7 @@\n #include \"list-objects-filter-options.h\"\n #include \"hook.h\"\n #include \"bundle.h\"\n+#include \"bundle-uri.h\"\n \n /*\n * Overall FIXMEs:\n@@ -77,6 +78,7 @@ static int option_filter_submodules = -1; /* unspecified */\n static int config_filter_submodules = -1; /* unspecified */\n static struct string_list server_options = STRING_LIST_INIT_NODUP;\n static int option_remote_submodules;\n+static const char *bundle_uri;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -160,6 +162,8 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"any cloned submodules will use their remote-tracking branch\")),\n \tOPT_BOOL(0, \"sparse\", &option_sparse_checkout,\n \t\t N_(\"initialize sparse-checkout file to include only files at root\")),\n+\tOPT_STRING(0, \"bundle-uri\", &bundle_uri,\n+\t\t N_(\"uri\"), N_(\"a URI for downloading bundles before fetching from origin remote\")),\n \tOPT_END()\n };\n \n@@ -1232,6 +1236,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (transport->smart_options && !deepen && !filter_options.choice)\n \t\ttransport->smart_options->check_self_contained_and_connected = 1;\n \n+\t/*\n+\t * Before fetching from the remote, download and install bundle\n+\t * data from the --bundle-uri option.\n+\t */\n+\tif (bundle_uri) {\n+\t\t/* At this point, we need the_repository to match the cloned repo. */\n+\t\trepo_init(the_repository, git_dir, work_tree);\n+\t\tif (fetch_bundle_uri(the_repository, bundle_uri))\n+\t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n+\t\t\t\tbundle_uri);\n+\t}\n \n \tstrvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n \trefspec_ref_prefixes(&remote->fetch,\ndiff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh\nnew file mode 100755\nindex 00000000000..f709bcb729c\n--- /dev/null\n+++ b/t/t5558-clone-bundle-uri.sh\n@@ -0,0 +1,36 @@\n+#!/bin/sh\n+\n+test_description='test fetching bundles with --bundle-uri'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'fail to clone from non-existent file' '\n+\ttest_when_finished rm -rf test &&\n+\tgit clone --bundle-uri=\"$(pwd)/does-not-exist\" . test 2>err &&\n+\tgrep \"failed to download bundle from URI\" err\n+'\n+\n+test_expect_success 'fail to clone from non-bundle file' '\n+\ttest_when_finished rm -rf test &&\n+\techo bogus >bogus &&\n+\tgit clone --bundle-uri=\"$(pwd)/bogus\" . test 2>err &&\n+\tgrep \"is not a bundle\" err\n+'\n+\n+test_expect_success 'create bundle' '\n+\tgit init clone-from &&\n+\tgit -C clone-from checkout -b topic &&\n+\ttest_commit -C clone-from A &&\n+\ttest_commit -C clone-from B &&\n+\tgit -C clone-from bundle create B.bundle topic\n+'\n+\n+test_expect_success 'clone with path bundle' '\n+\tgit clone --bundle-uri=\"clone-from/B.bundle\" \\\n+\t\tclone-from clone-path &&\n+\tgit -C clone-path rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"460909","messageId":"66a1ce4045192d153d5ac66cc248ac569cac8cf6.1660050704.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v3.git.1660050703.gitgitgadget@gmail.com","subject":"[PATCH v3 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-09T13:11:42Z","receivedAt":"2022-08-09T13:12:08Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe previous change created the 'git clone --bundle-uri=' option.\nCurrently, must be a filename.\n\nUpdate copy_uri_to_file() to first inspect the URI for an HTTP(S) prefix\nand use git-remote-https as the way to download the data at that URI.\nOtherwise, check to see if file:// is present and modify the prefix\naccordingly.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n bundle-uri.c | 70 +++++++++++++++++++++++++++++++++++--\n t/t5558-clone-bundle-uri.sh | 45 ++++++++++++++++++++++++\n 2 files changed, 112 insertions(+), 3 deletions(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex b35babc36aa..4a8cc74ed05 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -23,10 +23,74 @@ static int find_temp_filename(struct strbuf *name)\n \treturn 0;\n }\n \n-static int copy_uri_to_file(const char *file, const char *uri)\n+static int download_https_uri_to_file(const char *file, const char *uri)\n {\n-\t/* File-based URIs only for now. */\n-\treturn copy_file(file, uri, 0);\n+\tint result = 0;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tFILE *child_in = NULL, *child_out = NULL;\n+\tstruct strbuf line = STRBUF_INIT;\n+\tint found_get = 0;\n+\n+\tstrvec_pushl(&cp.args, \"git-remote-https\", uri, NULL);\n+\tcp.in = -1;\n+\tcp.out = -1;\n+\n+\tif (start_command(&cp))\n+\t\treturn 1;\n+\n+\tchild_in = fdopen(cp.in, \"w\");\n+\tif (!child_in) {\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tchild_out = fdopen(cp.out, \"r\");\n+\tif (!child_out) {\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tfprintf(child_in, \"capabilities\\n\");\n+\tfflush(child_in);\n+\n+\twhile (!strbuf_getline(&line, child_out)) {\n+\t\tif (!line.len)\n+\t\t\tbreak;\n+\t\tif (!strcmp(line.buf, \"get\"))\n+\t\t\tfound_get = 1;\n+\t}\n+\tstrbuf_release(&line);\n+\n+\tif (!found_get) {\n+\t\tresult = error(_(\"insufficient capabilities\"));\n+\t\tgoto cleanup;\n+\t}\n+\n+\tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n+\n+cleanup:\n+\tif (child_in)\n+\t\tfclose(child_in);\n+\tif (finish_command(&cp))\n+\t\treturn 1;\n+\tif (child_out)\n+\t\tfclose(child_out);\n+\treturn result;\n+}\n+\n+static int copy_uri_to_file(const char *filename, const char *uri)\n+{\n+\tconst char *out;\n+\n+\tif (starts_with(uri, \"https:\") ||\n+\t starts_with(uri, \"http:\"))\n+\t\treturn download_https_uri_to_file(filename, uri);\n+\n+\tif (skip_prefix(uri, \"file://\", &out))\n+\t\turi = out;\n+\n+\t/* Copy as a file */\n+\treturn copy_file(filename, uri, 0);\n }\n \n static int unbundle_from_file(struct repository *r, const char *file)\ndiff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh\nindex f709bcb729c..ad666a2d28a 100755\n--- a/t/t5558-clone-bundle-uri.sh\n+++ b/t/t5558-clone-bundle-uri.sh\n@@ -33,4 +33,49 @@ test_expect_success 'clone with path bundle' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'clone with file:// bundle' '\n+\tgit clone --bundle-uri=\"file://$(pwd)/clone-from/B.bundle\" \\\n+\t\tclone-from clone-file &&\n+\tgit -C clone-file rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+#########################################################################\n+# HTTP tests begin here\n+\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'fail to fetch from non-existent HTTP URL' '\n+\ttest_when_finished rm -rf test &&\n+\tgit clone --bundle-uri=\"$HTTPD_URL/does-not-exist\" . test 2>err &&\n+\tgrep \"failed to download bundle from URI\" err\n+'\n+\n+test_expect_success 'fail to fetch from non-bundle HTTP URL' '\n+\ttest_when_finished rm -rf test &&\n+\techo bogus >\"$HTTPD_DOCUMENT_ROOT_PATH/bogus\" &&\n+\tgit clone --bundle-uri=\"$HTTPD_URL/bogus\" . test 2>err &&\n+\tgrep \"is not a bundle\" err\n+'\n+\n+test_expect_success 'clone HTTP bundle' '\n+\tcp clone-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n+\n+\tgit clone --no-local --mirror clone-from \\\n+\t\t\"$HTTPD_DOCUMENT_ROOT_PATH/fetch.git\" &&\n+\n+\tgit clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n+\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http &&\n+\tgit -C clone-http rev-parse refs/bundles/topic >actual &&\n+\tgit -C clone-from rev-parse topic >expect &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_config -C clone-http log.excludedecoration refs/bundle/\n+'\n+\n+# Do not add tests here unless they use the HTTP server, as they will\n+# not run unless the HTTP dependencies exist.\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"460910","messageId":"ed76d84c5a7def525ab39a8acecdac8bad12c54b.1660050704.git.gitgitgadget@gmail.com","threadId":"58220","inReplyTo":"pull.1300.v3.git.1660050703.gitgitgadget@gmail.com","subject":"[PATCH v3 5/5] clone: --bundle-uri cannot be combined with --depth","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-09T13:11:43Z","receivedAt":"2022-08-09T13:12:10Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nA previous change added the '--bundle-uri' option, but did not check\nif the --depth parameter was included. Since bundles are not compatible\nwith shallow clones, provide an error message to the user who is\nattempting this combination.\n\nI am leaving this as its own change, separate from the one that\nimplements '--bundle-uri', because this is more of an advisory for the\nuser. There is nothing wrong with bootstrapping with bundles and then\nfetching a shallow clone. However, that is likely going to involve too\nmuch work for the client _and_ the server. The client will download all\nof this bundle information containing the full history of the\nrepository only to ignore most of it. The server will get a shallow\nfetch request, but with a list of haves that might cause a more painful\ncomputation of that shallow pack-file.\n\nReviewed-by: Josh Steadmon \nSigned-off-by: Derrick Stolee \n---\n Documentation/git-clone.txt | 3 ++-\n builtin/clone.c | 3 +++\n t/t5606-clone-options.sh | 8 ++++++++\n 3 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 60fedf7eb5e..d032d971dd7 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -327,7 +327,8 @@ or `--mirror` is given)\n \tBefore fetching from the remote, fetch a bundle from the given\n \t`` and unbundle the data into the local repository. The refs\n \tin the bundle will be stored under the hidden `refs/bundle/*`\n-\tnamespace.\n+\tnamespace. This option is incompatible with `--depth`,\n+\t`--shallow-since`, and `--shallow-exclude`.\n \n :git-clone: 1\n include::urls.txt[]\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 4224d562758..4463789680b 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -937,6 +937,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_no_checkout = 1;\n \t}\n \n+\tif (bundle_uri && deepen)\n+\t\tdie(_(\"--bundle-uri is incompatible with --depth, --shallow-since, and --shallow-exclude\"));\n+\n \trepo_name = argv[0];\n \n \tpath = get_repo_path(repo_name, &is_bundle);\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 8f676d6b0c0..f6bb02ab947 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -58,6 +58,14 @@ test_expect_success 'disallows --bare with --separate-git-dir' '\n \n '\n \n+test_expect_success 'disallows --bundle-uri with shallow options' '\n+\tfor option in --depth=1 --shallow-since=01-01-2000 --shallow-exclude=HEAD\n+\tdo\n+\t\ttest_must_fail git clone --bundle-uri=bundle $option from to 2>err &&\n+\t\tgrep \"bundle-uri is incompatible\" err || return 1\n+\tdone\n+'\n+\n test_expect_success 'reject cloning shallow repository' '\n \ttest_when_finished \"rm -rf repo\" &&\n \ttest_must_fail git clone --reject-shallow shallow-repo out 2>err &&\n-- \ngitgitgadget\n"},{"id":"461813","messageId":"xmqq8rngas5q.fsf@gitster.g","threadId":"58220","inReplyTo":"00debaf6e77852efe1dcad4bfda5ebd5bf590ac4.1660050704.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 3/5] clone: add --bundle-uri option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-22T21:24:01Z","receivedAt":"2022-08-22T21:24:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> +\t/*\n> +\t * Before fetching from the remote, download and install bundle\n> +\t * data from the --bundle-uri option.\n> +\t */\n> +\tif (bundle_uri) {\n> +\t\t/* At this point, we need the_repository to match the cloned repo. */\n> +\t\trepo_init(the_repository, git_dir, work_tree);\n> +\t\tif (fetch_bundle_uri(the_repository, bundle_uri))\n> +\t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n> +\t\t\t\tbundle_uri);\n> +\t}\n\nI do not offhand know why I suddenly started seeing the issue for\nthis relatively old commit I have had in my tree for at least 10\ndays, but I am getting this\n\nbuiltin/clone.c: In function 'cmd_clone':\nbuiltin/clone.c:1248:17: error: ignoring return value of 'repo_init' declared with attribute 'warn_unused_result' [-Werror=unused-result]\n 1248 | repo_init(the_repository, git_dir, work_tree);\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\n\nwith the commit merged in 'seen'.\n\nIt seems that the updated ab/submodule-helper-prep is doing it. \n\nWhy can't that (or any of Ævar's) topic focus on what it needs to\ndo, without churning the codebase and inflict damages to other\ntopics like this? Quite frustrating.\n\nI'll redo today's integration without the offending topic to give\nother topics test coverage.\n\n\n"},{"id":"461857","messageId":"5229519b-3af2-a023-8996-43343b130722@github.com","threadId":"58220","inReplyTo":"xmqq8rngas5q.fsf@gitster.g","subject":"Re: [PATCH v3 3/5] clone: add --bundle-uri option","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-08-23T14:05:13Z","receivedAt":"2022-08-23T17:07:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/22/2022 5:24 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> +\t/*\n>> +\t * Before fetching from the remote, download and install bundle\n>> +\t * data from the --bundle-uri option.\n>> +\t */\n>> +\tif (bundle_uri) {\n>> +\t\t/* At this point, we need the_repository to match the cloned repo. */\n>> +\t\trepo_init(the_repository, git_dir, work_tree);\n>> +\t\tif (fetch_bundle_uri(the_repository, bundle_uri))\n>> +\t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n>> +\t\t\t\tbundle_uri);\n>> +\t}\n> \n> I do not offhand know why I suddenly started seeing the issue for\n> this relatively old commit I have had in my tree for at least 10\n> days, but I am getting this\n> \n> builtin/clone.c: In function 'cmd_clone':\n> builtin/clone.c:1248:17: error: ignoring return value of 'repo_init' declared with attribute 'warn_unused_result' [-Werror=unused-result]\n> 1248 | repo_init(the_repository, git_dir, work_tree);\n> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> \n> \n> with the commit merged in 'seen'.\n\nThank you for pointing this out. Here is a patch that fixes\nthe issue from this patch:\n\n--- >8 ---\n\nFrom 6df8bc6d7ffdc1b115d85ef9550bab5dcf1811f8 Mon Sep 17 00:00:00 2001\nFrom: Derrick Stolee \nDate: Tue, 23 Aug 2022 09:53:47 -0400\nSubject: [PATCH] clone: warn on failure to repo_init()\n\nThe --bundle-uri option was added in 55568919616 (clone: add\n--bundle-uri option, 2022-08-09), but this also introduced a call to\nrepo_init() whose return value was ignored. Fix that ignored value by\nwarning that the bundle URI process could not continue if it failed.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/clone.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 4463789680b..e21d42dfee5 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1245,8 +1245,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t */\n \tif (bundle_uri) {\n \t\t/* At this point, we need the_repository to match the cloned repo. */\n-\t\trepo_init(the_repository, git_dir, work_tree);\n-\t\tif (fetch_bundle_uri(the_repository, bundle_uri))\n+\t\tif (repo_init(the_repository, git_dir, work_tree))\n+\t\t\twarning(_(\"failed to initialize the repo, skipping bundle URI\"));\n+\t\telse if (fetch_bundle_uri(the_repository, bundle_uri))\n \t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n \t\t\t\tbundle_uri);\n \t}\n-- \n2.37.1.vfs.0.0.rebase\n\n\n"},{"id":"461898","messageId":"xmqq1qt5ablm.fsf@gitster.g","threadId":"58220","inReplyTo":"5229519b-3af2-a023-8996-43343b130722@github.com","subject":"Re: [PATCH v3 3/5] clone: add --bundle-uri option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-24T15:46:13Z","receivedAt":"2022-08-24T15:48:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> From: Derrick Stolee \n> Date: Tue, 23 Aug 2022 09:53:47 -0400\n> Subject: [PATCH] clone: warn on failure to repo_init()\n>\n> The --bundle-uri option was added in 55568919616 (clone: add\n> --bundle-uri option, 2022-08-09), but this also introduced a call to\n> repo_init() whose return value was ignored. Fix that ignored value by\n> warning that the bundle URI process could not continue if it failed.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n\nOK. It looks like a sensible thing to do.\n\n> builtin/clone.c | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 4463789680b..e21d42dfee5 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -1245,8 +1245,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \t */\n> \tif (bundle_uri) {\n> \t\t/* At this point, we need the_repository to match the cloned repo. */\n> -\t\trepo_init(the_repository, git_dir, work_tree);\n> -\t\tif (fetch_bundle_uri(the_repository, bundle_uri))\n> +\t\tif (repo_init(the_repository, git_dir, work_tree))\n> +\t\t\twarning(_(\"failed to initialize the repo, skipping bundle URI\"));\n> +\t\telse if (fetch_bundle_uri(the_repository, bundle_uri))\n> \t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n> \t\t\t\tbundle_uri);\n> \t}\n"},{"id":"462066","messageId":"20220829045837.49659-1-tenglong.tl@alibaba-inc.com","threadId":"58220","inReplyTo":"66a1ce4045192d153d5ac66cc248ac569cac8cf6.1660050704.git.gitgitgadget@gmail.com","subject":"[PATCH v3 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-08-29T04:58:37Z","receivedAt":"2022-08-29T04:58:54Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Derrick Stolee writes:\n\n> +\twhile (!strbuf_getline(&line, child_out)) {\n> +\t\tif (!line.len)\n> +\t\t\tbreak;\n> +\t\tif (!strcmp(line.buf, \"get\"))\n> +\t\t\tfound_get = 1;\n> +\t}\n\nClear implementation for me to read, thanks.\n\nI'm not sure but maybe a nit, should it be a \"break;\" after\n\"found_get = 1;\" for omitting the left uncencerned capabilities\nto quit earlier?\n\nThanks.\n"},{"id":"462190","messageId":"dcc46c8a-4c49-05d4-db23-0497e08f9fed@github.com","threadId":"58220","inReplyTo":"20220829045837.49659-1-tenglong.tl@alibaba-inc.com","subject":"Re: [PATCH v3 4/5] bundle-uri: add support for http(s):// and file://","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-08-30T13:33:28Z","receivedAt":"2022-08-30T13:33:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/29/2022 12:58 AM, Teng Long wrote:\n> Derrick Stolee writes:\n> \n>> +\twhile (!strbuf_getline(&line, child_out)) {\n>> +\t\tif (!line.len)\n>> +\t\t\tbreak;\n>> +\t\tif (!strcmp(line.buf, \"get\"))\n>> +\t\t\tfound_get = 1;\n>> +\t}\n> \n> Clear implementation for me to read, thanks.\n> \n> I'm not sure but maybe a nit, should it be a \"break;\" after\n> \"found_get = 1;\" for omitting the left uncencerned capabilities\n> to quit earlier?\n\nTeng, you are right that it does not hurt to have a 'break'\nhere. Since this topic has already merged, that change will\nneed a forward-fix that I can add to a future series.\n\nThanks,\n-Stolee\n"}]}