{"thread":{"id":"57465","subject":"[PATCH 00/25] [RFC] Bundle URIs","startedAt":"2022-02-23T18:31:09Z","lastAt":"2022-04-08T18:26:56Z","messageCount":46,"participants":["Derrick Stolee via GitGitGadget","Ævar Arnfjörð Bjarmason via GitGitGadget","Ævar Arnfjörð Bjarmason","Derrick Stolee","Elijah Newren","Teng Long","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":25},"messages":[{"id":"449303","messageId":"pull.1160.git.1645641063.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":null,"subject":"[PATCH 00/25] [RFC] Bundle URIs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:38Z","receivedAt":"2022-02-23T18:31:09Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"There have been several suggestions to improve Git clone speeds and\nreliability by supplementing the Git protocol with static content. The\nPackfile URI [0] feature lets the Git response include URIs that point to\npackfiles that the client must download to complete the request.\n\nLast year, Ævar suggested using bundles instead of packfiles [1] [2]. This\ndesign has the same benefits to the packfile URI feature because it offloads\nmost object downloads to static content fetches. The main advantage over\npackfile URIs is that the remote Git server does not need to know what is in\nthose bundles. The Git client tells the server what it downloaded during the\nfetch negotiation afterwards. This includes any chance that the client did\nnot have access to those bundles or otherwise failed to access them. I\nagreed that this was a much more desirable way to serve static content, but\nhad concerns about the flexibility of that design [3]. I have not heard more\non the topic since October, so I started investigating this idea myself in\nDecember, resulting in this RFC.\n\nI focused on maximizing flexibility for the service that organizes and\nserves bundles. This includes:\n\n * Bundle URIs work for full and partial clones.\n\n * Bundle URIs can assist with git fetch in addition to git clone.\n\n * Users can set up bundle servers independent of the remote Git server if\n they specify the bundle URI via a --bundle-uri argument.\n\nThis series is based on the recently-submitted series that adds object\nfilters to bundles [4]. There is a slight adjacent-line-add conflict with\njs/apply-partial-clone-filters-recursively, but that is in the last few\npatches, so it will be easy to rebase by the time we have a fully-reviewable\npatch series for those steps.\n\nThe general breakdown is as follows:\n\n * Patch 1 adds documentation for the feature in its entirety.\n\n * Patches 2-14 add the ability to run ‘git clone --bundle-uri=’\n\n * Patches 15-17 add bundle fetches to ‘git fetch’ calls\n\n * Patches 18-25 add a new ‘features’ capability that allows a server to\n advertise bundle URIs (and in the future, other features).\n\nI consider the patches in their current form to be “RFC quality”. There are\nmultiple places where tests are missing or special cases are not checked.\nThe goal for this RFC is to seek feedback on the high-level ideas before\ncommitting to the deep work of creating mergeable patches.\n\n\nTesting with this series\n========================\n\nTo get a full test of the features being proposed here, I created an MVP of\na bundle server by pushing bundle data to some publicly-readable Azure\nStorage accounts. These bundle servers mirror the following public\nrepositories on GitHub:\n\n * git/git\n * git-for-windows/git\n * homebrew/homebrew-core\n * cocoapods/specs\n * torvalds/linux\n\nIn addition, the Azure Storage accounts are available in different regions:\n\n * East US: https://gitbundleserver.z13.web.core.windows.net\n * West US: https://gitbundleserverwestus.z22.web.core.windows.net\n * Europe: https://gitbundleservereurope.z6.web.core.windows.net\n * East Asia: https://gitbundleservereastasia.z7.web.core.windows.net\n * South Asia: https://gitbundleserversouthasia.z23.web.core.windows.net\n * Australia: https://gitbundleserveraustralia.z26.web.core.windows.net\n\nTo test this RFC against these servers, choose your $org/$repo to clone and\nyour region's bundle server $url and run\n\n$ git clone --bundle-uri=$url/$org/$repo/ https://github.com/$org/$repo\n\n\nNote that these servers are set up using \"almost free\" storage in Azure.\nNetwork connectivity of this storage can be slower than that of GitHub data\ncenters, so your results may vary.\n\nFrom my location in Raleigh, NC, USA, I am able to clone torvalds/linux from\nthe bundle servers quite a bit faster than from GitHub:\n\n@derrickstolee With Bundles Without Bundles Full Clone 491.9s 964.9s Partial\nClone 132.3s 171.7s\n\nI recruited GitHub employees from across the globe to test this experimental\ndeployment of bundle servers and found mixed results. Some users had better\nperformance with bundle servers, and many had very close results. Some had\nmuch worse connections to bundle servers than the GitHub remote, because of\nthe use of the cheapest form of Azure Storage services compared to highly\noptimized GitHub infrastructure.\n\nI plan to keep these servers running for a while, so this test should be\npossible throughout the review of this RFC and the patch series as they are\nreviewed. I'd love to hear if anyone else does this experiment and has\nanything to say about the results.\n\n\nImplementation Plan\n===================\n\nThe first patch contains a design document that is \"aspirational\": It\ndescribes the feature as it should be when the implementation is complete.\nMost of that implementation is included in this RFC, though there are some\nminor tweaks to the functionality I want to change or add before the patches\nare under full review. In particular, things are poorly tested and\nundocumented the further you look in the RFC.\n\nHere is a potential plan for splitting this RFC into digestible pieces that\ncan be reviewed in sequence:\n\n 0. Update the git bundle create command to take a --filter option, allowing\n bundles to store packfiles restricted to an object filter. This is\n necessary for using bundle URIs to benefit partial clones. This step was\n already submitted for full review [4]. These patches are based on those.\n\n 1. Integrate bundle URIs into git clone with a --bundle-uri option. This\n will include the full understanding of a table of contents, but will not\n integrate with git fetch or allow the server to advertise URIs.\n\n 2. Integrate bundle URIs into git fetch, triggered by config values that\n are set during git clone if the server indicates that the bundle\n strategy works for fetches.\n\n 3. Create a new \"recommended features\" capability in protocol v2 where the\n server can recommend features such as bundle URIs, partial clone, and\n sparse-checkout. These features will be extremely limited in scope and\n blocked by opt-in config options. The design for this portion could be\n replaced by a \"bundle-uri\" capability that only advertises bundle URIs\n and no other information.\n\n\nIntended Focus of this RFC\n==========================\n\nThis RFC is very large, and that's even with many patches not including full\ndocumentation or tests. These commits are not intended to be reviewed as if\nI intended to merge this as-is.\n\nOne thing this feature establishes is a new standard by which the Git client\nwill communicate with external servers. The goal of this RFC is to determine\nif this standard is well designed, and whether we need to make it more\nrobust. Alternatively, the design might need to be changed for reasons I\ncannot predict.\n\nFor that reason, hopefully most of the feedback is directly on the first\npatch, which contains the design document. In particular, the design\ndocument repeats the implementation plan, and I'd like extra eyes on that,\ntoo.\n\n[0]\nhttps://github.com/git/git/blob/master/Documentation/technical/packfile-uri.txt\nThe packfile URI feature in Git (Created June 2020)\n\n[1]\nhttps://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\nAn earlier RFC for a bundle URI feature. (August 2021)\n\n[2]\nhttps://lore.kernel.org/git/cover-0.3-00000000000-20211025T211159Z-avarab@gmail.com/\nAn earlier patch series creating a 'bundle-uri' protocol v2 capability.\n(October 2021)\n\n[3]\nhttps://lore.kernel.org/git/e7fe220b-2877-107e-8f7e-ea507a65feff@gmail.com/\nMy earlier thoughts on the previous RFCs, many of which are integrated into\nthis RFC (August 2021)\n\n[4]\nhttps://lore.kernel.org/git/pull.1159.git.1645638911.gitgitgadget@gmail.com/\nAdd object filters to bundles (February 2022)\n\nThanks, -Stolee\n\nDerrick Stolee (24):\n docs: document bundle URI standard\n bundle: alphabetize subcommands better\n dir: extract starts_with_dot[_dot]_slash()\n remote: move relative_url()\n remote: allow relative_url() to return an absolute url\n http: make http_get_file() external\n remote-curl: add 'get' capability\n bundle: implement 'fetch' command for direct bundles\n bundle: parse table of contents during 'fetch'\n bundle: add --filter option to 'fetch'\n bundle: allow relative URLs in table of contents\n bundle: make it easy to call 'git bundle fetch'\n clone: add --bundle-uri option\n clone: --bundle-uri cannot be combined with --depth\n config: add git_config_get_timestamp()\n bundle: only fetch bundles if timestamp is new\n fetch: fetch bundles before fetching original data\n protocol-caps: implement cap_features()\n serve: understand but do not advertise 'features' capability\n serve: advertise 'features' when config exists\n connect: implement get_recommended_features()\n transport: add connections for 'features' capability\n clone: use server-recommended bundle URI\n t5601: basic bundle URI test\n\nÆvar Arnfjörð Bjarmason (1):\n connect.c: refactor sending of agent & object-format\n\n Documentation/gitremote-helpers.txt | 6 +\n Documentation/technical/bundle-uri.txt | 404 +++++++++++++++++++++\n builtin/bundle.c | 478 ++++++++++++++++++++++++-\n builtin/clone.c | 51 +++\n builtin/fetch.c | 17 +\n builtin/submodule--helper.c | 129 -------\n bundle.c | 21 ++\n bundle.h | 9 +\n config.c | 39 ++\n config.h | 14 +\n connect.c | 69 +++-\n dir.h | 11 +\n fsck.c | 14 +-\n http.c | 4 +-\n http.h | 9 +\n protocol-caps.c | 66 ++++\n protocol-caps.h | 1 +\n remote-curl.c | 32 ++\n remote.c | 104 ++++++\n remote.h | 35 ++\n serve.c | 23 ++\n t/t5601-clone.sh | 12 +\n t/t5701-git-serve.sh | 9 +\n transport-helper.c | 14 +\n transport-internal.h | 9 +\n transport.c | 38 ++\n transport.h | 5 +\n 27 files changed, 1467 insertions(+), 156 deletions(-)\n create mode 100644 Documentation/technical/bundle-uri.txt\n\n\nbase-commit: ec51d0a50e6e64ae37795d77f7d33204b9b71ecd\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1160%2Fderrickstolee%2Fbundle%2Frfc-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1160/derrickstolee/bundle/rfc-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1160\n-- \ngitgitgadget\n"},{"id":"449304","messageId":"a9bcb6065523e2d2307fdc5b7bc92b04aba012e8.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 02/25] bundle: alphabetize subcommands better","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:40Z","receivedAt":"2022-02-23T18:31:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe usage strings for the 'git bundle' subcommands are not alphabetical.\nThis also applies to their inspection within cmd_bundle(). Fix this\nordering before we insert a new subcommand.\n\nThis change does not reorder the cmd_bundle_*() methods to avoid moving\nlines that are more likely wanted in a future 'git blame' call. It is\nfine that those longer methods are not ordered alphabetically.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/bundle.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 5a85d7cd0fe..8187b7df739 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -13,9 +13,9 @@\n \n static const char * const builtin_bundle_usage[] = {\n N_(\"git bundle create [] \"),\n- N_(\"git bundle verify [] \"),\n N_(\"git bundle list-heads [...]\"),\n N_(\"git bundle unbundle [...]\"),\n+ N_(\"git bundle verify [] \"),\n NULL\n };\n \n@@ -24,11 +24,6 @@ static const char * const builtin_bundle_create_usage[] = {\n NULL\n };\n \n-static const char * const builtin_bundle_verify_usage[] = {\n- N_(\"git bundle verify [] \"),\n- NULL\n-};\n-\n static const char * const builtin_bundle_list_heads_usage[] = {\n N_(\"git bundle list-heads [...]\"),\n NULL\n@@ -39,6 +34,11 @@ static const char * const builtin_bundle_unbundle_usage[] = {\n NULL\n };\n \n+static const char * const builtin_bundle_verify_usage[] = {\n+ N_(\"git bundle verify [] \"),\n+ NULL\n+};\n+\n static int parse_options_cmd_bundle(int argc,\n \t\tconst char **argv,\n \t\tconst char* prefix,\n@@ -209,12 +209,12 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)\n \n \telse if (!strcmp(argv[0], \"create\"))\n \t\tresult = cmd_bundle_create(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"verify\"))\n-\t\tresult = cmd_bundle_verify(argc, argv, prefix);\n \telse if (!strcmp(argv[0], \"list-heads\"))\n \t\tresult = cmd_bundle_list_heads(argc, argv, prefix);\n \telse if (!strcmp(argv[0], \"unbundle\"))\n \t\tresult = cmd_bundle_unbundle(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"verify\"))\n+\t\tresult = cmd_bundle_verify(argc, argv, prefix);\n \telse {\n \t\terror(_(\"Unknown subcommand: %s\"), argv[0]);\n \t\tusage_with_options(builtin_bundle_usage, options);\n-- \ngitgitgadget\n\n"},{"id":"449305","messageId":"60a8d52af64cdc3ca8b374c714cd3af1fc74f5ec.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 05/25] remote: allow relative_url() to return an absolute url","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:43Z","receivedAt":"2022-02-23T18:31:19Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen the 'url' parameter was absolute, the previous implementation would\nconcatenate 'remote_url' with 'url'. Instead, we want to return 'url' in\nthis case.\n\nSigned-off-by: Derrick Stolee \n---\n remote.c | 12 ++++++++++--\n remote.h | 1 +\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex c4a56749e85..ac1d98ae922 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2766,10 +2766,18 @@ char *relative_url(const char *remote_url,\n \tint is_relative = 0;\n \tint colonsep = 0;\n \tchar *out;\n-\tchar *remoteurl = xstrdup(remote_url);\n+\tchar *remoteurl;\n \tstruct strbuf sb = STRBUF_INIT;\n-\tsize_t len = strlen(remoteurl);\n+\tsize_t len;\n+\n+\tif (!url_is_local_not_ssh(url) || is_absolute_path(url))\n+\t\treturn xstrdup(url);\n+\n+\tlen = strlen(remote_url);\n+\tif (!len)\n+\t\tBUG(\"invalid empty remote_url\");\n \n+\tremoteurl = xstrdup(remote_url);\n \tif (is_dir_sep(remoteurl[len-1]))\n \t\tremoteurl[len-1] = '\\0';\n \ndiff --git a/remote.h b/remote.h\nindex 91c7f187863..438152ef562 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -434,6 +434,7 @@ void apply_push_cas(struct push_cas_option *, struct remote *, struct ref *);\n * http://a.com/b ../../../c http:/c error out\n * http://a.com/b ../../../../c http:c error out\n * http://a.com/b ../../../../../c .:c error out\n+ * http://a.com/b http://d.org/e http://d.org/e as is\n */\n char *relative_url(const char *remote_url,\n \t\t const char *url,\n-- \ngitgitgadget\n\n"},{"id":"449306","messageId":"fcd51a650217c4344e2ac79b53f06bc3eca3adb3.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 03/25] dir: extract starts_with_dot[_dot]_slash()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:41Z","receivedAt":"2022-02-23T18:31:20Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWe will want to use this logic to assist checking if paths are absolute\nor relative, so extract it into a helpful place. This creates a\ncollision with similar methods in builtin/fsck.c, but those methods have\nimportant differences. Prepend \"fsck_\" to those methods to emphasize\nthat they are custom to the fsck builtin.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/submodule--helper.c | 10 ----------\n dir.h | 11 +++++++++++\n fsck.c | 14 +++++++-------\n 3 files changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c5d3fc3817f..c17dde4170f 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -70,16 +70,6 @@ static int print_default_remote(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static int starts_with_dot_slash(const char *str)\n-{\n-\treturn str[0] == '.' && is_dir_sep(str[1]);\n-}\n-\n-static int starts_with_dot_dot_slash(const char *str)\n-{\n-\treturn str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);\n-}\n-\n /*\n * Returns 1 if it was the last chop before ':'.\n */\ndiff --git a/dir.h b/dir.h\nindex 8e02dfb505d..5e38d1ba536 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -578,4 +578,15 @@ void connect_work_tree_and_git_dir(const char *work_tree,\n void relocate_gitdir(const char *path,\n \t\t const char *old_git_dir,\n \t\t const char *new_git_dir);\n+\n+static inline int starts_with_dot_slash(const char *str)\n+{\n+\treturn str[0] == '.' && is_dir_sep(str[1]);\n+}\n+\n+static inline int starts_with_dot_dot_slash(const char *str)\n+{\n+\treturn str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);\n+}\n+\n #endif\ndiff --git a/fsck.c b/fsck.c\nindex 3ec500d707a..32cd3bc081f 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -976,31 +976,31 @@ done:\n }\n \n /*\n- * Like builtin/submodule--helper.c's starts_with_dot_slash, but without\n+ * Like dir.h's starts_with_dot_slash, but without\n * relying on the platform-dependent is_dir_sep helper.\n *\n * This is for use in checking whether a submodule URL is interpreted as\n * relative to the current directory on any platform, since \\ is a\n * directory separator on Windows but not on other platforms.\n */\n-static int starts_with_dot_slash(const char *str)\n+static int fsck_starts_with_dot_slash(const char *str)\n {\n \treturn str[0] == '.' && (str[1] == '/' || str[1] == '\\\\');\n }\n \n /*\n- * Like starts_with_dot_slash, this is a variant of submodule--helper's\n- * helper of the same name with the twist that it accepts backslash as a\n+ * Like fsck_starts_with_dot_slash, this is a variant of dir.h's\n+ * helper with the twist that it accepts backslash as a\n * directory separator even on non-Windows platforms.\n */\n-static int starts_with_dot_dot_slash(const char *str)\n+static int fsck_starts_with_dot_dot_slash(const char *str)\n {\n-\treturn str[0] == '.' && starts_with_dot_slash(str + 1);\n+\treturn str[0] == '.' && fsck_starts_with_dot_slash(str + 1);\n }\n \n static int submodule_url_is_relative(const char *url)\n {\n-\treturn starts_with_dot_slash(url) || starts_with_dot_dot_slash(url);\n+\treturn fsck_starts_with_dot_slash(url) || fsck_starts_with_dot_dot_slash(url);\n }\n \n /*\n-- \ngitgitgadget\n\n"},{"id":"449307","messageId":"d36012fa27ab1d56d6711a9c920bd69b4f5c1b47.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 04/25] remote: move relative_url()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:42Z","receivedAt":"2022-02-23T18:31:22Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThis method was initially written in 63e95beb0 (submodule: port\nresolve_relative_url from shell to C, 2016-05-15). As we will need\nsimilar functionality in the bundle URI feature, extract this to be\navailable in remote.h.\n\nThe code is exactly the same. The prototype is different only in\nwhitespace. The documentation comment only adds explicit instructions on\nwhat happens when supplying two absolute URLs.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/submodule--helper.c | 119 ------------------------------------\n remote.c | 96 +++++++++++++++++++++++++++++\n remote.h | 30 +++++++++\n 3 files changed, 126 insertions(+), 119 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c17dde4170f..506ebd8e6bc 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -70,125 +70,6 @@ static int print_default_remote(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-/*\n- * Returns 1 if it was the last chop before ':'.\n- */\n-static int chop_last_dir(char **remoteurl, int is_relative)\n-{\n-\tchar *rfind = find_last_dir_sep(*remoteurl);\n-\tif (rfind) {\n-\t\t*rfind = '\\0';\n-\t\treturn 0;\n-\t}\n-\n-\trfind = strrchr(*remoteurl, ':');\n-\tif (rfind) {\n-\t\t*rfind = '\\0';\n-\t\treturn 1;\n-\t}\n-\n-\tif (is_relative || !strcmp(\".\", *remoteurl))\n-\t\tdie(_(\"cannot strip one component off url '%s'\"),\n-\t\t\t*remoteurl);\n-\n-\tfree(*remoteurl);\n-\t*remoteurl = xstrdup(\".\");\n-\treturn 0;\n-}\n-\n-/*\n- * The `url` argument is the URL that navigates to the submodule origin\n- * repo. When relative, this URL is relative to the superproject origin\n- * URL repo. The `up_path` argument, if specified, is the relative\n- * path that navigates from the submodule working tree to the superproject\n- * working tree. Returns the origin URL of the submodule.\n- *\n- * Return either an absolute URL or filesystem path (if the superproject\n- * origin URL is an absolute URL or filesystem path, respectively) or a\n- * relative file system path (if the superproject origin URL is a relative\n- * file system path).\n- *\n- * When the output is a relative file system path, the path is either\n- * relative to the submodule working tree, if up_path is specified, or to\n- * the superproject working tree otherwise.\n- *\n- * NEEDSWORK: This works incorrectly on the domain and protocol part.\n- * remote_url url outcome expectation\n- * http://a.com/b ../c http://a.com/c as is\n- * http://a.com/b/ ../c http://a.com/c same as previous line, but\n- * ignore trailing slash in url\n- * http://a.com/b ../../c http://c error out\n- * http://a.com/b ../../../c http:/c error out\n- * http://a.com/b ../../../../c http:c error out\n- * http://a.com/b ../../../../../c .:c error out\n- * NEEDSWORK: Given how chop_last_dir() works, this function is broken\n- * when a local part has a colon in its path component, too.\n- */\n-static char *relative_url(const char *remote_url,\n-\t\t\t\tconst char *url,\n-\t\t\t\tconst char *up_path)\n-{\n-\tint is_relative = 0;\n-\tint colonsep = 0;\n-\tchar *out;\n-\tchar *remoteurl = xstrdup(remote_url);\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tsize_t len = strlen(remoteurl);\n-\n-\tif (is_dir_sep(remoteurl[len-1]))\n-\t\tremoteurl[len-1] = '\\0';\n-\n-\tif (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))\n-\t\tis_relative = 0;\n-\telse {\n-\t\tis_relative = 1;\n-\t\t/*\n-\t\t * Prepend a './' to ensure all relative\n-\t\t * remoteurls start with './' or '../'\n-\t\t */\n-\t\tif (!starts_with_dot_slash(remoteurl) &&\n-\t\t !starts_with_dot_dot_slash(remoteurl)) {\n-\t\t\tstrbuf_reset(&sb);\n-\t\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n-\t\t\tfree(remoteurl);\n-\t\t\tremoteurl = strbuf_detach(&sb, NULL);\n-\t\t}\n-\t}\n-\t/*\n-\t * When the url starts with '../', remove that and the\n-\t * last directory in remoteurl.\n-\t */\n-\twhile (url) {\n-\t\tif (starts_with_dot_dot_slash(url)) {\n-\t\t\turl += 3;\n-\t\t\tcolonsep |= chop_last_dir(&remoteurl, is_relative);\n-\t\t} else if (starts_with_dot_slash(url))\n-\t\t\turl += 2;\n-\t\telse\n-\t\t\tbreak;\n-\t}\n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, colonsep ? \":\" : \"/\", url);\n-\tif (ends_with(url, \"/\"))\n-\t\tstrbuf_setlen(&sb, sb.len - 1);\n-\tfree(remoteurl);\n-\n-\tif (starts_with_dot_slash(sb.buf))\n-\t\tout = xstrdup(sb.buf + 2);\n-\telse\n-\t\tout = xstrdup(sb.buf);\n-\n-\tif (!up_path || !is_relative) {\n-\t\tstrbuf_release(&sb);\n-\t\treturn out;\n-\t}\n-\n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%s%s\", up_path, out);\n-\tfree(out);\n-\treturn strbuf_detach(&sb, NULL);\n-}\n-\n static char *resolve_relative_url(const char *rel_url, const char *up_path, int quiet)\n {\n \tchar *remoteurl, *resolved_url;\ndiff --git a/remote.c b/remote.c\nindex c97c626eaa8..c4a56749e85 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -14,6 +14,7 @@\n #include \"strvec.h\"\n #include \"commit-reach.h\"\n #include \"advice.h\"\n+#include \"connect.h\"\n \n enum map_direction { FROM_SRC, FROM_DST };\n \n@@ -2727,3 +2728,98 @@ void remote_state_clear(struct remote_state *remote_state)\n \thashmap_clear_and_free(&remote_state->remotes_hash, struct remote, ent);\n \thashmap_clear_and_free(&remote_state->branches_hash, struct remote, ent);\n }\n+\n+/*\n+ * Returns 1 if it was the last chop before ':'.\n+ */\n+static int chop_last_dir(char **remoteurl, int is_relative)\n+{\n+\tchar *rfind = find_last_dir_sep(*remoteurl);\n+\tif (rfind) {\n+\t\t*rfind = '\\0';\n+\t\treturn 0;\n+\t}\n+\n+\trfind = strrchr(*remoteurl, ':');\n+\tif (rfind) {\n+\t\t*rfind = '\\0';\n+\t\treturn 1;\n+\t}\n+\n+\tif (is_relative || !strcmp(\".\", *remoteurl))\n+\t\tdie(_(\"cannot strip one component off url '%s'\"),\n+\t\t\t*remoteurl);\n+\n+\tfree(*remoteurl);\n+\t*remoteurl = xstrdup(\".\");\n+\treturn 0;\n+}\n+\n+/*\n+ * NEEDSWORK: Given how chop_last_dir() works, this function is broken\n+ * when a local part has a colon in its path component, too.\n+ */\n+char *relative_url(const char *remote_url,\n+\t\t const char *url,\n+\t\t const char *up_path)\n+{\n+\tint is_relative = 0;\n+\tint colonsep = 0;\n+\tchar *out;\n+\tchar *remoteurl = xstrdup(remote_url);\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tsize_t len = strlen(remoteurl);\n+\n+\tif (is_dir_sep(remoteurl[len-1]))\n+\t\tremoteurl[len-1] = '\\0';\n+\n+\tif (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))\n+\t\tis_relative = 0;\n+\telse {\n+\t\tis_relative = 1;\n+\t\t/*\n+\t\t * Prepend a './' to ensure all relative\n+\t\t * remoteurls start with './' or '../'\n+\t\t */\n+\t\tif (!starts_with_dot_slash(remoteurl) &&\n+\t\t !starts_with_dot_dot_slash(remoteurl)) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n+\t\t\tfree(remoteurl);\n+\t\t\tremoteurl = strbuf_detach(&sb, NULL);\n+\t\t}\n+\t}\n+\t/*\n+\t * When the url starts with '../', remove that and the\n+\t * last directory in remoteurl.\n+\t */\n+\twhile (url) {\n+\t\tif (starts_with_dot_dot_slash(url)) {\n+\t\t\turl += 3;\n+\t\t\tcolonsep |= chop_last_dir(&remoteurl, is_relative);\n+\t\t} else if (starts_with_dot_slash(url))\n+\t\t\turl += 2;\n+\t\telse\n+\t\t\tbreak;\n+\t}\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, colonsep ? \":\" : \"/\", url);\n+\tif (ends_with(url, \"/\"))\n+\t\tstrbuf_setlen(&sb, sb.len - 1);\n+\tfree(remoteurl);\n+\n+\tif (starts_with_dot_slash(sb.buf))\n+\t\tout = xstrdup(sb.buf + 2);\n+\telse\n+\t\tout = xstrdup(sb.buf);\n+\n+\tif (!up_path || !is_relative) {\n+\t\tstrbuf_release(&sb);\n+\t\treturn out;\n+\t}\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s\", up_path, out);\n+\tfree(out);\n+\treturn strbuf_detach(&sb, NULL);\n+}\ndiff --git a/remote.h b/remote.h\nindex 4a1209ae2c8..91c7f187863 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -409,4 +409,34 @@ int parseopt_push_cas_option(const struct option *, const char *arg, int unset);\n int is_empty_cas(const struct push_cas_option *);\n void apply_push_cas(struct push_cas_option *, struct remote *, struct ref *);\n \n+/*\n+ * The `url` argument is the URL that navigates to the submodule origin\n+ * repo. When relative, this URL is relative to the superproject origin\n+ * URL repo. The `up_path` argument, if specified, is the relative\n+ * path that navigates from the submodule working tree to the superproject\n+ * working tree. Returns the origin URL of the submodule.\n+ *\n+ * Return either an absolute URL or filesystem path (if the superproject\n+ * origin URL is an absolute URL or filesystem path, respectively) or a\n+ * relative file system path (if the superproject origin URL is a relative\n+ * file system path).\n+ *\n+ * When the output is a relative file system path, the path is either\n+ * relative to the submodule working tree, if up_path is specified, or to\n+ * the superproject working tree otherwise.\n+ *\n+ * NEEDSWORK: This works incorrectly on the domain and protocol part.\n+ * remote_url url outcome expectation\n+ * http://a.com/b ../c http://a.com/c as is\n+ * http://a.com/b/ ../c http://a.com/c same as previous line, but\n+ * ignore trailing slash in url\n+ * http://a.com/b ../../c http://c error out\n+ * http://a.com/b ../../../c http:/c error out\n+ * http://a.com/b ../../../../c http:c error out\n+ * http://a.com/b ../../../../../c .:c error out\n+ */\n+char *relative_url(const char *remote_url,\n+\t\t const char *url,\n+\t\t const char *up_path);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"449308","messageId":"0abec796b0089b84d23cb52bb127788fdd04961c.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 01/25] docs: document bundle URI standard","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:39Z","receivedAt":"2022-02-23T18:31:23Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIntroduce the idea of bundle URIs to the Git codebase through an\naspirational design document. This document includes the full design\nintended to include the feature in its fully-implemented form. This will\ntake several steps as detailed in the Implementation Plan section.\n\nBy committing this document now, it can be used to motivate changes\nnecessary to reach these final goals. The design can still be altered as\nnew information is discovered.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/technical/bundle-uri.txt | 404 +++++++++++++++++++++++++\n 1 file changed, 404 insertions(+)\n create mode 100644 Documentation/technical/bundle-uri.txt\n\ndiff --git a/Documentation/technical/bundle-uri.txt b/Documentation/technical/bundle-uri.txt\nnew file mode 100644\nindex 00000000000..5c0b9e8e3ef\n--- /dev/null\n+++ b/Documentation/technical/bundle-uri.txt\n@@ -0,0 +1,404 @@\n+Bundle URIs\n+===========\n+\n+Bundle URIs are locations where Git can download one or more bundles in\n+order to bootstrap the object database in advance of fetching the remaining\n+objects from a remote.\n+\n+One goal is to speed up clones and fetches for users with poor network\n+connectivity to the origin server. Another benefit is to allow heavy users,\n+such as CI build farms, to use local resources for the majority of Git data\n+and thereby reducing the load on the origin server.\n+\n+To enable the bundle URI feature, users can specify a bundle URI using\n+command-line options or the origin server can advertise one or more URIs\n+via a protocol v2 capability.\n+\n+Server requirements\n+-------------------\n+\n+To provide a server-side implementation of bundle servers, no other parts\n+of the Git protocol are required. This allows server maintainers to use\n+static content solutions such as CDNs in order to serve the bundle files.\n+\n+At the current scope of the bundle URI feature, all URIs are expected to\n+be HTTP(S) URLs where content is downloaded to a local file using a `GET`\n+request to that URL. The server could include authentication requirements\n+to those requests with the aim of triggering the configured credential\n+helper for secure access.\n+\n+Assuming a `200 OK` response from the server, the content at the URL is\n+expected to be of one of two forms:\n+\n+1. Bundle: A Git bundle file of version 2 or higher.\n+\n+2. Table of Contents: A plain-text file that is parsable using Git's\n+ config file parser. This file describes one or more bundles that are\n+ accessible from other URIs.\n+\n+Any other data provided by the server is considered erroneous.\n+\n+Table of Contents Format\n+------------------------\n+\n+If the content at a bundle URI is not a bundle, then it is expected to be\n+a plaintext file that is parseable using Git's config parser. This file\n+can contain any list of key/value pairs, but only a fixed set will be\n+considered by Git.\n+\n+bundle.tableOfContents.version::\n+\tThis value provides a version number for the table of contents. If\n+\ta future Git change enables a feature that needs the Git client to\n+\treact to a new key in the table of contents file, then this version\n+\twill increment. The only current version number is 1, and if any\n+\tother value is specified then Git will fail to use this file.\n+\n+bundle.tableOfContents.forFetch::\n+\tThis boolean value is a signal to the Git client that the bundle\n+\tserver has designed its bundle organization to assist `git fetch`\n+\tcommands in addition to `git clone` commands. If this is missing,\n+\tGit should not use this table of contents for `git fetch` as it\n+\tmay lead to excess data downloads.\n+\n+The remaining keys include an `` segment which is a server-designated\n+name for each available bundle.\n+\n+bundle..uri::\n+\tThis string value is the URI for downloading bundle ``. If\n+\tthe URI begins with a protocol (`http://` or `https://`) then the\n+\tURI is absolute. Otherwise, the URI is interpreted as relative to\n+\tthe URI used for the table of contents. If the URI begins with `/`,\n+\tthen that relative path is relative to the domain name used for\n+\tthe table of contents. (This use of relative paths is intended to\n+\tmake it easier to distribute a set of bundles across a large\n+\tnumber of servers or CDNs with different domain names.)\n+\n+bundle..timestamp::\n+\t(Optional) This value is the number of seconds since Unix epoch\n+\t(UTC) that this bundle was created. This is used as an approximation\n+\tof a point in time that the bundle matches the data available at\n+\tthe origin server.\n+\n+bundle..requires::\n+\t(Optional) This string value represents the ID of another bundle.\n+\tWhen present, the server is indicating that this bundle contains a\n+\tthin packfile. If the client does not have all necessary objects\n+\tto unbundle this packfile, then the client can download the bundle\n+\twith the `requires` ID and try again. (Note: it may be beneficial\n+\tto allow the server to specify multiple `requires` bundles.)\n+\n+bundle..filter::\n+\t(Optional) This string value represents an object filter that\n+\tshould also appear in the header of this bundle. The server uses\n+\tthis value to differentiate different kinds of bundles from which\n+\tthe client can choose those that match their object filters.\n+\n+Here is an example table of contents:\n+\n+```\n+[bundle \"tableofcontents\"]\n+\tversion = 1\n+\tforFetch = true\n+\n+[bundle \"2022-02-09-1644442601-daily\"]\n+\turi = https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-09-1644442601-daily.bundle\n+\ttimestamp = 1644442601\n+\trequires = 2022-02-02-1643842562\n+\n+[bundle \"2022-02-02-1643842562\"]\n+\turi = https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-02-1643842562.bundle\n+\ttimestamp = 1643842562\n+\n+[bundle \"2022-02-09-1644442631-daily-blobless\"]\n+\turi = 2022-02-09-1644442631-daily-blobless.bundle\n+\ttimestamp = 1644442631\n+\trequires = 2022-02-02-1643842568-blobless\n+\tfilter = blob:none\n+\n+[bundle \"2022-02-02-1643842568-blobless\"]\n+\turi = /git/git/2022-02-02-1643842568-blobless.bundle\n+\ttimestamp = 1643842568\n+\tfilter = blob:none\n+```\n+\n+This example uses all of the keys in the specification. Suppose that the\n+table of contents was found at the URI\n+`https://gitbundleserver.z13.web.core.windows.net/git/git/` and so the\n+two blobless bundles have the following fully-expanded URIs:\n+\n+* `https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-09-1644442631-daily-blobless.bundle`\n+* `https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-02-1643842568-blobless.bundle`\n+\n+Advertising Bundle URIs\n+-----------------------\n+\n+If a user knows a bundle URI for the repository they are cloning, then they\n+can specify that URI manually through a command-line option. However, a\n+Git host may want to advertise bundle URIs during the clone operation,\n+helping users unaware of the feature.\n+\n+Note: The exact details of this section are not final. This is a possible\n+way that Git could auto-discover bundle URIs, but is not a committed\n+direction until that feature is implemented.\n+\n+The only thing required for this feature is that the server can advertise\n+one or more bundle URIs. One way to implement this is to create a new\n+protocol v2 capability that advertises recommended features, including\n+bundle URIs.\n+\n+The client could choose an arbitrary bundle URI as an option _or_ select\n+the URI with lowest latency by some exploratory checks. It is up to the\n+server operator to decide if having multiple URIs is preferable to a\n+single URI that is geodistributed through server-side infrastructure.\n+\n+Cloning with Bundle URIs\n+------------------------\n+\n+The primary need for bundle URIs is to speed up clones. The Git client\n+will interact with bundle URIs according to the following flow:\n+\n+1. The user specifies a bundle URI with the `--bundle-uri` command-line\n+ option _or_ the client discovers a bundle URI that was advertised by\n+ the remote server.\n+\n+2. The client downloads the file at the bundle URI. If it is a bundle, then\n+ it is unbundled with the refs being stored in `refs/bundle/*`.\n+\n+3. If the file is instead a table of contents, then the bundles with\n+ matching `filter` settings are sorted by `timestamp` (if present),\n+ and the most-recent bundle is downloaded.\n+\n+4. If the current bundle header mentions negative commid OIDs that are not\n+ in the object database, then download the `requires` bundle and try\n+ again.\n+\n+5. After inspecting a bundle with no negative commit OIDs (or all OIDs are\n+ already in the object database somehow), then unbundle all of the\n+ bundles in reverse order, placing references within `refs/bundle/*`.\n+\n+6. The client performs a fetch negotiation with the origin server, using\n+ the `refs/bundle/*` references as `have`s and the server's ref\n+ advertisement as `want`s. This results in a pack-file containing the\n+ remaining objects requested by the clone but not in the bundles.\n+\n+Note that during a clone we expect that all bundles will be required. The\n+client could be extended to download all bundles in parallel, though they\n+need to be unbundled in the correct order.\n+\n+If a table of contents is used and it contains\n+`bundle.tableOfContents.forFetch = true`, then the client can store a\n+config value indicating to reuse this URI for later `git fetch` commands.\n+In this case, the client will also want to store the maximum timestamp of\n+a downloaded bundle.\n+\n+Fetching with Bundle URIs\n+-------------------------\n+\n+When the client fetches new data, it can decide to fetch from bundle\n+servers before fetching from the origin remote. This could be done via\n+a command-line option, but it is more likely useful to use a config value\n+such as the one specified during the clone.\n+\n+The fetch operation follows the same procedure to download bundles from a\n+table of contents (although we do _not_ want to use parallel downloads\n+here). We expect that the process will end because all negative commit\n+OIDs in a thin bundle are already in the object database.\n+\n+A further optimization is that the client can avoid downloading any\n+bundles if their timestamps are not larger than the stored timestamp.\n+After fetching new bundles, this local timestamp value is updated.\n+\n+Choices for Bundle Server Organization\n+--------------------------------------\n+\n+With this standard, there are many options available to the bundle server\n+in how it organizes Git data into bundles.\n+\n+* Bundles can have whatever name the server desires. This name could refer\n+ to immutable data by using a hash of the bundle contents. However, this\n+ means that a new URI will be needed after every update of the content.\n+ This might be acceptable if the server is advertising the URI (and the\n+ server is aware of new bundles being generated) but would not be\n+ ergonomic for users using the command line option.\n+\n+* If the server intends to only serve full clones, then the advertised URI\n+ could be a bundle file without a filter that is updated at some cadence.\n+\n+* If the server intends to serve clones, but wants clients to choose full\n+ or blobless partial clones, then the server can use a table of contents\n+ that lists two non-thin bundles and the client chooses between them only\n+ by the `bundle..filter` values.\n+\n+* If the server intends to improve clones with parallel downloads, then it\n+ can use a table of contents and split the repository into time intervals\n+ of approximately similar-sized bundles. Using `bundle..timestamp`\n+ and `bundle..requires` values helps the client decide the order to\n+ unbundle the bundles.\n+\n+* If the server intends to serve fetches, then it can use a table of\n+ contents to advertise a list of bundles that are updated regularly. The\n+ most recent bundles could be generated on short intervals, such as hourly.\n+ These small bundles could be merged together at some rate, such as 24\n+ hourly bundles merging into a single daily bundle. At some point, it may\n+ be beneficial to create a bundle that stores the majority of the history,\n+ such as all data older than 30 days.\n+\n+These recommendations are intended only as suggestions. Each repository is\n+different and every Git server has different needs. Hopefully the bundle\n+URI feature and its table of contents is flexible enough to satisfy all\n+needs. If not, then the format can be extended.\n+\n+Error Conditions\n+----------------\n+\n+If the Git client discovers something unexpected while downloading\n+information according to a bundle URI or the table of contents found at\n+that location, then Git can ignore that data and continue as if it was not\n+given a bundle URI. The remote Git server is the ultimate source of truth,\n+not the bundle URI.\n+\n+Here are a few example error conditions:\n+\n+* The client fails to connect with a server at the given URI or a connection\n+ is lost without any chance to recover.\n+\n+* The client receives a response other than `200 OK` (such as `404 Not Found`,\n+ `401 Not Authorized`, or `500 Internal Server Error`).\n+\n+* The client receives data that is not parsable as a bundle or table of\n+ contents.\n+\n+* The table of contents describes a directed cycle in the\n+ `bundle..requires` links.\n+\n+* A bundle includes a filter that does not match expectations.\n+\n+* The client cannot unbundle the bundles because the negative commit OIDs\n+ are not in the object database and there are no more\n+ `bundle..requires` links to follow.\n+\n+There are also situations that could be seen as wasteful, but are not\n+error conditions:\n+\n+* The downloaded bundles contain more information than is requested by\n+ the clone or fetch request. A primary example is if the user requests\n+ a clone with `--single-branch` but downloads bundles that store every\n+ reachable commit from all `refs/heads/*` references. This might be\n+ initially wasteful, but perhaps these objects will become reachable by\n+ a later ref update that the client cares about.\n+\n+* A bundle download during a `git fetch` contains objects already in the\n+ object database. This is probably unavoidable if we are using bundles\n+ for fetches, since the client will almost always be slightly ahead of\n+ the bundle servers after performing its \"catch-up\" fetch to the remote\n+ server. This extra work is most wasteful when the client is fetching\n+ much more frequently than the server is computing bundles, such as if\n+ the client is using hourly prefetches with background maintenance, but\n+ the server is computing bundles weekly. For this reason, the client\n+ should not use bundle URIs for fetch unless the server has explicitly\n+ recommended it through the `bundle.tableOfContents.forFetch = true`\n+ value.\n+\n+Implementation Plan\n+-------------------\n+\n+This design document is being submitted on its own as an aspirational\n+document, with the goal of implementing all of the mentioned client\n+features over the course of several patch series. Here is a potential\n+outline for submitting these features for full review:\n+\n+1. Update the `git bundle create` command to take a `--filter` option,\n+ allowing bundles to store packfiles restricted to an object filter.\n+ This is necessary for using bundle URIs to benefit partial clones.\n+\n+2. Integrate bundle URIs into `git clone` with a `--bundle-uri` option.\n+ This will include the full understanding of a table of contents, but\n+ will not integrate with `git fetch` or allow the server to advertise\n+ URIs.\n+\n+3. Integrate bundle URIs into `git fetch`, triggered by config values that\n+ are set during `git clone` if the server indicates that the bundle\n+ strategy works for fetches.\n+\n+4. Create a new \"recommended features\" capability in protocol v2 where the\n+ server can recommend features such as bundle URIs, partial clone, and\n+ sparse-checkout. These features will be extremely limited in scope and\n+ blocked by opt-in config options. The design for this portion could be\n+ replaced by a \"bundle-uri\" capability that only advertises bundle URIs\n+ and no other information.\n+\n+Related Work: Packfile URIs\n+---------------------------\n+\n+The Git protocol already has a capability where the Git server can list\n+a set of URLs along with the packfile response when serving a client\n+request. The client is then expected to download the packfiles at those\n+locations in order to have a complete understanding of the response.\n+\n+This mechanism is used by the Gerrit server (implemented with JGit) and\n+has been effective at reducing CPU load and improving user performance for\n+clones.\n+\n+A major downside to this mechanism is that the origin server needs to know\n+_exactly_ what is in those packfiles, and the packfiles need to be available\n+to the user for some time after the server has responded. This coupling\n+between the origin and the packfile data is difficult to manage.\n+\n+Further, this implementation is extremely hard to make work with fetches.\n+\n+Related Work: GVFS Cache Servers\n+--------------------------------\n+\n+The GVFS Protocol [2] is a set of HTTP endpoints designed independently of\n+the Git project before Git's partial clone was created. One feature of this\n+protocol is the idea of a \"cache server\" which can be colocated with build\n+machines or developer offices to transfer Git data without overloading the\n+central server.\n+\n+The endpoint that VFS for Git is famous for is the `GET /gvfs/objects/{oid}`\n+endpoint, which allows downloading an object on-demand. This is a critical\n+piece of the filesystem virtualization of that product.\n+\n+However, a more subtle need is the `GET /gvfs/prefetch?lastPackTimestamp=`\n+endpoint. Given an optional timestamp, the cache server responds with a list\n+of precomputed packfiles containing the commits and trees that were introduced\n+in those time intervals.\n+\n+The cache server computes these \"prefetch\" packfiles using the following\n+strategy:\n+\n+1. Every hour, an \"hourly\" pack is generated with a given timestamp.\n+2. Nightly, the previous 24 hourly packs are rolled up into a \"daily\" pack.\n+3. Nightly, all prefetch packs more than 30 days old are rolled up into\n+ one pack.\n+\n+When a user runs `gvfs clone` or `scalar clone` against a repo with cache\n+servers, the client requests all prefetch packfiles, which is at most\n+`24 + 30 + 1` packfiles downloading only commits and trees. The client\n+then follows with a request to the origin server for the references, and\n+attempts to checkout that tip reference. (There is an extra endpoint that\n+helps get all reachable trees from a given commit, in case that commit\n+was not already in a prefetch packfile.)\n+\n+During a `git fetch`, a hook requests the prefetch endpoint using the\n+most-recent timestamp from a previously-downloaded prefetch packfile.\n+Only the list of packfiles with later timestamps are downloaded. Most\n+users fetch hourly, so they get at most one hourly prefetch pack. Users\n+whose machines have been off or otherwise have not fetched in over 30 days\n+might redownload all prefetch packfiles. This is rare.\n+\n+It is important to note that the clients always contact the origin server\n+for the refs advertisement, so the refs are frequently \"ahead\" of the\n+prefetched pack data. The missing objects are downloaded on-demand using\n+the `GET gvfs/objects/{oid}` requests, when needed by a command such as\n+`git checkout` or `git log`. Some Git optimizations disable checks that\n+would cause these on-demand downloads to be too aggressive.\n+\n+See Also\n+--------\n+\n+[1] https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n+ An earlier RFC for a bundle URI feature.\n+\n+[2] https://github.com/microsoft/VFSForGit/blob/master/Protocol.md\n+ The GVFS Protocol\n\\ No newline at end of file\n-- \ngitgitgadget\n\n"},{"id":"449309","messageId":"0e19607d5d96308d07ae8df65d48d7f29be0ea50.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 06/25] http: make http_get_file() external","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:44Z","receivedAt":"2022-02-23T18:31:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThis method will be used in an upcoming extension of git-remote-curl to\ndownload a single file over HTTP(S) by request.\n\nSigned-off-by: Derrick Stolee \n---\n http.c | 4 ++--\n http.h | 9 +++++++++\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 229da4d1488..04e73149357 100644\n--- a/http.c\n+++ b/http.c\n@@ -1945,8 +1945,8 @@ int http_get_strbuf(const char *url,\n * If a previous interrupted download is detected (i.e. a previous temporary\n * file is still around) the download is resumed.\n */\n-static int http_get_file(const char *url, const char *filename,\n-\t\t\t struct http_get_options *options)\n+int http_get_file(const char *url, const char *filename,\n+\t\t struct http_get_options *options)\n {\n \tint ret;\n \tstruct strbuf tmpfile = STRBUF_INIT;\ndiff --git a/http.h b/http.h\nindex df1590e53a4..ba303cfb372 100644\n--- a/http.h\n+++ b/http.h\n@@ -163,6 +163,15 @@ struct http_get_options {\n */\n int http_get_strbuf(const char *url, struct strbuf *result, struct http_get_options *options);\n \n+/*\n+ * Downloads a URL and stores the result in the given file.\n+ *\n+ * If a previous interrupted download is detected (i.e. a previous temporary\n+ * file is still around) the download is resumed.\n+ */\n+int http_get_file(const char *url, const char *filename,\n+\t\t struct http_get_options *options);\n+\n int http_fetch_ref(const char *base, struct ref *ref);\n \n /* Helpers for fetching packs */\n-- \ngitgitgadget\n\n"},{"id":"449310","messageId":"ba605c0e4989eb8fabda4836ccd4ee0ef946e1e2.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 07/25] remote-curl: add 'get' capability","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:45Z","receivedAt":"2022-02-23T18:31:27Z","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\nRFC-TODO: This change requires tests directly on the remote helper.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/gitremote-helpers.txt | 6 ++++++\n remote-curl.c | 32 +++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\nindex 6f1e269ae43..f82588601a9 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,9 @@ 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 ``.\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 0dabef2dd7c..92beb98631b 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1270,6 +1270,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 http_get_options opts = { 0 };\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+\thttp_get_file(url.buf, path.buf, &opts);\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@@ -1542,9 +1569,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-- \ngitgitgadget\n\n"},{"id":"449311","messageId":"b993e7b47104f1d1740ab39d7ef3dc5bbccecbfc.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 08/25] bundle: implement 'fetch' command for direct bundles","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:46Z","receivedAt":"2022-02-23T18:31:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git bundle fetch ' command will be used to download one or\nmore bundles from a specified ''. The implementation being added\nhere focuses only on downloading a file from '' and unbundling it\nif it is a valid bundle file.\n\nIf it is not a bundle file, then we currently die(), but a later change\nwill attempt to interpret it as a table of contents with possibly\nmultiple bundles listed, along with other metadata for each bundle.\n\nThat explains a bit why cmd_bundle_fetch() has three steps carefully\ncommented, including a \"stack\" that currently can only hold one bundle.\nWe will later update this while loop to push onto the stack when\nnecessary.\n\nRFC-TODO: Add documentation to Documentation/git-bundle.txt\n\nRFC-TODO: Add direct tests of 'git bundle fetch' when the URI is a\nbundle file.\n\nRFC-TODO: Split out the docs and subcommand boilerplate into its own\ncommit.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/bundle.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 261 insertions(+)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 8187b7df739..0e06f1756d1 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -3,6 +3,10 @@\n #include \"parse-options.h\"\n #include \"cache.h\"\n #include \"bundle.h\"\n+#include \"run-command.h\"\n+#include \"hashmap.h\"\n+#include \"object-store.h\"\n+#include \"refs.h\"\n \n /*\n * Basic handler for bundle files to connect repositories via sneakernet.\n@@ -13,6 +17,7 @@\n \n static const char * const builtin_bundle_usage[] = {\n N_(\"git bundle create [] \"),\n+ N_(\"git bundle fetch [] \"),\n N_(\"git bundle list-heads [...]\"),\n N_(\"git bundle unbundle [...]\"),\n N_(\"git bundle verify [] \"),\n@@ -24,6 +29,11 @@ static const char * const builtin_bundle_create_usage[] = {\n NULL\n };\n \n+static const char * const builtin_bundle_fetch_usage[] = {\n+ N_(\"git bundle fetch [--filter=] \"),\n+ NULL\n+};\n+\n static const char * const builtin_bundle_list_heads_usage[] = {\n N_(\"git bundle list-heads [...]\"),\n NULL\n@@ -131,6 +141,255 @@ cleanup:\n \treturn ret;\n }\n \n+/**\n+ * The remote_bundle_info struct contains the necessary data for\n+ * the list of bundles advertised by a table of contents. If the\n+ * bundle URI instead contains a single bundle, then this struct\n+ * can represent a single bundle without a 'uri' but with a\n+ * tempfile storing its current location on disk.\n+ */\n+struct remote_bundle_info {\n+\tstruct hashmap_entry ent;\n+\n+\t/**\n+\t * The 'id' is a name given to the bundle for reference\n+\t * by other bundle infos.\n+\t */\n+\tchar *id;\n+\n+\t/**\n+\t * The 'uri' is the location of the remote bundle so\n+\t * it can be downloaded on-demand. This will be NULL\n+\t * if there was no table of contents.\n+\t */\n+\tchar *uri;\n+\n+\t/**\n+\t * The 'next_id' string, if non-NULL, contains the 'id'\n+\t * for a bundle that contains the prerequisites for this\n+\t * bundle. Used by table of contents to allow fetching\n+\t * a portion of a repository incrementally.\n+\t */\n+\tchar *next_id;\n+\n+\t/**\n+\t * A table of contents can include a timestamp for the\n+\t * bundle as a heuristic for describing a list of bundles\n+\t * in order of recency.\n+\t */\n+\ttimestamp_t timestamp;\n+\n+\t/**\n+\t * If the bundle has been downloaded, then 'file' is a\n+\t * filename storing its contents. Otherwise, 'file' is\n+\t * an empty string.\n+\t */\n+\tstruct strbuf file;\n+\n+\t/**\n+\t * The 'stack_next' pointer allows this struct to form\n+\t * a stack.\n+\t */\n+\tstruct remote_bundle_info *stack_next;\n+};\n+\n+static void download_uri_to_file(const char *uri, const char *file)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tFILE *child_in;\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\tdie(_(\"failed to start remote helper\"));\n+\n+\tchild_in = fdopen(cp.in, \"w\");\n+\tif (!child_in)\n+\t\tdie(_(\"cannot write to child process\"));\n+\n+\tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n+\tfclose(child_in);\n+\n+\tif (finish_command(&cp))\n+\t\tdie(_(\"remote helper failed\"));\n+}\n+\n+static void find_temp_filename(struct strbuf *name)\n+{\n+\tint fd;\n+\t/*\n+\t * Find a temporray 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\tdie(_(\"failed to create temporary file\"));\n+\tclose(fd);\n+\tunlink(name->buf);\n+}\n+\n+static void unbundle_fetched_bundle(struct remote_bundle_info *info)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tFILE *f;\n+\tstruct strbuf line = STRBUF_INIT;\n+\tstruct strbuf bundle_ref = STRBUF_INIT;\n+\tsize_t bundle_prefix_len;\n+\n+\tstrvec_pushl(&cp.args, \"bundle\", \"unbundle\",\n+\t\t\t\tinfo->file.buf, NULL);\n+\tcp.git_cmd = 1;\n+\tcp.out = -1;\n+\n+\tif (start_command(&cp))\n+\t\tdie(_(\"failed to start 'unbundle' process\"));\n+\n+\tstrbuf_addstr(&bundle_ref, \"refs/bundles/\");\n+\tbundle_prefix_len = bundle_ref.len;\n+\n+\tf = fdopen(cp.out, \"r\");\n+\twhile (strbuf_getline(&line, f) != EOF) {\n+\t\tstruct object_id oid, old_oid;\n+\t\tconst char *refname, *branch_name, *end;\n+\t\tchar *space;\n+\t\tint has_old;\n+\n+\t\tstrbuf_trim_trailing_newline(&line);\n+\n+\t\tspace = strchr(line.buf, ' ');\n+\n+\t\tif (!space)\n+\t\t\tcontinue;\n+\n+\t\trefname = space + 1;\n+\t\t*space = '\\0';\n+\t\tparse_oid_hex(line.buf, &oid, &end);\n+\n+\t\tif (!skip_prefix(refname, \"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+\n+\t\tupdate_ref(\"bundle fetch\", bundle_ref.buf, &oid,\n+\t\t\t\thas_old ? &old_oid : NULL,\n+\t\t\t\tREF_SKIP_OID_VERIFICATION,\n+\t\t\t\tUPDATE_REFS_MSG_ON_ERR);\n+\t}\n+\n+\tif (finish_command(&cp))\n+\t\tdie(_(\"failed to unbundle bundle from '%s'\"), info->uri);\n+\n+\tunlink_or_warn(info->file.buf);\n+}\n+\n+static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n+{\n+\tint ret = 0;\n+\tint progress = isatty(2);\n+\tchar *bundle_uri;\n+\tstruct remote_bundle_info first_file = {\n+\t\t.file = STRBUF_INIT,\n+\t};\n+\tstruct remote_bundle_info *stack = NULL;\n+\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"progress\", &progress,\n+\t\t\t N_(\"show progress meter\")),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options_cmd_bundle(argc, argv, prefix,\n+\t\t\tbuiltin_bundle_fetch_usage, options, &bundle_uri);\n+\n+\tif (!startup_info->have_repository)\n+\t\tdie(_(\"'fetch' requires a repository\"));\n+\n+\t/*\n+\t * Step 1: determine protocol for uri, and download contents to\n+\t * a temporary location.\n+\t */\n+\tfirst_file.uri = bundle_uri;\n+\tfind_temp_filename(&first_file.file);\n+\tdownload_uri_to_file(bundle_uri, first_file.file.buf);\n+\n+\t/*\n+\t * Step 2: Check if the file is a bundle (if so, add it to the\n+\t * stack and move to step 3).\n+\t */\n+\n+\tif (is_bundle(first_file.file.buf, 1)) {\n+\t\t/* The simple case: only one file, no stack to worry about. */\n+\t\tstack = &first_file;\n+\t} else {\n+\t\t/* TODO: Expect and parse a table of contents. */\n+\t\tdie(_(\"unexpected data at bundle URI\"));\n+\t}\n+\n+\t/*\n+\t * Step 3: For each bundle in the stack:\n+\t * \ti. If not downloaded to a temporary file, download it.\n+\t * \tii. Once downloaded, check that its prerequisites are in\n+\t * \t the object database. If not, then push its dependent\n+\t * \t bundle onto the stack. (Fail if no such bundle exists.)\n+\t * \tiii. If all prerequisites are present, then unbundle the\n+\t * \t temporary file and pop the bundle from the stack.\n+\t */\n+\twhile (stack) {\n+\t\tint valid = 1;\n+\t\tint bundle_fd;\n+\t\tstruct string_list_item *prereq;\n+\t\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n+\n+\t\tif (!stack->file.len) {\n+\t\t\tfind_temp_filename(&stack->file);\n+\t\t\tdownload_uri_to_file(stack->uri, stack->file.buf);\n+\t\t\tif (!is_bundle(stack->file.buf, 1))\n+\t\t\t\tdie(_(\"file downloaded from '%s' is not a bundle\"), stack->uri);\n+\t\t}\n+\n+\t\tbundle_header_init(&header);\n+\t\tbundle_fd = read_bundle_header(stack->file.buf, &header);\n+\t\tif (bundle_fd < 0)\n+\t\t\tdie(_(\"failed to read bundle from '%s'\"), stack->uri);\n+\n+\t\tfor_each_string_list_item(prereq, &header.prerequisites) {\n+\t\t\tstruct object_info info = OBJECT_INFO_INIT;\n+\t\t\tstruct object_id *oid = prereq->util;\n+\n+\t\t\tif (oid_object_info_extended(the_repository, oid, &info,\n+\t\t\t\t\t\t OBJECT_INFO_QUICK)) {\n+\t\t\t\tvalid = 0;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t\tclose(bundle_fd);\n+\t\tbundle_header_release(&header);\n+\n+\t\tif (valid) {\n+\t\t\tunbundle_fetched_bundle(stack);\n+\t\t} else if (stack->next_id) {\n+\t\t\t/*\n+\t\t\t * Load the next bundle from the hashtable and\n+\t\t\t * push it onto the stack.\n+\t\t\t */\n+\t\t} else {\n+\t\t\tdie(_(\"bundle from '%s' has missing prerequisites and no dependent bundle\"),\n+\t\t\t stack->uri);\n+\t\t}\n+\n+\t\tstack = stack->stack_next;\n+\t}\n+\n+\tfree(bundle_uri);\n+\treturn ret;\n+}\n+\n static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix) {\n \tstruct bundle_header header = BUNDLE_HEADER_INIT;\n \tint bundle_fd = -1;\n@@ -209,6 +468,8 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)\n \n \telse if (!strcmp(argv[0], \"create\"))\n \t\tresult = cmd_bundle_create(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"fetch\"))\n+\t\tresult = cmd_bundle_fetch(argc, argv, prefix);\n \telse if (!strcmp(argv[0], \"list-heads\"))\n \t\tresult = cmd_bundle_list_heads(argc, argv, prefix);\n \telse if (!strcmp(argv[0], \"unbundle\"))\n-- \ngitgitgadget\n\n"},{"id":"449312","messageId":"7221bd9a8c7b3c81e20a0b5cd2f7b8b5e248a81f.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 09/25] bundle: parse table of contents during 'fetch'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:47Z","receivedAt":"2022-02-23T18:31:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIn order to support a flexible bundle URI feature, we allow the server\nto return a \"table of contents\" file that is formatted according to Git\nconfig file standards. These files can describe multiple bundles,\nintended to assist with using bundle URIs for fetching or with partial\nclone.\n\nHere is an example table of contents file:\n\n[bundle \"tableofcontents\"]\n\tversion = 1\n\n[bundle \"2022-02-09-1644442601-daily\"]\n\turi = 2022-02-09-1644442601-daily.bundle\n\ttimestamp = 1644442601\n\trequires = 2022-02-02-1643842562\n\n[bundle \"2022-02-02-1643842562\"]\n\turi = 2022-02-02-1643842562.bundle\n\ttimestamp = 1643842562\n\n[bundle \"2022-02-09-1644442631-daily-blobless\"]\n\turi = 2022-02-09-1644442631-daily-blobless.bundle\n\ttimestamp = 1644442631\n\trequires = 2022-02-02-1643842568-blobless\n\tfilter = blob:none\n\n[bundle \"2022-02-02-1643842568-blobless\"]\n\turi = 2022-02-02-1643842568-blobless.bundle\n\ttimestamp = 1643842568\n\tfilter = blob:none\n\n(End of example.)\n\nThis file contains some important fixed values, such as\n\n * bundle.tableofcontents.version = 1\n\nAlso, different bundles are referenced by , using keys with names\n\n * bundle..uri: the URI to download this bundle. This could be an\n absolute URI or a URI relative to the bundle file's URI.\n * bundle..timestamp: the timestamp when this file was generated.\n * bundle..filter: the partial clone filter applied on this bundle.\n * bundle..requires: the ID for the previous bundle.\n\nThe current change does not parse the '.filter' option, but does use the\n'.requires' in the 'while (stack)' loop.\n\nThe process is that 'git bundle fetch' will parse the table of contents\nand pick the most-recent bundle and download that one. That bundle\nheader has a ref listing, including (possibly) a list of commits that\nare missing from the bundle. If any of those commits are missing, then\nGit downloads the bundle specified by the '.requires' value and tries\nagain. Eventually, Git should download a bundle where all missing\ncommits actually exist in the current repository, or Git downloads a\nbundle with no missing commits.\n\nOf course, the server could be advertising incorrect information, so it\ncould advertise bundles that never satisfy the missing objects. It could\nalso create a directed cycle in its '.requires' specifications. In each\nof these cases, Git will die with a \"bundle '' still invalid after\ndownloading required bundle\" message or a \"bundle from '' has\nmissing prerequisites and no dependent bundle\" message.\n\nRFC-TODO: add a direct test of table of contents parsing in this change.\nRFC-TODO: create tests that check these erroneous cases.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/bundle.c | 169 +++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 162 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 0e06f1756d1..66f3b3c9376 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -7,6 +7,8 @@\n #include \"hashmap.h\"\n #include \"object-store.h\"\n #include \"refs.h\"\n+#include \"config.h\"\n+#include \"packfile.h\"\n \n /*\n * Basic handler for bundle files to connect repositories via sneakernet.\n@@ -165,12 +167,21 @@ struct remote_bundle_info {\n \tchar *uri;\n \n \t/**\n-\t * The 'next_id' string, if non-NULL, contains the 'id'\n+\t * The 'requires_id' string, if non-NULL, contains the 'id'\n \t * for a bundle that contains the prerequisites for this\n \t * bundle. Used by table of contents to allow fetching\n \t * a portion of a repository incrementally.\n \t */\n-\tchar *next_id;\n+\tchar *requires_id;\n+\n+\t/**\n+\t * The 'filter_str' string, if non-NULL, specifies the\n+\t * filter capability exists in this bundle with the given\n+\t * specification. Allows selecting bundles that match the\n+\t * client's desired filter. If NULL, then no filter exists\n+\t * on the bundle.\n+\t */\n+\tchar *filter_str;\n \n \t/**\n \t * A table of contents can include a timestamp for the\n@@ -191,8 +202,106 @@ struct remote_bundle_info {\n \t * a stack.\n \t */\n \tstruct remote_bundle_info *stack_next;\n+\n+\t/**\n+\t * 'pushed' is set when first pushing the required bundle\n+\t * onto the stack. Used to error out when verifying the\n+\t * prerequisites and avoiding an infinite loop.\n+\t */\n+\tunsigned pushed:1;\n };\n \n+static int remote_bundle_cmp(const void *unused_cmp_data,\n+\t\t\t const struct hashmap_entry *a,\n+\t\t\t const struct hashmap_entry *b,\n+\t\t\t const void *key)\n+{\n+\tconst struct remote_bundle_info *ee1 =\n+\t\t\tcontainer_of(a, struct remote_bundle_info, ent);\n+\tconst struct remote_bundle_info *ee2 =\n+\t\t\tcontainer_of(b, struct remote_bundle_info, ent);\n+\n+\treturn strcmp(ee1->id, ee2->id);\n+}\n+\n+static int parse_toc_config(const char *key, const char *value, void *data)\n+{\n+\tstruct hashmap *toc = data;\n+\tconst char *key1, *key2, *id_end;\n+\tstruct strbuf id = STRBUF_INIT;\n+\tstruct remote_bundle_info info_lookup;\n+\tstruct remote_bundle_info *info;\n+\n+\tif (!skip_prefix(key, \"bundle.\", &key1))\n+\t\treturn -1;\n+\n+\tif (skip_prefix(key1, \"tableofcontents.\", &key2)) {\n+\t\tif (!strcmp(key2, \"version\")) {\n+\t\t\tint version = git_config_int(key, value);\n+\n+\t\t\tif (version != 1) {\n+\t\t\t\twarning(_(\"table of contents version %d not understood\"), version);\n+\t\t\t\treturn -1;\n+\t\t\t}\n+\t\t}\n+\n+\t\treturn 0;\n+\t}\n+\n+\tid_end = strchr(key1, '.');\n+\n+\t/*\n+\t * If this key is of the form \"bundle.\" with no third item,\n+\t * then we do not know about it. We should ignore it. Later versions\n+\t * might start caring about this data on an optional basis. Increase\n+\t * the version number to add keys that must be understood.\n+\t */\n+\tif (!id_end)\n+\t\treturn 0;\n+\n+\tstrbuf_add(&id, key1, id_end - key1);\n+\tkey2 = id_end + 1;\n+\n+\tinfo_lookup.id = id.buf;\n+\thashmap_entry_init(&info_lookup.ent, strhash(info_lookup.id));\n+\tif (!(info = hashmap_get_entry(toc, &info_lookup, ent, NULL))) {\n+\t\tCALLOC_ARRAY(info, 1);\n+\t\tinfo->id = strbuf_detach(&id, NULL);\n+\t\tstrbuf_init(&info->file, 0);\n+\t\thashmap_entry_init(&info->ent, strhash(info->id));\n+\t\thashmap_add(toc, &info->ent);\n+\t}\n+\n+\tif (!strcmp(key2, \"uri\")) {\n+\t\tif (info->uri)\n+\t\t\twarning(_(\"duplicate 'uri' value for id '%s'\"), info->id);\n+\t\telse\n+\t\t\tinfo->uri = xstrdup(value);\n+\t\treturn 0;\n+\t} else if (!strcmp(key2, \"timestamp\")) {\n+\t\tif (info->timestamp)\n+\t\t\twarning(_(\"duplicate 'timestamp' value for id '%s'\"), info->id);\n+\t\telse\n+\t\t\tinfo->timestamp = git_config_int64(key, value);\n+\t\treturn 0;\n+\t} else if (!strcmp(key2, \"requires\")) {\n+\t\tif (info->requires_id)\n+\t\t\twarning(_(\"duplicate 'requires' value for id '%s'\"), info->id);\n+\t\telse\n+\t\t\tinfo->requires_id = xstrdup(value);\n+\t\treturn 0;\n+\t} else if (!strcmp(key2, \"filter\")) {\n+\t\tif (info->filter_str)\n+\t\t\twarning(_(\"duplicate 'filter' value for id '%s'\"), info->id);\n+\t\telse\n+\t\t\tinfo->filter_str = xstrdup(value);\n+\t\treturn 0;\n+\t}\n+\n+\t/* Return 0 here to ignore unknown options. */\n+\treturn 0;\n+}\n+\n static void download_uri_to_file(const char *uri, const char *file)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n@@ -289,13 +398,14 @@ static void unbundle_fetched_bundle(struct remote_bundle_info *info)\n \n static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n {\n-\tint ret = 0;\n+\tint ret = 0, used_hashmap = 0;\n \tint progress = isatty(2);\n \tchar *bundle_uri;\n \tstruct remote_bundle_info first_file = {\n \t\t.file = STRBUF_INIT,\n \t};\n \tstruct remote_bundle_info *stack = NULL;\n+\tstruct hashmap toc = { 0 };\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"progress\", &progress,\n@@ -319,15 +429,31 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \n \t/*\n \t * Step 2: Check if the file is a bundle (if so, add it to the\n-\t * stack and move to step 3).\n+\t * stack and move to step 3). Otherwise, expect it to be a table\n+\t * of contents. Use the table to populate a hashtable of bundles\n+\t * and push the most recent bundle to the stack.\n \t */\n \n \tif (is_bundle(first_file.file.buf, 1)) {\n \t\t/* The simple case: only one file, no stack to worry about. */\n \t\tstack = &first_file;\n \t} else {\n-\t\t/* TODO: Expect and parse a table of contents. */\n-\t\tdie(_(\"unexpected data at bundle URI\"));\n+\t\tstruct hashmap_iter iter;\n+\t\tstruct remote_bundle_info *info;\n+\t\ttimestamp_t max_time = 0;\n+\n+\t\t/* populate a hashtable with all relevant bundles. */\n+\t\tused_hashmap = 1;\n+\t\thashmap_init(&toc, remote_bundle_cmp, NULL, 0);\n+\t\tgit_config_from_file(parse_toc_config, first_file.file.buf, &toc);\n+\n+\t\t/* initialize stack using timestamp heuristic. */\n+\t\thashmap_for_each_entry(&toc, &iter, info, ent) {\n+\t\t\tif (info->timestamp > max_time || !stack) {\n+\t\t\t\tstack = info;\n+\t\t\t\tmax_time = info->timestamp;\n+\t\t\t}\n+\t\t}\n \t}\n \n \t/*\n@@ -357,6 +483,7 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t\tif (bundle_fd < 0)\n \t\t\tdie(_(\"failed to read bundle from '%s'\"), stack->uri);\n \n+\t\treprepare_packed_git(the_repository);\n \t\tfor_each_string_list_item(prereq, &header.prerequisites) {\n \t\t\tstruct object_info info = OBJECT_INFO_INIT;\n \t\t\tstruct object_id *oid = prereq->util;\n@@ -373,11 +500,28 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \n \t\tif (valid) {\n \t\t\tunbundle_fetched_bundle(stack);\n-\t\t} else if (stack->next_id) {\n+\t\t} else if (stack->pushed) {\n+\t\t\tdie(_(\"bundle '%s' still invalid after downloading required bundle\"), stack->id);\n+\t\t} else if (stack->requires_id) {\n \t\t\t/*\n \t\t\t * Load the next bundle from the hashtable and\n \t\t\t * push it onto the stack.\n \t\t\t */\n+\t\t\tstruct remote_bundle_info *info;\n+\t\t\tstruct remote_bundle_info info_lookup = { 0 };\n+\t\t\tinfo_lookup.id = stack->requires_id;\n+\n+\t\t\thashmap_entry_init(&info_lookup.ent, strhash(info_lookup.id));\n+\t\t\tif ((info = hashmap_get_entry(&toc, &info_lookup, ent, NULL))) {\n+\t\t\t\t/* Push onto the stack */\n+\t\t\t\tstack->pushed = 1;\n+\t\t\t\tinfo->stack_next = stack;\n+\t\t\t\tstack = info;\n+\t\t\t\tcontinue;\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"unable to find bundle '%s' required by bundle '%s'\"),\n+\t\t\t\t stack->requires_id, stack->id);\n+\t\t\t}\n \t\t} else {\n \t\t\tdie(_(\"bundle from '%s' has missing prerequisites and no dependent bundle\"),\n \t\t\t stack->uri);\n@@ -386,6 +530,17 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t\tstack = stack->stack_next;\n \t}\n \n+\tif (used_hashmap) {\n+\t\tstruct hashmap_iter iter;\n+\t\tstruct remote_bundle_info *info;\n+\t\thashmap_for_each_entry(&toc, &iter, info, ent) {\n+\t\t\tfree(info->id);\n+\t\t\tfree(info->uri);\n+\t\t\tfree(info->requires_id);\n+\t\t\tfree(info->filter_str);\n+\t\t}\n+\t\thashmap_clear_and_free(&toc, struct remote_bundle_info, ent);\n+\t}\n \tfree(bundle_uri);\n \treturn ret;\n }\n-- \ngitgitgadget\n\n"},{"id":"449313","messageId":"bdd5c664cde070a93d7e698aff2ab11ce7557ee2.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 11/25] bundle: allow relative URLs in table of contents","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:49Z","receivedAt":"2022-02-23T18:31:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen hosting bundle data, it can be helpful to distribute that data\nacross multiple CDNs. This might require a change in the base URI, all\nthe way to the domain name. If all bundles require an absolute URI in\ntheir 'uri' value, then every push to a CDN would require altering the\ntable of contents to match the expected domain and exact location within\nit.\n\nAllow the table of contents to specify a relative URI for the bundles.\nThis allows easier distribution of bundle data.\n\nRFC-TODO: An earlier change referenced relative URLs, but it was not\nimplemented until this change.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/bundle.c | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 27da5e3737f..ec969a62ae1 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -10,6 +10,7 @@\n #include \"config.h\"\n #include \"packfile.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"remote.h\"\n \n /*\n * Basic handler for bundle files to connect repositories via sneakernet.\n@@ -453,6 +454,8 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \n \t\t/* initialize stack using timestamp heuristic. */\n \t\thashmap_for_each_entry(&toc, &iter, info, ent) {\n+\t\t\tchar *old_uri;\n+\n \t\t\t/* Skip if filter does not match. */\n \t\t\tif (!filter && info->filter_str)\n \t\t\t\tcontinue;\n@@ -460,6 +463,10 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t\t\t (!info->filter_str || strcasecmp(filter, info->filter_str)))\n \t\t\t\tcontinue;\n \n+\t\t\told_uri = info->uri;\n+\t\t\tinfo->uri = relative_url(bundle_uri, info->uri, NULL);\n+\t\t\tfree(old_uri);\n+\n \t\t\t/*\n \t\t\t * Now that the filter matches, start with the\n \t\t\t * bundle with largest timestamp.\n-- \ngitgitgadget\n\n"},{"id":"449314","messageId":"fa0e83b39dd3c3bb16e932c39aff080a1fdf8fe4.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 10/25] bundle: add --filter option to 'fetch'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:48Z","receivedAt":"2022-02-23T18:31:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen a repository uses an object filter for partial clone, the 'git\nbundle fetch' command should try to download bundles that match that\nfilter.\n\nTeach 'git bundle fetch' to take a '--filter' option and then only\nconsider bundles that match that filter (or lack thereof). This allows\nthe bundle server to advertise different sets of bundles for different\nfilters.\n\nAdd some verification to be sure that the bundle we downloaded actually\nuses that filter. This is especially important when no filter is\nrequested but the downloaded bundle _does_ have a filter.\n\nRFC-TODO: add tests for the happy path.\n\nRFC-TODO: add tests for these validations.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/bundle.c | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 66f3b3c9376..27da5e3737f 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -9,6 +9,7 @@\n #include \"refs.h\"\n #include \"config.h\"\n #include \"packfile.h\"\n+#include \"list-objects-filter-options.h\"\n \n /*\n * Basic handler for bundle files to connect repositories via sneakernet.\n@@ -406,10 +407,13 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t};\n \tstruct remote_bundle_info *stack = NULL;\n \tstruct hashmap toc = { 0 };\n+\tconst char *filter = NULL;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"progress\", &progress,\n \t\t\t N_(\"show progress meter\")),\n+\t\tOPT_STRING(0, \"filter\", &filter,\n+\t\t\t N_(\"filter-spec\"), N_(\"only install bundles matching this filter\")),\n \t\tOPT_END()\n \t};\n \n@@ -449,6 +453,17 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \n \t\t/* initialize stack using timestamp heuristic. */\n \t\thashmap_for_each_entry(&toc, &iter, info, ent) {\n+\t\t\t/* Skip if filter does not match. */\n+\t\t\tif (!filter && info->filter_str)\n+\t\t\t\tcontinue;\n+\t\t\tif (filter &&\n+\t\t\t (!info->filter_str || strcasecmp(filter, info->filter_str)))\n+\t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * Now that the filter matches, start with the\n+\t\t\t * bundle with largest timestamp.\n+\t\t\t */\n \t\t\tif (info->timestamp > max_time || !stack) {\n \t\t\t\tstack = info;\n \t\t\t\tmax_time = info->timestamp;\n@@ -468,6 +483,7 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \twhile (stack) {\n \t\tint valid = 1;\n \t\tint bundle_fd;\n+\t\tconst char *filter_str = NULL;\n \t\tstruct string_list_item *prereq;\n \t\tstruct bundle_header header = BUNDLE_HEADER_INIT;\n \n@@ -483,6 +499,16 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t\tif (bundle_fd < 0)\n \t\t\tdie(_(\"failed to read bundle from '%s'\"), stack->uri);\n \n+\t\tif (header.filter)\n+\t\t\tfilter_str = list_objects_filter_spec(header.filter);\n+\n+\t\tif (filter && (!filter_str || strcasecmp(filter, filter_str)))\n+\t\t\tdie(_(\"bundle from '%s' does not match expected filter\"),\n+\t\t\t stack->uri);\n+\t\tif (!filter && filter_str)\n+\t\t\tdie(_(\"bundle from '%s' has an unexpected filter\"),\n+\t\t\t stack->uri);\n+\n \t\treprepare_packed_git(the_repository);\n \t\tfor_each_string_list_item(prereq, &header.prerequisites) {\n \t\t\tstruct object_info info = OBJECT_INFO_INIT;\n-- \ngitgitgadget\n\n"},{"id":"449315","messageId":"f170cb27ce70d142aef3884a8866b87d4822be5f.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 12/25] bundle: make it easy to call 'git bundle fetch'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:50Z","receivedAt":"2022-02-23T18:31:47Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nFuture changes will integrate 'git bundle fetch' into the 'git clone'\nand 'git fetch' operations. Make it easy to fetch bundles via a helper\nmethod.\n\nSigned-off-by: Derrick Stolee \n---\n bundle.c | 21 +++++++++++++++++++++\n bundle.h | 9 +++++++++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/bundle.c b/bundle.c\nindex 3d97de40ef0..9e1b5300366 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -649,3 +649,24 @@ int unbundle(struct repository *r, struct bundle_header *header,\n \t\treturn error(_(\"index-pack died\"));\n \treturn 0;\n }\n+\n+int fetch_bundle_uri(const char *bundle_uri,\n+\t\t const char *filter)\n+{\n+\tint res = 0;\n+\tstruct strvec args = STRVEC_INIT;\n+\n+\tstrvec_pushl(&args, \"bundle\", \"fetch\", NULL);\n+\n+\tif (filter)\n+\t\tstrvec_pushf(&args, \"--filter=%s\", filter);\n+\tstrvec_push(&args, bundle_uri);\n+\n+\tif (run_command_v_opt(args.v, RUN_GIT_CMD)) {\n+\t\twarning(_(\"failed to download bundle from uri '%s'\"), bundle_uri);\n+\t\tres = 1;\n+\t}\n+\n+\tstrvec_clear(&args);\n+\treturn res;\n+}\ndiff --git a/bundle.h b/bundle.h\nindex eb026153d56..bf865b19687 100644\n--- a/bundle.h\n+++ b/bundle.h\n@@ -45,4 +45,13 @@ int unbundle(struct repository *r, struct bundle_header *header,\n int list_bundle_refs(struct bundle_header *header,\n \t\tint argc, const char **argv);\n \n+struct list_objects_filter_options;\n+/**\n+ * Fetch bundles from the given URI with the given filter.\n+ *\n+ * Uses 'git bundle fetch' as a subprocess.\n+ */\n+int fetch_bundle_uri(const char *bundle_uri,\n+\t\t const char *filter);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"449316","messageId":"050725d90ef019ca2684ec0afbfd701efea7f88a.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 13/25] clone: add --bundle-uri option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:51Z","receivedAt":"2022-02-23T18:31:48Z","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\nThe 'git bundle fetch' command allows users to bootstrap a repository\nusing a set of bundles. However, this would require them to use 'git\ninit' first, followed by the 'git bundle fetch', and finally add a\nremote, fetch, and checkout the branch they want.\n\nInstead, integrate this workflow directly into 'git clone' with the\n--bundle-uri' option. If the user is aware of a bundle server, then they\ncan tell Git to bootstrap the new repository with these bundles before\nfetching the remaining objects from the origin server.\n\nRFC-TODO: Document this option in git-clone.txt.\n\nRFC-TODO: I added a comment about the location of this code being\nnecessary for the later step of auto-discovering the bundle URI from the\norigin server. This is probably not actually a requirement, but rather a\npain point around how I implemented the feature. If a --bundle-uri\noption is specified, but SSH is used for the clone, then the SSH\nconnection is left open while Git downloads bundles from another server.\nThis is sub-optimal and should be reconsidered when fully reviewed.\n\nRFC-TODO: create tests for this option with a variety of URI types.\n\nRFC-TODO: a simple end-to-end test is available at the end of the\nseries.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/clone.c | 33 +++++++++++++++++++++++++++++++++\n 1 file changed, 33 insertions(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 9c29093b352..6df3d513dc4 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -33,6 +33,7 @@\n #include \"packfile.h\"\n #include \"list-objects-filter-options.h\"\n #include \"hook.h\"\n+#include \"bundle.h\"\n \n /*\n * Overall FIXMEs:\n@@ -74,6 +75,7 @@ static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options;\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@@ -155,6 +157,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@@ -1185,6 +1189,35 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \trefs = transport_get_remote_refs(transport, &transport_ls_refs_options);\n \n+\t/*\n+\t * NOTE: The bundle URI download takes place after transport_get_remote_refs()\n+\t * because a later change will introduce a check for recommended features,\n+\t * which might include a recommended bundle URI.\n+\t */\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\tconst char *filter = NULL;\n+\n+\t\tif (filter_options.filter_spec.nr)\n+\t\t\tfilter = expand_list_objects_filter_spec(&filter_options);\n+\t\t/*\n+\t\t * Set the config for fetching from this bundle URI in the\n+\t\t * future, but do it before fetch_bundle_uri() which might\n+\t\t * un-set it (for instance, if there is no table of contents).\n+\t\t */\n+\t\tgit_config_set(\"fetch.bundleuri\", bundle_uri);\n+\t\tif (filter)\n+\t\t\tgit_config_set(\"fetch.bundlefilter\", filter);\n+\n+\t\tif (!fetch_bundle_uri(bundle_uri, filter))\n+\t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n+\t\t\t\tbundle_uri);\n+\t}\n+\n \tif (refs)\n \t\tmapped_refs = wanted_peer_refs(refs, &remote->fetch);\n \n-- \ngitgitgadget\n\n"},{"id":"449317","messageId":"cdc81b2160ee4f5b98bc4eb24b0a933426eed494.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 14/25] clone: --bundle-uri cannot be combined with --depth","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:52Z","receivedAt":"2022-02-23T18:31:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 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\nRFC-TODO: add a test case for this error message.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/clone.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 6df3d513dc4..cfe3d96047a 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -912,6 +912,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_no_checkout = 1;\n \t}\n \n+\tif (bundle_uri) {\n+\t\tif (deepen)\n+\t\t\tdie(_(\"--bundle-uri is incompatible with --depth, --shallow-since, and --shallow-exclude\"));\n+\t}\n+\n \trepo_name = argv[0];\n \n \tpath = get_repo_path(repo_name, &is_bundle);\n-- \ngitgitgadget\n\n"},{"id":"449318","messageId":"7ec7ba3f328f9d6fa7f919ce18a138840d44ee49.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 15/25] config: add git_config_get_timestamp()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:53Z","receivedAt":"2022-02-23T18:31:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe existing config parsing methods do not include a way to consistently\nparse timestamps across all platforms. Recall that \"unsigned long\" is\n32 bits on 64-bit Windows, so git_config_get_ulong() is insufficient.\n\nAdding a new type requires quite a bit of boilerplate to match the style\nof other types.\n\nRFC-QUESTION: Would this be better to use uintmax_t, which could be cast\nto timestamp_t or other types more robust than \"unsigned long\"?\n\nSigned-off-by: Derrick Stolee \n---\n config.c | 39 +++++++++++++++++++++++++++++++++++++++\n config.h | 14 ++++++++++++++\n 2 files changed, 53 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex e0c03d154c9..84021b7d504 100644\n--- a/config.c\n+++ b/config.c\n@@ -1228,6 +1228,15 @@ int git_parse_ulong(const char *value, unsigned long *ret)\n \treturn 1;\n }\n \n+int git_parse_timestamp(const char *value, timestamp_t *ret)\n+{\n+\tuintmax_t tmp;\n+\tif (!git_parse_unsigned(value, &tmp, maximum_unsigned_value_of_type(timestamp_t)))\n+\t\treturn 0;\n+\t*ret = tmp;\n+\treturn 1;\n+}\n+\n int git_parse_ssize_t(const char *value, ssize_t *ret)\n {\n \tintmax_t tmp;\n@@ -1296,6 +1305,14 @@ unsigned long git_config_ulong(const char *name, const char *value)\n \treturn ret;\n }\n \n+timestamp_t git_config_timestamp(const char *name, const char *value)\n+{\n+\ttimestamp_t ret;\n+\tif (!git_parse_timestamp(value, &ret))\n+\t\tdie_bad_number(name, value);\n+\treturn ret;\n+}\n+\n ssize_t git_config_ssize_t(const char *name, const char *value)\n {\n \tssize_t ret;\n@@ -2328,6 +2345,16 @@ int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned lon\n \t\treturn 1;\n }\n \n+int git_configset_get_timestamp(struct config_set *cs, const char *key, timestamp_t *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_timestamp(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n int git_configset_get_bool(struct config_set *cs, const char *key, int *dest)\n {\n \tconst char *value;\n@@ -2471,6 +2498,13 @@ int repo_config_get_ulong(struct repository *repo,\n \treturn git_configset_get_ulong(repo->config, key, dest);\n }\n \n+int repo_config_get_timestamp(struct repository *repo,\n+\t\t\t const char *key, timestamp_t *dest)\n+{\n+\tgit_config_check_init(repo);\n+\treturn git_configset_get_timestamp(repo->config, key, dest);\n+}\n+\n int repo_config_get_bool(struct repository *repo,\n \t\t\t const char *key, int *dest)\n {\n@@ -2544,6 +2578,11 @@ int git_config_get_ulong(const char *key, unsigned long *dest)\n \treturn repo_config_get_ulong(the_repository, key, dest);\n }\n \n+int git_config_get_timestamp(const char *key, timestamp_t *dest)\n+{\n+\treturn repo_config_get_timestamp(the_repository, key, dest);\n+}\n+\n int git_config_get_bool(const char *key, int *dest)\n {\n \treturn repo_config_get_bool(the_repository, key, dest);\ndiff --git a/config.h b/config.h\nindex ab0106d2875..a6e4d35da0a 100644\n--- a/config.h\n+++ b/config.h\n@@ -206,6 +206,7 @@ int config_with_options(config_fn_t fn, void *,\n \n int git_parse_ssize_t(const char *, ssize_t *);\n int git_parse_ulong(const char *, unsigned long *);\n+int git_parse_timestamp(const char *, timestamp_t *);\n \n /**\n * Same as `git_config_bool`, except that it returns -1 on error rather\n@@ -226,6 +227,11 @@ int64_t git_config_int64(const char *, const char *);\n */\n unsigned long git_config_ulong(const char *, const char *);\n \n+/**\n+ * Identical to `git_config_int`, but for (unsigned) timestamps.\n+ */\n+timestamp_t git_config_timestamp(const char *name, const char *value);\n+\n ssize_t git_config_ssize_t(const char *, const char *);\n \n /**\n@@ -469,6 +475,7 @@ int git_configset_get_string(struct config_set *cs, const char *key, char **dest\n int git_configset_get_string_tmp(struct config_set *cs, const char *key, const char **dest);\n int git_configset_get_int(struct config_set *cs, const char *key, int *dest);\n int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest);\n+int git_configset_get_timestamp(struct config_set *cs, const char *key, timestamp_t *dest);\n int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);\n int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);\n int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);\n@@ -489,6 +496,8 @@ int repo_config_get_int(struct repository *repo,\n \t\t\tconst char *key, int *dest);\n int repo_config_get_ulong(struct repository *repo,\n \t\t\t const char *key, unsigned long *dest);\n+int repo_config_get_timestamp(struct repository *repo,\n+\t\t\t const char *key, timestamp_t *dest);\n int repo_config_get_bool(struct repository *repo,\n \t\t\t const char *key, int *dest);\n int repo_config_get_bool_or_int(struct repository *repo,\n@@ -558,6 +567,11 @@ int git_config_get_int(const char *key, int *dest);\n */\n int git_config_get_ulong(const char *key, unsigned long *dest);\n \n+/**\n+ * Similar to `git_config_get_int` but for (unsigned) timestamps.\n+ */\n+int git_config_get_timestamp(const char *key, timestamp_t *dest);\n+\n /**\n * Finds and parses the value into a boolean value, for the configuration\n * variable `key` respecting keywords like \"true\" and \"false\". Integer\n-- \ngitgitgadget\n\n"},{"id":"449319","messageId":"cf184fd1d34b5613fd1e28fd5da75a475c25884f.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 18/25] connect.c: refactor sending of agent & object-format","fromName":"Ævar Arnfjörð Bjarmason via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:56Z","receivedAt":"2022-02-23T18:31:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=\n \n\nRefactor the sending of the \"agent\" and \"object-format\" capabilities\ninto a function.\n\nThis was added in its current form in ab67235bc4 (connect: parse v2\nrefs with correct hash algorithm, 2020-05-25). When we connect to a v2\nserver we need to know about its object-format, and it needs to know\nabout ours. Since most things in connect.c and transport.c piggy-back\non the eager getting of remote refs via the handshake() those commands\ncan make use of the just-sent-over object-format by ls-refs.\n\nBut I'm about to add a command that may come after ls-refs, and may\nnot, but we need the server to know about our user-agent and\nobject-format. So let's split this into a function.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Derrick Stolee \n---\n connect.c | 33 ++++++++++++++++++++-------------\n 1 file changed, 20 insertions(+), 13 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex eaf7d6d2618..9d78d681e95 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -473,6 +473,24 @@ void check_stateless_delimiter(int stateless_rpc,\n \t\tdie(\"%s\", error);\n }\n \n+static void send_capabilities(int fd_out, struct packet_reader *reader)\n+{\n+\tconst char *hash_name;\n+\n+\tif (server_supports_v2(\"agent\", 0))\n+\t\tpacket_write_fmt(fd_out, \"agent=%s\", git_user_agent_sanitized());\n+\n+\tif (server_feature_v2(\"object-format\", &hash_name)) {\n+\t\tint hash_algo = hash_algo_by_name(hash_name);\n+\t\tif (hash_algo == GIT_HASH_UNKNOWN)\n+\t\t\tdie(_(\"unknown object format '%s' specified by server\"), hash_name);\n+\t\treader->hash_algo = &hash_algos[hash_algo];\n+\t\tpacket_write_fmt(fd_out, \"object-format=%s\", reader->hash_algo->name);\n+\t} else {\n+\t\treader->hash_algo = &hash_algos[GIT_HASH_SHA1];\n+\t}\n+}\n+\n struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,\n \t\t\t struct ref **list, int for_push,\n \t\t\t struct transport_ls_refs_options *transport_options,\n@@ -480,7 +498,6 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,\n \t\t\t int stateless_rpc)\n {\n \tint i;\n-\tconst char *hash_name;\n \tstruct strvec *ref_prefixes = transport_options ?\n \t\t&transport_options->ref_prefixes : NULL;\n \tchar **unborn_head_target = transport_options ?\n@@ -490,18 +507,8 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,\n \tif (server_supports_v2(\"ls-refs\", 1))\n \t\tpacket_write_fmt(fd_out, \"command=ls-refs\\n\");\n \n-\tif (server_supports_v2(\"agent\", 0))\n-\t\tpacket_write_fmt(fd_out, \"agent=%s\", git_user_agent_sanitized());\n-\n-\tif (server_feature_v2(\"object-format\", &hash_name)) {\n-\t\tint hash_algo = hash_algo_by_name(hash_name);\n-\t\tif (hash_algo == GIT_HASH_UNKNOWN)\n-\t\t\tdie(_(\"unknown object format '%s' specified by server\"), hash_name);\n-\t\treader->hash_algo = &hash_algos[hash_algo];\n-\t\tpacket_write_fmt(fd_out, \"object-format=%s\", reader->hash_algo->name);\n-\t} else {\n-\t\treader->hash_algo = &hash_algos[GIT_HASH_SHA1];\n-\t}\n+\t/* Send capabilities */\n+\tsend_capabilities(fd_out, reader);\n \n \tif (server_options && server_options->nr &&\n \t server_supports_v2(\"server-option\", 1))\n-- \ngitgitgadget\n\n"},{"id":"449320","messageId":"42d351f4aae9532d2787e72ae4be2e08fe614f44.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 17/25] fetch: fetch bundles before fetching original data","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:55Z","receivedAt":"2022-02-23T18:31:55Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIf a user cloned using a bundle URI, then they might want to re-use it\nto download new bundles during 'git fetch' before fetching the remaining\nobjects from the origin server. Use the 'fetch.bundleURI' config as the\nindicator for whether this extra step should happen.\n\nDo not fetch bundles if --dry-run is specified.\n\nRFC-TODO: add tests.\n\nRFC-TODO: update Documentation/git-fetch.txt\n\nRFC-TODO: update Documentation/config/\n\nSigned-off-by: Derrick Stolee \n---\n builtin/fetch.c | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 6f5e1578639..c0fece55632 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -29,6 +29,7 @@\n #include \"commit-graph.h\"\n #include \"shallow.h\"\n #include \"worktree.h\"\n+#include \"bundle.h\"\n \n #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)\n \n@@ -2081,6 +2082,22 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t/* FETCH_HEAD never gets updated in --dry-run mode */\n \tif (dry_run)\n \t\twrite_fetch_head = 0;\n+\telse {\n+\t\t/*\n+\t\t * --dry-run mode skips bundle downloads, which might\n+\t\t * update some refs.\n+\t\t */\n+\t\tchar *bundle_uri = NULL;\n+\t\tgit_config_get_string(\"fetch.bundleuri\", &bundle_uri);\n+\n+\t\tif (bundle_uri) {\n+\t\t\tchar *filter = NULL;\n+\t\t\tgit_config_get_string(\"fetch.bundlefilter\", &filter);\n+\t\t\tfetch_bundle_uri(bundle_uri, filter);\n+\t\t\tfree(bundle_uri);\n+\t\t\tfree(filter);\n+\t\t}\n+\t}\n \n \tif (all) {\n \t\tif (argc == 1)\n-- \ngitgitgadget\n\n"},{"id":"449321","messageId":"5f7f54e9205acbe072ad436e83e3e98a36f1df3b.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 16/25] bundle: only fetch bundles if timestamp is new","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:54Z","receivedAt":"2022-02-23T18:31:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIf a bundle server is providing a table of contents with timestamps for\nthe bundles, then we can store the most-recent timestamp and use that as\na test if the bundle server has any new information. Teach 'git bundle\nfetch' to store the timestamp in the config file as\n'fetch.bundleTimestamp' and compare the existing value to the\nmost-recent timestamp in the bundle server's table of contents. If the\nnew timestamp is at most the stored timestamp, then exit early (with\nsuccess). If the new timestamp is greater than the stored timestamp,\nthen continue with the normal fetch logic of downloading the most-recent\nbundle until all missing objects are satisfied. Store that new timestamp\nin the config for next time.\n\nRFC-TODO: Update documentation of 'git bundle fetch' to match his new\nbehavior.\n\nRFC-TODO: Add 'fetch.bundleTimestamp' to Documentation/config/\n\nSigned-off-by: Derrick Stolee \n---\n builtin/bundle.c | 21 ++++++++++++++++++++-\n 1 file changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex ec969a62ae1..cab99ee2b15 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -409,6 +409,9 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \tstruct remote_bundle_info *stack = NULL;\n \tstruct hashmap toc = { 0 };\n \tconst char *filter = NULL;\n+\tconst char *timestamp_key = \"fetch.bundletimestamp\";\n+\ttimestamp_t stored_time = 0;\n+\ttimestamp_t max_time = 0;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"progress\", &progress,\n@@ -424,6 +427,8 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \tif (!startup_info->have_repository)\n \t\tdie(_(\"'fetch' requires a repository\"));\n \n+\tgit_config_get_timestamp(timestamp_key, &stored_time);\n+\n \t/*\n \t * Step 1: determine protocol for uri, and download contents to\n \t * a temporary location.\n@@ -445,7 +450,6 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t} else {\n \t\tstruct hashmap_iter iter;\n \t\tstruct remote_bundle_info *info;\n-\t\ttimestamp_t max_time = 0;\n \n \t\t/* populate a hashtable with all relevant bundles. */\n \t\tused_hashmap = 1;\n@@ -476,6 +480,13 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t\t\t\tmax_time = info->timestamp;\n \t\t\t}\n \t\t}\n+\n+\t\ttrace2_data_intmax(\"bundle\", the_repository, \"max_time\", max_time);\n+\t\ttrace2_data_intmax(\"bundle\", the_repository, \"stored_time\", stored_time);\n+\n+\t\t/* Skip fetching bundles if data isn't new enough. */\n+\t\tif (max_time <= stored_time)\n+\t\t\tgoto cleanup;\n \t}\n \n \t/*\n@@ -563,6 +574,14 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n \t\tstack = stack->stack_next;\n \t}\n \n+\tif (max_time) {\n+\t\tstruct strbuf tstr = STRBUF_INIT;\n+\t\tstrbuf_addf(&tstr, \"%\"PRIuMAX\"\", max_time);\n+\t\tgit_config_set_gently(timestamp_key, tstr.buf);\n+\t\tstrbuf_release(&tstr);\n+\t}\n+\n+cleanup:\n \tif (used_hashmap) {\n \t\tstruct hashmap_iter iter;\n \t\tstruct remote_bundle_info *info;\n-- \ngitgitgadget\n\n"},{"id":"449322","messageId":"9a72854af51cae0813cf98318f5ea20a9c8bd559.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 19/25] protocol-caps: implement cap_features()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:57Z","receivedAt":"2022-02-23T18:32:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'features' capability sends a list of \"key=value\" pairs from the\nserver. These are a set of fixed config values, all prefixed with\n\"serve.\" to avoid conflicting with other config values of similar names.\n\nThe initial set chosen here are:\n\n* bundleURI: Allow advertising one or more bundle servers by URI.\n\n* partialCloneFilter: Advertise one or more recommended partial clone\n filters.\n\n* sparseCheckout: Advertise that this repository recommends using the\n sparse-checkout feature in cone mode.\n\nThe client will have the choice to enable these features.\n\nRFC-TODO: Create Documentation/config/serve.txt\n\nSigned-off-by: Derrick Stolee \n---\n protocol-caps.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++\n protocol-caps.h | 1 +\n 2 files changed, 67 insertions(+)\n\ndiff --git a/protocol-caps.c b/protocol-caps.c\nindex bbde91810ac..88b01c4133e 100644\n--- a/protocol-caps.c\n+++ b/protocol-caps.c\n@@ -8,6 +8,7 @@\n #include \"object-store.h\"\n #include \"string-list.h\"\n #include \"strbuf.h\"\n+#include \"config.h\"\n \n struct requested_info {\n \tunsigned size : 1;\n@@ -111,3 +112,68 @@ int cap_object_info(struct repository *r, struct packet_reader *request)\n \n \treturn 0;\n }\n+\n+static void send_lines(struct repository *r, struct packet_writer *writer,\n+\t\t struct string_list *str_list)\n+{\n+\tstruct string_list_item *item;\n+\n+\tif (!str_list->nr)\n+\t\treturn;\n+\n+\tfor_each_string_list_item (item, str_list) {\n+\t\tpacket_writer_write(writer, \"%s\", item->string);\n+\t}\n+}\n+\n+int cap_features(struct repository *r, struct packet_reader *request)\n+{\n+\tstruct packet_writer writer;\n+\tstruct string_list feature_list = STRING_LIST_INIT_DUP;\n+\tint i = 0;\n+\tconst char *keys[] = {\n+\t\t\"bundleuri\",\n+\t\t\"partialclonefilter\",\n+\t\t\"sparsecheckout\",\n+\t\tNULL\n+\t};\n+\tstruct strbuf serve_feature = STRBUF_INIT;\n+\tstruct strbuf key_equals_value = STRBUF_INIT;\n+\tsize_t len;\n+\tstrbuf_add(&serve_feature, \"serve.\", 6);\n+\tlen = serve_feature.len;\n+\n+\tpacket_writer_init(&writer, 1);\n+\n+\twhile (keys[i]) {\n+\t\tstruct string_list_item *item;\n+\t\tconst struct string_list *values = NULL;\n+\t\tstrbuf_setlen(&serve_feature, len);\n+\t\tstrbuf_addstr(&serve_feature, keys[i]);\n+\n+\t\tvalues = repo_config_get_value_multi(r, serve_feature.buf);\n+\n+\t\tif (values) {\n+\t\t\tfor_each_string_list_item(item, values) {\n+\t\t\t\tstrbuf_reset(&key_equals_value);\n+\t\t\t\tstrbuf_addstr(&key_equals_value, keys[i]);\n+\t\t\t\tstrbuf_addch(&key_equals_value, '=');\n+\t\t\t\tstrbuf_addstr(&key_equals_value, item->string);\n+\n+\t\t\t\tstring_list_append(&feature_list, key_equals_value.buf);\n+\t\t\t}\n+\t\t}\n+\n+\t\ti++;\n+\t}\n+\tstrbuf_release(&serve_feature);\n+\tstrbuf_release(&key_equals_value);\n+\n+\tsend_lines(r, &writer, &feature_list);\n+\n+\tstring_list_clear(&feature_list, 1);\n+\n+\tpacket_flush(1);\n+\n+\treturn 0;\n+}\ndiff --git a/protocol-caps.h b/protocol-caps.h\nindex 15c4550360c..681d2106d88 100644\n--- a/protocol-caps.h\n+++ b/protocol-caps.h\n@@ -4,5 +4,6 @@\n struct repository;\n struct packet_reader;\n int cap_object_info(struct repository *r, struct packet_reader *request);\n+int cap_features(struct repository *r, struct packet_reader *request);\n \n #endif /* PROTOCOL_CAPS_H */\n-- \ngitgitgadget\n\n"},{"id":"449323","messageId":"a6779c713afc19bfaf0ffa10bf97346b9b4e96fb.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 21/25] serve: advertise 'features' when config exists","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:59Z","receivedAt":"2022-02-23T18:32:02Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'features' capability allows a server to recommend some Git features\nat a high level. Previous changes implemented the capability so servers\nunderstand it, but it was never advertised.\n\nNow, allow it to be advertised, but only when the capability will\nactually _do_ something. That is, advertise if and only if a config\nvalue exists with the prefix \"serve.\". This avoids unnecessary round\ntrips for an empty result.\n\nSigned-off-by: Derrick Stolee \n---\n serve.c | 18 +++++++++++++++---\n t/t5701-git-serve.sh | 9 +++++++++\n 2 files changed, 24 insertions(+), 3 deletions(-)\n\ndiff --git a/serve.c b/serve.c\nindex a1c853dda1f..7dcabb68147 100644\n--- a/serve.c\n+++ b/serve.c\n@@ -18,12 +18,24 @@ static int always_advertise(struct repository *r,\n \treturn 1;\n }\n \n-static int never_advertise(struct repository *r,\n-\t\t\t struct strbuf *value)\n+static int key_serve_prefix(const char *key, const char *value, void *data)\n {\n+\tint *signal = data;\n+\tif (!strncmp(key, \"serve.\", 6)) {\n+\t\t*signal = 1;\n+\t\treturn 1;\n+\t}\n \treturn 0;\n }\n \n+static int has_serve_config(struct repository *r,\n+\t\t\t struct strbuf *value)\n+{\n+\tint signal = 0;\n+\trepo_config(r, key_serve_prefix, &signal);\n+\treturn signal;\n+}\n+\n static int agent_advertise(struct repository *r,\n \t\t\t struct strbuf *value)\n {\n@@ -144,7 +156,7 @@ static struct protocol_capability capabilities[] = {\n \t},\n \t{\n \t\t.name = \"features\",\n-\t\t.advertise = never_advertise,\n+\t\t.advertise = has_serve_config,\n \t\t.command = cap_features,\n \t},\n };\ndiff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\nindex 1896f671cb3..6ef721c3f97 100755\n--- a/t/t5701-git-serve.sh\n+++ b/t/t5701-git-serve.sh\n@@ -30,6 +30,15 @@ test_expect_success 'test capability advertisement' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'test capability advertisement' '\n+\ttest_when_finished git config --unset serve.bundleuri &&\n+\tgit config serve.bundleuri \"file://$(pwd)\" &&\n+\tGIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \\\n+\t\t--advertise-capabilities >out &&\n+\ttest-tool pkt-line unpack actual &&\n+\tgrep features actual\n+'\n+\n test_expect_success 'stateless-rpc flag does not list capabilities' '\n \t# Empty request\n \ttest-tool pkt-line pack >in <<-EOF &&\n-- \ngitgitgadget\n\n"},{"id":"449324","messageId":"5b983cc3c104fe6ed64608320387bf82fefcdbb4.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 20/25] serve: understand but do not advertise 'features' capability","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:30:58Z","receivedAt":"2022-02-23T18:32:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe previous change implemented cap_features() to return a set of\n'key=value' pairs when this capability is run. Add the capability to our\nlist of understood capabilities.\n\nThis change does not advertise the capability. When deploying a new\ncapability to a distributed fleet of Git servers, it is important to\ndelay advertising the capability until all nodes understand it. A later\nchange will advertise it when appropriate, but as a separate change to\nsimplify this transition.\n\nSigned-off-by: Derrick Stolee \n---\n serve.c | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/serve.c b/serve.c\nindex b3fe9b5126a..a1c853dda1f 100644\n--- a/serve.c\n+++ b/serve.c\n@@ -18,6 +18,12 @@ static int always_advertise(struct repository *r,\n \treturn 1;\n }\n \n+static int never_advertise(struct repository *r,\n+\t\t\t struct strbuf *value)\n+{\n+\treturn 0;\n+}\n+\n static int agent_advertise(struct repository *r,\n \t\t\t struct strbuf *value)\n {\n@@ -136,6 +142,11 @@ static struct protocol_capability capabilities[] = {\n \t\t.advertise = always_advertise,\n \t\t.command = cap_object_info,\n \t},\n+\t{\n+\t\t.name = \"features\",\n+\t\t.advertise = never_advertise,\n+\t\t.command = cap_features,\n+\t},\n };\n \n void protocol_v2_advertise_capabilities(void)\n-- \ngitgitgadget\n\n"},{"id":"449325","messageId":"7a8b13c308c1e54e4f51429805fc309384a93120.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 22/25] connect: implement get_recommended_features()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:31:00Z","receivedAt":"2022-02-23T18:32:06Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThis method allows a client to request and parse the 'features' capability\nof protocol v2. The response is expected to be a list of 'key=value'\nlines, but this implementation does no checking of the lines, expecting\na later parse of the lines to be careful of the existence of that '='\ncharacter.\n\nThis change is based on an earlier patch [1] written for a similar\ncapability.\n\n[1] https://lore.kernel.org/git/RFC-patch-04.13-21caf01775-20210805T150534Z-avarab@gmail.com/\n\nCo-authored-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Derrick Stolee \n---\n connect.c | 36 ++++++++++++++++++++++++++++++++++++\n remote.h | 4 ++++\n 2 files changed, 40 insertions(+)\n\ndiff --git a/connect.c b/connect.c\nindex 9d78d681e95..e1e6f4770dd 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -491,6 +491,42 @@ static void send_capabilities(int fd_out, struct packet_reader *reader)\n \t}\n }\n \n+int get_recommended_features(int fd_out, struct packet_reader *reader,\n+\t\t\t struct string_list *list, int stateless_rpc)\n+{\n+\tint line_nr = 1;\n+\n+\tserver_supports_v2(\"features\", 1);\n+\n+\t/* (Re-)send capabilities */\n+\tsend_capabilities(fd_out, reader);\n+\n+\t/* Send command */\n+\tpacket_write_fmt(fd_out, \"command=features\\n\");\n+\tpacket_delim(fd_out);\n+\tpacket_flush(fd_out);\n+\n+\t/* Process response from server */\n+\twhile (packet_reader_read(reader) == PACKET_READ_NORMAL) {\n+\t\tconst char *line = reader->line;\n+\t\tline_nr++;\n+\n+\t\tstring_list_append(list, line);\n+\t}\n+\n+\tif (reader->status != PACKET_READ_FLUSH)\n+\t\treturn error(_(\"expected flush after features listing\"));\n+\n+\t/*\n+\t * Might die(), but obscure enough that that's OK, e.g. in\n+\t * serve.c, we'll call BUG() on its equivalent (the\n+\t * PACKET_READ_RESPONSE_END check).\n+\t */\n+\tcheck_stateless_delimiter(stateless_rpc, reader,\n+\t\t_(\"expected response end packet after features listing\"));\n+\treturn 0;\n+}\n+\n struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,\n \t\t\t struct ref **list, int for_push,\n \t\t\t struct transport_ls_refs_options *transport_options,\ndiff --git a/remote.h b/remote.h\nindex 438152ef562..268e8134f5e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -236,6 +236,10 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,\n \t\t\t const struct string_list *server_options,\n \t\t\t int stateless_rpc);\n \n+/* Used for protocol v2 in order to retrieve recommended features */\n+int get_recommended_features(int fd_out, struct packet_reader *reader,\n+\t\t\t struct string_list *list, int stateless_rpc);\n+\n int resolve_remote_symref(struct ref *ref, struct ref *list);\n \n /*\n-- \ngitgitgadget\n\n"},{"id":"449326","messageId":"f327ade8e461689d62088df8fb6fdda749154438.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 23/25] transport: add connections for 'features' capability","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:31:01Z","receivedAt":"2022-02-23T18:32:07Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nTo allow 'git clone' to check the 'features' capability, we need to fill\nin some boilerplate methods that help detect if the capability exists\nand then to execute the get_recommended_features() method with the\nproper context. This involves jumping through some vtables.\n\nSigned-off-by: Derrick Stolee \n---\n transport-helper.c | 14 ++++++++++++++\n transport-internal.h | 9 +++++++++\n transport.c | 38 ++++++++++++++++++++++++++++++++++++++\n transport.h | 5 +++++\n 4 files changed, 66 insertions(+)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex a0297b0986c..642472e2adb 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -1264,11 +1264,25 @@ static struct ref *get_refs_list_using_list(struct transport *transport,\n \treturn ret;\n }\n \n+static int get_features(struct transport *transport,\n+\t\t struct string_list *list)\n+{\n+\tget_helper(transport);\n+\n+\tif (process_connect(transport, 0)) {\n+\t\tdo_take_over(transport);\n+\t\treturn transport->vtable->get_features(transport, list);\n+\t}\n+\n+\treturn -1;\n+}\n+\n static struct transport_vtable vtable = {\n \t.set_option\t= set_helper_option,\n \t.get_refs_list\t= get_refs_list,\n \t.fetch_refs\t= fetch_refs,\n \t.push_refs\t= push_refs,\n+\t.get_features\t= get_features,\n \t.connect\t= connect_helper,\n \t.disconnect\t= release_helper\n };\ndiff --git a/transport-internal.h b/transport-internal.h\nindex c4ca0b733ac..759c79148db 100644\n--- a/transport-internal.h\n+++ b/transport-internal.h\n@@ -5,6 +5,7 @@ struct ref;\n struct transport;\n struct strvec;\n struct transport_ls_refs_options;\n+struct string_list;\n \n struct transport_vtable {\n \t/**\n@@ -51,6 +52,14 @@ struct transport_vtable {\n \t * process involved generating new commits.\n \t **/\n \tint (*push_refs)(struct transport *transport, struct ref *refs, int flags);\n+\n+\t/**\n+\t * get_features() requests a list of recommended features and\n+\t * populates the given string_list with those 'key=value' pairs.\n+\t */\n+\tint (*get_features)(struct transport *transport,\n+\t\t\t struct string_list *list);\n+\n \tint (*connect)(struct transport *connection, const char *name,\n \t\t const char *executable, int fd[2]);\n \ndiff --git a/transport.c b/transport.c\nindex 2a3e3241545..99d6b719f35 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -349,6 +349,20 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus\n \treturn handshake(transport, for_push, options, 1);\n }\n \n+static int get_features(struct transport *transport,\n+\t\t struct string_list *list)\n+{\n+\tstruct git_transport_data *data = transport->data;\n+\tstruct packet_reader reader;\n+\n+\tpacket_reader_init(&reader, data->fd[0], NULL, 0,\n+\t\t\t PACKET_READ_CHOMP_NEWLINE |\n+\t\t\t PACKET_READ_GENTLE_ON_EOF);\n+\n+\treturn get_recommended_features(data->fd[1], &reader, list,\n+\t\t\t\t\ttransport->stateless_rpc);\n+}\n+\n static int fetch_refs_via_pack(struct transport *transport,\n \t\t\t int nr_heads, struct ref **to_fetch)\n {\n@@ -890,6 +904,7 @@ static struct transport_vtable taken_over_vtable = {\n \t.get_refs_list\t= get_refs_via_connect,\n \t.fetch_refs\t= fetch_refs_via_pack,\n \t.push_refs\t= git_transport_push,\n+\t.get_features\t= get_features,\n \t.disconnect\t= disconnect_git\n };\n \n@@ -1043,6 +1058,7 @@ static struct transport_vtable builtin_smart_vtable = {\n \t.get_refs_list\t= get_refs_via_connect,\n \t.fetch_refs\t= fetch_refs_via_pack,\n \t.push_refs\t= git_transport_push,\n+\t.get_features\t= get_features,\n \t.connect\t= connect_git,\n \t.disconnect\t= disconnect_git\n };\n@@ -1456,6 +1472,28 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)\n \treturn rc;\n }\n \n+struct string_list *transport_remote_features(struct transport *transport)\n+{\n+\tconst struct transport_vtable *vtable = transport->vtable;\n+\tstruct string_list *list = NULL;\n+\n+\tif (!server_supports_v2(\"features\", 0))\n+\t\treturn NULL;\n+\n+\tif (!vtable->get_features) {\n+\t\twarning(_(\"'features' not supported by this remote\"));\n+\t\treturn NULL;\n+\t}\n+\n+\tCALLOC_ARRAY(list, 1);\n+\tstring_list_init_dup(list);\n+\n+\tif (vtable->get_features(transport, list))\n+\t\twarning(_(\"failed to get recommended features from remote\"));\n+\n+\treturn list;\n+}\n+\n void transport_unlock_pack(struct transport *transport, unsigned int flags)\n {\n \tint in_signal_handler = !!(flags & TRANSPORT_UNLOCK_PACK_IN_SIGNAL_HANDLER);\ndiff --git a/transport.h b/transport.h\nindex 3f16e50c196..bfa2dd48d85 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -272,6 +272,11 @@ struct transport_ls_refs_options {\n const struct ref *transport_get_remote_refs(struct transport *transport,\n \t\t\t\t\t struct transport_ls_refs_options *transport_options);\n \n+/**\n+ * Get recommended config from remote.\n+ */\n+struct string_list *transport_remote_features(struct transport *transport);\n+\n /*\n * Fetch the hash algorithm used by a remote.\n *\n-- \ngitgitgadget\n\n"},{"id":"449327","messageId":"883e17d0d21277e0ec8538da3e3b20f90b19128f.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 24/25] clone: use server-recommended bundle URI","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:31:02Z","receivedAt":"2022-02-23T18:32:11Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAfter the ref advertisement initializes the connection between the\nclient and the remote, use the 'features' capability (if available) to\nget a list of recommended features from the server.\n\nIn this change, we only update the bundle URI setting. The bundles are\ndownloaded immediately afterwards if the bundle URI becomes non-null.\n\nRFC-TODO: don't overwrite a given --bundle-uri option.\nRFC-TODO: implement the other capabilities.\nRFC-TODO: guard this entire request behind opt-in config.\nRFC-TODO: prevent using an HTTP(S) URI when in an SSH clone.\nRFC-TODO: prevent using a local path for the bundle URI.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/clone.c | 25 +++++++++++++++++++------\n 1 file changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex cfe3d96047a..92b8727fc9d 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -876,6 +876,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct remote *remote;\n \tint err = 0, complete_refs_before_fetch = 1;\n \tint submodule_progress;\n+\tstruct string_list *feature_list = NULL;\n \n \tstruct transport_ls_refs_options transport_ls_refs_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n@@ -1194,11 +1195,23 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \trefs = transport_get_remote_refs(transport, &transport_ls_refs_options);\n \n-\t/*\n-\t * NOTE: The bundle URI download takes place after transport_get_remote_refs()\n-\t * because a later change will introduce a check for recommended features,\n-\t * which might include a recommended bundle URI.\n-\t */\n+\tfeature_list = transport_remote_features(transport);\n+\n+\tif (feature_list) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, feature_list) {\n+\t\t\tchar *value;\n+\t\t\tchar *equals = strchr(item->string, '=');\n+\n+\t\t\tif (!equals)\n+\t\t\t\tcontinue;\n+\t\t\t*equals = '\\0';\n+\t\t\tvalue = equals + 1;\n+\n+\t\t\tif (!strcmp(item->string, \"bundleuri\"))\n+\t\t\t\tbundle_uri = value;\n+\t\t}\n+\t}\n \n \t/*\n \t * Before fetching from the remote, download and install bundle\n@@ -1218,7 +1231,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tif (filter)\n \t\t\tgit_config_set(\"fetch.bundlefilter\", filter);\n \n-\t\tif (!fetch_bundle_uri(bundle_uri, filter))\n+\t\tif (fetch_bundle_uri(bundle_uri, filter))\n \t\t\twarning(_(\"failed to fetch objects from bundle URI '%s'\"),\n \t\t\t\tbundle_uri);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"449328","messageId":"a2d64f369d0f00208574a8ff67e04c08e54c4317.1645641063.git.gitgitgadget@gmail.com","threadId":"57465","inReplyTo":"pull.1160.git.1645641063.gitgitgadget@gmail.com","subject":"[PATCH 25/25] t5601: basic bundle URI test","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-23T18:31:03Z","receivedAt":"2022-02-23T18:32:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThis test demonstrates an end-to-end form of the bundle URI feature\ngiven by an HTTP server advertising the 'features' capability with a\nbundle URI that is a bundle file on that same HTTP server. We verify\nthat we unbundled a bundle, which could only have happened if we\nsuccessfully downloaded that file.\n\nRFC-TODO: Create similar tests throughout the series that perform\nsimilar tests, including examples with table of contents and partial\nclones.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5601-clone.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 83c24fc97a7..b2409a4c04c 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -769,6 +769,18 @@ test_expect_success 'reject cloning shallow repository using HTTP' '\n \tgit clone --no-reject-shallow $HTTPD_URL/smart/repo.git repo\n '\n \n+test_expect_success 'auto-discover bundle URI from HTTP clone' '\n+\ttest_when_finished rm -rf repo \"$HTTPD_DOCUMENT_ROOT_PATH/repo2.git\" &&\n+\tgit -C src bundle create \"$HTTPD_DOCUMENT_ROOT_PATH/everything.bundle\" --all &&\n+\tgit clone --bare --no-local src \"$HTTPD_DOCUMENT_ROOT_PATH/repo2.git\" &&\n+\tgit -C \"$HTTPD_DOCUMENT_ROOT_PATH/repo2.git\" config \\\n+\t\tserve.bundleuri $HTTPD_URL/everything.bundle &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n+\t\tgit -c protocol.version=2 clone \\\n+\t\t$HTTPD_URL/smart/repo2.git repo &&\n+\ttest_subcommand_inexact git bundle unbundle There have been several suggestions to improve Git clone speeds and\n> reliability by supplementing the Git protocol with static content. The\n> Packfile URI [0] feature lets the Git response include URIs that point to\n> packfiles that the client must download to complete the request.\n>\n> Last year, Ævar suggested using bundles instead of packfiles [1] [2]. This\n> design has the same benefits to the packfile URI feature because it offloads\n> most object downloads to static content fetches. The main advantage over\n> packfile URIs is that the remote Git server does not need to know what is in\n> those bundles. The Git client tells the server what it downloaded during the\n> fetch negotiation afterwards. This includes any chance that the client did\n> not have access to those bundles or otherwise failed to access them. I\n> agreed that this was a much more desirable way to serve static content, but\n> had concerns about the flexibility of that design [3]. I have not heard more\n> on the topic since October, so I started investigating this idea myself in\n> December, resulting in this RFC.\n\nThis timing is both quite fortunate & unfortunate for me, since I'd been\nblocked / waiting on various things until very recently to submit a\nnon-RFC re-roll of (a larger version of) that series you mentioned from\nOctober.\n\nI guess the good news is that we'll have at least one guaranteed very\ninterested reviewer for each other's patches, and that the design that\nmakes it into git.git in the end will definitely be well hashed out :)\n\nI won't be able to review this in any detail right at this hour, but\nwill be doing so. I'd also like to submit what I've got in some form\nsoon for hashing the two out.\n\nIt will be some 50+ patches on the ML in total though related to this\ntopic, so I think the two of us coming up with some way to manage all of\nthat for both ourselves & others would be nice. Perhaps we could also\nhave an off-list (video) chat in real time to clarify/discuss various\nthing related to this.\n\nHaving said that, basically:\n\n> I focused on maximizing flexibility for the service that organizes and\n> serves bundles. This includes:\n>\n> * Bundle URIs work for full and partial clones.\n>\n> * Bundle URIs can assist with git fetch in addition to git clone.\n>\n> * Users can set up bundle servers independent of the remote Git server if\n> they specify the bundle URI via a --bundle-uri argument.\n>\n> This series is based on the recently-submitted series that adds object\n> filters to bundles [4]. There is a slight adjacent-line-add conflict with\n> js/apply-partial-clone-filters-recursively, but that is in the last few\n> patches, so it will be easy to rebase by the time we have a fully-reviewable\n> patch series for those steps.\n>\n> The general breakdown is as follows:\n>\n> * Patch 1 adds documentation for the feature in its entirety.\n>\n> * Patches 2-14 add the ability to run ‘git clone --bundle-uri=’\n>\n> * Patches 15-17 add bundle fetches to ‘git fetch’ calls\n>\n> * Patches 18-25 add a new ‘features’ capability that allows a server to\n> advertise bundle URIs (and in the future, other features).\n>\n> I consider the patches in their current form to be “RFC quality”. There are\n> multiple places where tests are missing or special cases are not checked.\n> The goal for this RFC is to seek feedback on the high-level ideas before\n> committing to the deep work of creating mergeable patches.\n\nHaving skimmed through all of this a *very rough* overview of what\nyou've got here & the direction I chose to go in is:\n\n1. I didn't go for an initial step of teaching \"git bundle\" any direct\n remote operation, rather it's straight to the protocol v2 bits etc.\n\n I don't think there's anything wrong with that, but didn't see much\n point in teaching \"git bundle\" to do that when the eventual state is\n to have \"git fetch\" do so anyway.\n\n But in either case the \"fetch\" parts are either a thin wrapper for\n \"git bundle fetch\", or a \"git bundle fetch/unbundle\" is a thin\n equivalent to \"init\" \"fetch\" (with bundle-uri) + \"unbundle\".\n\n2. By far the main difference is that you're heavily leaning on a TOC\n format which encodes certain assumptions that aren't true of\n clones/fetches in general (but probably are for most fetches), whereas\n my design (as we previously discussed) leans entirely on the client\n making sense of the bundle header & content itself.\n\n E.g. you have a \"bundle.tableOfContents.forFetch\", but e.g. if you've\n got a git.git clone of \"master\" and want to:\n\n git fetch origin refs/heads/todo:todo\n\n The assumption that we can cleanly separate \"clone\" from \"fetch\" breaks\n down.\n\n I.e. such a thing needs to assume that \"clone\" implies \"you have\n most of the objects you need already\" and that \"fetch\" means \"..an\n incremental update thereof\", doesn't it?\n\n Whereas I think (but we'll hash that out) that having a client fetch the\n bundle header and working that out via current reachability checks will\n be just as fast/faster, and such a thing is definitely more\n general/applicable to all sorts/types of fetches.\n\n (A TOC mechanism might still be good/valuable, but I hope it can be a\n cheap/discardable way to simply cache those bundle headers, or serve\n them up all at once)\n\n3. Ditto \"bundle..timestamp\" in the design (presumably assumes not-rewound\n histories), and \"requires\" (can also currently be inferred from bundle\n headers).\n\n4. I still need to go over your just-submitted \"bundle filters\"\n (https://lore.kernel.org/git/pull.1159.git.1645638911.gitgitgadget@gmail.com/)\n in detail but by adding a @filter to the file format (good!) presumably the\n \"bundle..filter\" amounts to a cache of the headers (which was 100% in line\n with any design I had for such extra information associated with a bundle).\n\nIn (partial) summary: I really want to lean more heavily into the\ndistributed nature of git in that a \"bundle clone\" be no more special\nthan the same operation performed locally where \"clone/fetch\" is\npointed-to a directory containing X number of local bundles, and has to\nmake sense of whether those help with the clone/fetch operation. I.e. by\nparsing their headers & comparing that to the ref advertisement.\n\nMaybe a meta-format TOC will be needed eventually, and I'm not against\nsuch a thing.\n\nI'd just like to make sure we wouldn't add such a thing as a premature\noptimization or something that would needlessly complicate the\ndesign. In particular (quoting from a part of 01/25:\n \n +A further optimization is that the client can avoid downloading any\n +bundles if their timestamps are not larger than the stored timestamp.\n +After fetching new bundles, this local timestamp value is updated.\n\nSuch caching seems sensible, but to me seems basically redundant to what\nyou'd get by doing the same with just:\n\n * A set of dumb bundle files in a directory on a webserver\n * Having unique names for each of those (e.g. naming them\n https:///.bundle instead of\n https:///weekly.bundle)\n * Since the content wouldn't change (HTTP headers indicating caching\n forever) a client would have downloaded say the last 6 of your set of\n 7 \"daily\" rotating bundles already, and we'd locally cache their\n entire header, not just a timestamp.\n\nI.e. I think you'd get the same reduction in requests and more from\nthat. I.e. (to go back to the earlier example) of:\n\n git fetch origin refs/heads/todo:todo\n\nYou'd get the tip of \"ls-refs\" for TODO, and locally discover that one\nof the 6 \"daily\" bundles whose headers (but not necessarily content) you\nalready downloaded had that advertised OID, and grab it from there.\n\nThe critical difference being that such an arrangement would not be\nassuming linear history/additive only (i.e. only fast-forward) which the\n\"forFetch\" + \"timestamp\" surely does.\n\nAnd, I think we'll be much better off both in the short and long term by\nheavily leaning into HTTP caching features and things like request\npipelining + range requests than a custom meta-index format.\n\nIOW is a TOC format needed if we assume for a moment for the sake of\nargument that for a given repository the say 100 bundles you'd\npotentially serve up aren't remote at all, but something you've got\nmmap()'d and can inspect the bundle headers for and compare with the\nremote \"ls-refs\"?\n\nBecause if that's the case we could basically get to the same place via\nHTTP caching features, and doing it that way has the advantage of\npiggy-backing on all existing caching infrastructure.\n\nHave 1000 computers on your network that keep fetching torvalds/linux?\nStick a proxy configured to cache the first say 1MB of \nin front of them.\n\nNow all their requests to discover if the bundles help will be local\n(and it would probably make sense to cache the actual content\ntoo). Whereas any type of custom caching strategy would be\nper-git-client.\n\nJust food for thought, and sorry that this E-Mail/braindump got so long\nalready...\n"},{"id":"449437","messageId":"15aed4cc-2d16-0b3f-5235-f7858a705c52@github.com","threadId":"57465","inReplyTo":"220224.86czjdb22l.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-02-24T14:11:41Z","receivedAt":"2022-02-24T14:11:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/23/2022 5:17 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:\n> \n> [Note: The E-Mail address you CC'd for me (presumably, dropped in this\n> reply) is not my E-Mail address, this one is]\n\nSorry about that. I appear to have gotten it right on the partial\nbundles series, but somehow had a brain fart here.\n \n> [Also CC-ing some people who have expressed interest in this are, and\n> would probably like to be kept in the loop going forward]\n\nThanks. The more eyes, the better.\n\n>> There have been several suggestions to improve Git clone speeds and\n>> reliability by supplementing the Git protocol with static content. The\n>> Packfile URI [0] feature lets the Git response include URIs that point to\n>> packfiles that the client must download to complete the request.\n>>\n>> Last year, Ævar suggested using bundles instead of packfiles [1] [2]. This\n>> design has the same benefits to the packfile URI feature because it offloads\n>> most object downloads to static content fetches. The main advantage over\n>> packfile URIs is that the remote Git server does not need to know what is in\n>> those bundles. The Git client tells the server what it downloaded during the\n>> fetch negotiation afterwards. This includes any chance that the client did\n>> not have access to those bundles or otherwise failed to access them. I\n>> agreed that this was a much more desirable way to serve static content, but\n>> had concerns about the flexibility of that design [3]. I have not heard more\n>> on the topic since October, so I started investigating this idea myself in\n>> December, resulting in this RFC.\n> \n> This timing is both quite fortunate & unfortunate for me, since I'd been\n> blocked / waiting on various things until very recently to submit a\n> non-RFC re-roll of (a larger version of) that series you mentioned from\n> October.\n> \n> I guess the good news is that we'll have at least one guaranteed very\n> interested reviewer for each other's patches, and that the design that\n> makes it into git.git in the end will definitely be well hashed out :)\n> \n> I won't be able to review this in any detail right at this hour, but\n> will be doing so. I'd also like to submit what I've got in some form\n> soon for hashing the two out.\n> \n> It will be some 50+ patches on the ML in total though related to this\n> topic, so I think the two of us coming up with some way to manage all of\n> that for both ourselves & others would be nice. Perhaps we could also\n> have an off-list (video) chat in real time to clarify/discuss various\n> thing related to this.\n\nI look forward to seeing your full implementation. There are many things\nabout your RFC that left me confused and not fully understanding your\nvision. You reference a few of them further down, so I'll mention them\nspecifically in that context.\n\n> Having said that, basically:\n> \n>> I focused on maximizing flexibility for the service that organizes and\n>> serves bundles. This includes:\n>>\n>> * Bundle URIs work for full and partial clones.\n>>\n>> * Bundle URIs can assist with git fetch in addition to git clone.\n>>\n>> * Users can set up bundle servers independent of the remote Git server if\n>> they specify the bundle URI via a --bundle-uri argument.\n>>\n>> This series is based on the recently-submitted series that adds object\n>> filters to bundles [4]. There is a slight adjacent-line-add conflict with\n>> js/apply-partial-clone-filters-recursively, but that is in the last few\n>> patches, so it will be easy to rebase by the time we have a fully-reviewable\n>> patch series for those steps.\n>>\n>> The general breakdown is as follows:\n>>\n>> * Patch 1 adds documentation for the feature in its entirety.\n>>\n>> * Patches 2-14 add the ability to run ‘git clone --bundle-uri=’\n>>\n>> * Patches 15-17 add bundle fetches to ‘git fetch’ calls\n>>\n>> * Patches 18-25 add a new ‘features’ capability that allows a server to\n>> advertise bundle URIs (and in the future, other features).\n>>\n>> I consider the patches in their current form to be “RFC quality”. There are\n>> multiple places where tests are missing or special cases are not checked.\n>> The goal for this RFC is to seek feedback on the high-level ideas before\n>> committing to the deep work of creating mergeable patches.\n> \n> Having skimmed through all of this a *very rough* overview of what\n> you've got here & the direction I chose to go in is:\n> \n> 1. I didn't go for an initial step of teaching \"git bundle\" any direct\n> remote operation, rather it's straight to the protocol v2 bits etc.\n> \n> I don't think there's anything wrong with that, but didn't see much\n> point in teaching \"git bundle\" to do that when the eventual state is\n> to have \"git fetch\" do so anyway.\n> \n> But in either case the \"fetch\" parts are either a thin wrapper for\n> \"git bundle fetch\", or a \"git bundle fetch/unbundle\" is a thin\n> equivalent to \"init\" \"fetch\" (with bundle-uri) + \"unbundle\".\n\nI'm not married to this specific implementation, although having the\nbundle fetch be something a user could run independently of 'git fetch'\nor 'git clone' might be desirable.\n\n> 2. By far the main difference is that you're heavily leaning on a TOC\n> format which encodes certain assumptions that aren't true of\n> clones/fetches in general (but probably are for most fetches), whereas\n> my design (as we previously discussed) leans entirely on the client\n> making sense of the bundle header & content itself.\n> \n> E.g. you have a \"bundle.tableOfContents.forFetch\", but e.g. if you've\n> got a git.git clone of \"master\" and want to:\n> \n> git fetch origin refs/heads/todo:todo\n> \n> The assumption that we can cleanly separate \"clone\" from \"fetch\" breaks\n> down.\n> \n> I.e. such a thing needs to assume that \"clone\" implies \"you have\n> most of the objects you need already\" and that \"fetch\" means \"..an\n> incremental update thereof\", doesn't it?\n> \n> Whereas I think (but we'll hash that out) that having a client fetch the\n> bundle header and working that out via current reachability checks will\n> be just as fast/faster, and such a thing is definitely more\n> general/applicable to all sorts/types of fetches.\n> \n> (A TOC mechanism might still be good/valuable, but I hope it can be a\n> cheap/discardable way to simply cache those bundle headers, or serve\n> them up all at once)\n\nNote that the TOC is completely optional, and you could serve a bundle from\nthe advertised URI. The biggest difference is that the TOC allows flexibility\nthat your design did not (or at least, I could not detect how to get that\nflexibility out of it).\n\nThe biggest thing that I am trying to understand from your design is something\nI'm going to make an educated guess about: if you _do_ break the repo into\nmultiple bundles, then the bundle URI advertisement sends multiple URIs that\nmust _all_ be inspected to get the full bundle content.\n\nThe change in the TOC model is that there can be multiple potential servers\nhosting multiple bundles, and the bundle URI advertisement sending multiple\nURIs means \"any of these would work. Try one close to you, or use the others\nas a fallback.\" This seems like the biggest incompatibility in our approaches\nbased on my understanding.\n\nFinally, the biggest thing that is possible in my model that is not in yours\nis the idea of an independent bundle server that is not known by the origin\nserver. It seems that everything relies on the bundle URI advertisement,\nwhile this implementation allows a --bundle-uri= at clone time or a\nconfigured URI.\n\nAgain, the TOC is critical here, so there can be one well-known URI for the\nTOC that doesn't change over time, and can be used to fetch multiple bundles.\n\nAnd perhaps you are intending the bundle URI to point to a directory listing,\nbut that seems like it would need a format that Git understands. And you\nmention parsing bundle headers, which I would really like to see how you\nintend to implement that without downloading too much data from the remotes.\nThe TOC can be extended to include whatever header information you intend to\nuse for these decisions.\n\n> 3. Ditto \"bundle..timestamp\" in the design (presumably assumes not-rewound\n> histories), and \"requires\" (can also currently be inferred from bundle\n> headers).\n> \n> 4. I still need to go over your just-submitted \"bundle filters\"\n> (https://lore.kernel.org/git/pull.1159.git.1645638911.gitgitgadget@gmail.com/)\n> in detail but by adding a @filter to the file format (good!) presumably the\n> \"bundle..filter\" amounts to a cache of the headers (which was 100% in line\n> with any design I had for such extra information associated with a bundle).\n> \n> In (partial) summary: I really want to lean more heavily into the\n> distributed nature of git in that a \"bundle clone\" be no more special\n> than the same operation performed locally where \"clone/fetch\" is\n> pointed-to a directory containing X number of local bundles, and has to\n> make sense of whether those help with the clone/fetch operation. I.e. by\n> parsing their headers & comparing that to the ref advertisement.\n\nCloning and fetching from bundles is fundamentally different from the\ndynamic fetch negotiation of the Git protocol, so I see this intent as\na drawback to your approach, not a strength.\n\n> Maybe a meta-format TOC will be needed eventually, and I'm not against\n> such a thing.\n> \n> I'd just like to make sure we wouldn't add such a thing as a premature\n> optimization or something that would needlessly complicate the\n> design. In particular (quoting from a part of 01/25:\n> \n> +A further optimization is that the client can avoid downloading any\n> +bundles if their timestamps are not larger than the stored timestamp.\n> +After fetching new bundles, this local timestamp value is updated.\n> \n> Such caching seems sensible, but to me seems basically redundant to what\n> you'd get by doing the same with just:\n> \n> * A set of dumb bundle files in a directory on a webserver\n> * Having unique names for each of those (e.g. naming them\n> https:///.bundle instead of\n> https:///weekly.bundle)\n\nThe bundles on my prototype server do have unique names (the timestamp\nis part of the name), even though they don't include a hash of the\ncontents. This is mostly for readability when a human looks at the\nTOC, since it does not affect the correctness. A hash could be used in\nthe name if the bundle server wanted, but it also isn't required.\n\n> * Since the content wouldn't change (HTTP headers indicating caching\n> forever) a client would have downloaded say the last 6 of your set of\n> 7 \"daily\" rotating bundles already, and we'd locally cache their\n> entire header, not just a timestamp.\n\nThis model either requires Git understanding how to walk that\ndirectory of files on the webserver OR for the origin server to be\naware of the hash of every bundle that might be fetched. That coupling\nis exactly the kind of thing I think is too difficult for serving with\npackfile URIs.\n\n> I.e. I think you'd get the same reduction in requests and more from\n> that. I.e. (to go back to the earlier example) of:\n> \n> git fetch origin refs/heads/todo:todo\n> \n> You'd get the tip of \"ls-refs\" for TODO, and locally discover that one\n> of the 6 \"daily\" bundles whose headers (but not necessarily content) you\n> already downloaded had that advertised OID, and grab it from there.\n\nIt would be easy to special-case custom refspecs as not wanting bundle\ndata. This goes back to the idea that serving from static content is\n_not_ dynamic enough to handle these cases and therefore should not be\na major concern for the design.\n\nEverything about using static content should be a _heuristic_ that\nworks for most users most-frequent operations. Most users have a\nstandard refs/heads/*:refs/remotes/origin/* refspec when fetching, so\nthey want as much data as they can get from that refspec from the\nbundles.\n\n> The critical difference being that such an arrangement would not be\n> assuming linear history/additive only (i.e. only fast-forward) which the\n> \"forFetch\" + \"timestamp\" surely does.\n\nGenerally, refs/heads/* does move forward. Yes, if someone force-pushes\nthen there is a chance that some extra data from their older version is\nstored in a bundle somewhere.\n\n> And, I think we'll be much better off both in the short and long term by\n> heavily leaning into HTTP caching features and things like request\n> pipelining + range requests than a custom meta-index format.\n\nThe point of TOC is not to split bundles small enough to avoid those\nfeatures. Those things are still possible, and I expect that anyone\norganizing a bundle server would want one very large bundle to handle\nthe majority of the repo data. Range requests are critical for allowing\nresumable clones for that large data.\n\n> IOW is a TOC format needed if we assume for a moment for the sake of\n> argument that for a given repository the say 100 bundles you'd\n> potentially serve up aren't remote at all, but something you've got\n> mmap()'d and can inspect the bundle headers for and compare with the\n> remote \"ls-refs\"?\n> \n> Because if that's the case we could basically get to the same place via\n> HTTP caching features, and doing it that way has the advantage of\n> piggy-backing on all existing caching infrastructure.\n1. 100 bundles is probably at least triple the maximum I would\n recommend, unless you have a have a super-active monorepo that\n wants bundles computed hourly.\n\n2. You keep saying \"inspecting the headers\" to make a decision, but\n I have yet to see you present a way that you would organize bundles\n such that that decision isn't just \"get all bundles I haven't\n already downloaded\" and then be equivalent to the timestamp\n heuristic.\n\n> Have 1000 computers on your network that keep fetching torvalds/linux?\n> Stick a proxy configured to cache the first say 1MB of \n> in front of them.\n> \n> Now all their requests to discover if the bundles help will be local\n> (and it would probably make sense to cache the actual content\n> too). Whereas any type of custom caching strategy would be\n> per-git-client.\n> \n> Just food for thought, and sorry that this E-Mail/braindump got so long\n> already...\n\nI recommend that when you have the time you look carefully at Patch 1\nwhich actually includes the design document for the feature. That doc\ncontains concrete descriptions of all the ways the presented design is\nflexible for users and server administrators.\n\nIn particular, I think an extremely valuable aspect of this design is\nthe ability for someone to spin up their own bundle server in their\nbuild lab and modify their build scripts to fetch from that bundle\nserver before going to the origin remote. If your design can handle\nthat scenario, then I'd love to see how.\n\nTake your time preparing your patches, and I'll give them a careful\nread then they are available. I expect that you have some great ideas\nthat could augment this design, and I'm sure that your code will have\nsome implementation details that are better than mine. We can find a\nway to combine to get the best of both worlds.\n\nThat said, I do feel that I must not have done an adequate job of\ndescribing how important I think it is to have a more flexible design\nthan what you presented before. You continue to push back that your\nideas are sufficient, but I disagree. Unfortunately, I cannot be\nsure either way until I see your full implementation.\n\nThanks,\n-Stolee\n"},{"id":"450049","messageId":"CABPp-BEXgmGW=Lk5-JE6bc1F8RbGidDVjALAZraeZ-2_u476gg@mail.gmail.com","threadId":"57465","inReplyTo":"0abec796b0089b84d23cb52bb127788fdd04961c.1645641063.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/25] docs: document bundle URI standard","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-03-02T02:28:55Z","receivedAt":"2022-03-02T02:29:13Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Feb 23, 2022 at 10:31 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> Introduce the idea of bundle URIs to the Git codebase through an\n> aspirational design document. This document includes the full design\n> intended to include the feature in its fully-implemented form. This will\n> take several steps as detailed in the Implementation Plan section.\n>\n> By committing this document now, it can be used to motivate changes\n> necessary to reach these final goals. The design can still be altered as\n> new information is discovered.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> Documentation/technical/bundle-uri.txt | 404 +++++++++++++++++++++++++\n> 1 file changed, 404 insertions(+)\n> create mode 100644 Documentation/technical/bundle-uri.txt\n>\n> diff --git a/Documentation/technical/bundle-uri.txt b/Documentation/technical/bundle-uri.txt\n> new file mode 100644\n> index 00000000000..5c0b9e8e3ef\n> --- /dev/null\n> +++ b/Documentation/technical/bundle-uri.txt\n> @@ -0,0 +1,404 @@\n> +Bundle URIs\n> +===========\n> +\n> +Bundle URIs are locations where Git can download one or more bundles in\n> +order to bootstrap the object database in advance of fetching the remaining\n> +objects from a remote.\n> +\n> +One goal is to speed up clones and fetches for users with poor network\n> +connectivity to the origin server. Another benefit is to allow heavy users,\n> +such as CI build farms, to use local resources for the majority of Git data\n> +and thereby reducing the load on the origin server.\n> +\n> +To enable the bundle URI feature, users can specify a bundle URI using\n> +command-line options or the origin server can advertise one or more URIs\n> +via a protocol v2 capability.\n> +\n> +Server requirements\n> +-------------------\n> +\n> +To provide a server-side implementation of bundle servers, no other parts\n> +of the Git protocol are required. This allows server maintainers to use\n> +static content solutions such as CDNs in order to serve the bundle files.\n> +\n> +At the current scope of the bundle URI feature, all URIs are expected to\n> +be HTTP(S) URLs where content is downloaded to a local file using a `GET`\n> +request to that URL. The server could include authentication requirements\n> +to those requests with the aim of triggering the configured credential\n> +helper for secure access.\n\nSo folks using ssh to clone, who have never configured a credential\nhelper before, might need to start doing so. This makes sense and I\ndon't think I see a way around it, but we might want to call it out a\nbit more prominently. Cloning over https seems to be rare in the\nvarious setups I've seen (I know there are others where it's common,\njust noting that many users may never have had to use https for\ncloning before), so this is a potentially big point for users to be\naware of in terms of setup.\n\n> +\n> +Assuming a `200 OK` response from the server, the content at the URL is\n> +expected to be of one of two forms:\n> +\n> +1. Bundle: A Git bundle file of version 2 or higher.\n> +\n> +2. Table of Contents: A plain-text file that is parsable using Git's\n> + config file parser. This file describes one or more bundles that are\n> + accessible from other URIs.\n> +\n> +Any other data provided by the server is considered erroneous.\n> +\n> +Table of Contents Format\n> +------------------------\n> +\n> +If the content at a bundle URI is not a bundle, then it is expected to be\n> +a plaintext file that is parseable using Git's config parser. This file\n> +can contain any list of key/value pairs, but only a fixed set will be\n> +considered by Git.\n> +\n> +bundle.tableOfContents.version::\n> + This value provides a version number for the table of contents. If\n> + a future Git change enables a feature that needs the Git client to\n> + react to a new key in the table of contents file, then this version\n> + will increment. The only current version number is 1, and if any\n> + other value is specified then Git will fail to use this file.\n\nWhat does \"Git will fail to use this file\" mean? Does it mean Git\nwill throw an error? clone/fetch without the aid of bundle uris but\nshow a warning? something else?\n\n> +\n> +bundle.tableOfContents.forFetch::\n> + This boolean value is a signal to the Git client that the bundle\n> + server has designed its bundle organization to assist `git fetch`\n> + commands in addition to `git clone` commands. If this is missing,\n> + Git should not use this table of contents for `git fetch` as it\n> + may lead to excess data downloads.\n> +\n> +The remaining keys include an `` segment which is a server-designated\n> +name for each available bundle.\n> +\n> +bundle..uri::\n> + This string value is the URI for downloading bundle ``. If\n> + the URI begins with a protocol (`http://` or `https://`) then the\n> + URI is absolute. Otherwise, the URI is interpreted as relative to\n> + the URI used for the table of contents. If the URI begins with `/`,\n> + then that relative path is relative to the domain name used for\n> + the table of contents. (This use of relative paths is intended to\n> + make it easier to distribute a set of bundles across a large\n> + number of servers or CDNs with different domain names.)\n> +\n> +bundle..timestamp::\n> + (Optional) This value is the number of seconds since Unix epoch\n> + (UTC) that this bundle was created. This is used as an approximation\n> + of a point in time that the bundle matches the data available at\n> + the origin server.\n\nAs an approximation, is there a risk of drift where the user has\ntimetamp A which is very close to B and makes decisions based upon it\nwhich results in their not getting dependent objects they need? Or is\nit just an optimization for them to only download certain bundles and\nlook at them, and then they'll iteratively go back and download more\n(as per the 'requires' field below) if they don't have enough objects\nto unbundle what they previously downloaded?\n\n> +\n> +bundle..requires::\n> + (Optional) This string value represents the ID of another bundle.\n> + When present, the server is indicating that this bundle contains a\n> + thin packfile. If the client does not have all necessary objects\n> + to unbundle this packfile, then the client can download the bundle\n> + with the `requires` ID and try again. (Note: it may be beneficial\n> + to allow the server to specify multiple `requires` bundles.)\n> +\n> +bundle..filter::\n> + (Optional) This string value represents an object filter that\n> + should also appear in the header of this bundle. The server uses\n> + this value to differentiate different kinds of bundles from which\n> + the client can choose those that match their object filters.\n> +\n> +Here is an example table of contents:\n> +\n> +```\n> +[bundle \"tableofcontents\"]\n> + version = 1\n> + forFetch = true\n> +\n> +[bundle \"2022-02-09-1644442601-daily\"]\n> + uri = https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-09-1644442601-daily.bundle\n> + timestamp = 1644442601\n> + requires = 2022-02-02-1643842562\n> +\n> +[bundle \"2022-02-02-1643842562\"]\n> + uri = https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-02-1643842562.bundle\n> + timestamp = 1643842562\n> +\n> +[bundle \"2022-02-09-1644442631-daily-blobless\"]\n> + uri = 2022-02-09-1644442631-daily-blobless.bundle\n> + timestamp = 1644442631\n> + requires = 2022-02-02-1643842568-blobless\n> + filter = blob:none\n> +\n> +[bundle \"2022-02-02-1643842568-blobless\"]\n> + uri = /git/git/2022-02-02-1643842568-blobless.bundle\n> + timestamp = 1643842568\n> + filter = blob:none\n> +```\n> +\n> +This example uses all of the keys in the specification. Suppose that the\n> +table of contents was found at the URI\n> +`https://gitbundleserver.z13.web.core.windows.net/git/git/` and so the\n> +two blobless bundles have the following fully-expanded URIs:\n> +\n> +* `https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-09-1644442631-daily-blobless.bundle`\n> +* `https://gitbundleserver.z13.web.core.windows.net/git/git/2022-02-02-1643842568-blobless.bundle`\n> +\n> +Advertising Bundle URIs\n> +-----------------------\n> +\n> +If a user knows a bundle URI for the repository they are cloning, then they\n> +can specify that URI manually through a command-line option. However, a\n> +Git host may want to advertise bundle URIs during the clone operation,\n> +helping users unaware of the feature.\n> +\n> +Note: The exact details of this section are not final. This is a possible\n> +way that Git could auto-discover bundle URIs, but is not a committed\n> +direction until that feature is implemented.\n> +\n> +The only thing required for this feature is that the server can advertise\n> +one or more bundle URIs. One way to implement this is to create a new\n> +protocol v2 capability that advertises recommended features, including\n> +bundle URIs.\n> +\n> +The client could choose an arbitrary bundle URI as an option _or_ select\n> +the URI with lowest latency by some exploratory checks. It is up to the\n> +server operator to decide if having multiple URIs is preferable to a\n> +single URI that is geodistributed through server-side infrastructure.\n> +\n> +Cloning with Bundle URIs\n> +------------------------\n> +\n> +The primary need for bundle URIs is to speed up clones. The Git client\n> +will interact with bundle URIs according to the following flow:\n> +\n> +1. The user specifies a bundle URI with the `--bundle-uri` command-line\n> + option _or_ the client discovers a bundle URI that was advertised by\n> + the remote server.\n> +\n> +2. The client downloads the file at the bundle URI. If it is a bundle, then\n> + it is unbundled with the refs being stored in `refs/bundle/*`.\n> +\n> +3. If the file is instead a table of contents, then the bundles with\n> + matching `filter` settings are sorted by `timestamp` (if present),\n> + and the most-recent bundle is downloaded.\n> +\n> +4. If the current bundle header mentions negative commid OIDs that are not\n> + in the object database, then download the `requires` bundle and try\n> + again.\n> +\n> +5. After inspecting a bundle with no negative commit OIDs (or all OIDs are\n> + already in the object database somehow), then unbundle all of the\n> + bundles in reverse order, placing references within `refs/bundle/*`.\n> +\n> +6. The client performs a fetch negotiation with the origin server, using\n> + the `refs/bundle/*` references as `have`s and the server's ref\n> + advertisement as `want`s. This results in a pack-file containing the\n> + remaining objects requested by the clone but not in the bundles.\n\nDoes step 6 potentially involve a new, second connection to the origin\nserver? I'm wondering about timeouts closing the original connection\nwhile the client is downloading the bundle uris. Will the client\nhandle that automatically, or will they potentially be forced to\nre-issue the clone/fetch command? I'm also wondering if we want to be\n\"nice\" and pre-emptively close the original connection to the server\nwhile we fetch the bundles -- for example, some servers have a\nthreadpool for processing fetch/clone requests and will only serve a\nlimited number; IIRC Gerrit operates this way. I have no idea if\nthat's a good idea or a horrible one. If a second connection is tried\nautomatically, will the user potentially be forced to re-enter\nconnection credentials again? And is there a risk that after the\nsecond connection, there are new bundle uris for the client to go\nfetch (and/or a removal of the original ones, e.g. replacing the old\n\"daily\" bundle with a new one)? Does this possibly cause us some\ntimestamp confusion as I noted earlier?\n\n> +Note that during a clone we expect that all bundles will be required. The\n> +client could be extended to download all bundles in parallel, though they\n> +need to be unbundled in the correct order.\n\nWhat does required mean? That the origin server can refuse to service\nrequests if the client does not have commits found in said bundles?\nThat new enough Git clients are expected to download all the bundles\n(and no config option will be provided to users to just do traditional\nnegotation without first downloading them)? Something else?\n\nIf users are doing a single-branch clone or a shallow clone, will the\nrequirements still hold? (I'm not a fan of shallow clones, but they\nare sadly used in a number of places and I'm curious how the features\ninteract or conflict.)\n\n> +\n> +If a table of contents is used and it contains\n> +`bundle.tableOfContents.forFetch = true`, then the client can store a\n> +config value indicating to reuse this URI for later `git fetch` commands.\n> +In this case, the client will also want to store the maximum timestamp of\n> +a downloaded bundle.\n> +\n> +Fetching with Bundle URIs\n> +-------------------------\n> +\n> +When the client fetches new data, it can decide to fetch from bundle\n> +servers before fetching from the origin remote. This could be done via\n> +a command-line option, but it is more likely useful to use a config value\n> +such as the one specified during the clone.\n> +\n> +The fetch operation follows the same procedure to download bundles from a\n> +table of contents (although we do _not_ want to use parallel downloads\n> +here). We expect that the process will end because all negative commit\n> +OIDs in a thin bundle are already in the object database.\n> +\n> +A further optimization is that the client can avoid downloading any\n> +bundles if their timestamps are not larger than the stored timestamp.\n> +After fetching new bundles, this local timestamp value is updated.\n\nWhat about the transition period where repositories were cloned before\nbundle URIs became a thing (or were turned on within an organization),\nand the user then goes to fetch? Will git go and download a bunch of\nuseless large bundles (and maybe one small useful one) the day this\nfeature is turned on, making users think this is a bad feature?\n\nShould git perhaps treat the already-cloned case without a stored\ntimestamp as a request to store a timestamp of \"now\", making it ignore\nthe current bundles? (If so, are there races where it later goes to\ngrab a bundle slightly newer than \"now\" but which depends on an older\nbundle that has some objects we are missing?)\n\n> +\n> +Choices for Bundle Server Organization\n> +--------------------------------------\n> +\n> +With this standard, there are many options available to the bundle server\n> +in how it organizes Git data into bundles.\n> +\n> +* Bundles can have whatever name the server desires. This name could refer\n> + to immutable data by using a hash of the bundle contents. However, this\n> + means that a new URI will be needed after every update of the content.\n> + This might be acceptable if the server is advertising the URI (and the\n> + server is aware of new bundles being generated) but would not be\n> + ergonomic for users using the command line option.\n> +\n> +* If the server intends to only serve full clones, then the advertised URI\n> + could be a bundle file without a filter that is updated at some cadence.\n> +\n> +* If the server intends to serve clones, but wants clients to choose full\n> + or blobless partial clones, then the server can use a table of contents\n> + that lists two non-thin bundles and the client chooses between them only\n> + by the `bundle..filter` values.\n> +\n> +* If the server intends to improve clones with parallel downloads, then it\n> + can use a table of contents and split the repository into time intervals\n> + of approximately similar-sized bundles. Using `bundle..timestamp`\n> + and `bundle..requires` values helps the client decide the order to\n> + unbundle the bundles.\n> +\n> +* If the server intends to serve fetches, then it can use a table of\n> + contents to advertise a list of bundles that are updated regularly. The\n> + most recent bundles could be generated on short intervals, such as hourly.\n> + These small bundles could be merged together at some rate, such as 24\n> + hourly bundles merging into a single daily bundle. At some point, it may\n> + be beneficial to create a bundle that stores the majority of the history,\n> + such as all data older than 30 days.\n> +\n> +These recommendations are intended only as suggestions. Each repository is\n> +different and every Git server has different needs. Hopefully the bundle\n> +URI feature and its table of contents is flexible enough to satisfy all\n> +needs. If not, then the format can be extended.\n> +\n> +Error Conditions\n> +----------------\n> +\n> +If the Git client discovers something unexpected while downloading\n> +information according to a bundle URI or the table of contents found at\n> +that location, then Git can ignore that data and continue as if it was not\n> +given a bundle URI. The remote Git server is the ultimate source of truth,\n> +not the bundle URI.\n\nThis seems to contradict the earlier statement that for clones all\nbundle URIs would be \"required\". I like the idea of bundle URIs only\nbeing an optimization that can be ignored, just noting the potential\nconfusion.\n\n> +\n> +Here are a few example error conditions:\n> +\n> +* The client fails to connect with a server at the given URI or a connection\n> + is lost without any chance to recover.\n> +\n> +* The client receives a response other than `200 OK` (such as `404 Not Found`,\n> + `401 Not Authorized`, or `500 Internal Server Error`).\n> +\n> +* The client receives data that is not parsable as a bundle or table of\n> + contents.\n> +\n> +* The table of contents describes a directed cycle in the\n> + `bundle..requires` links.\n> +\n> +* A bundle includes a filter that does not match expectations.\n> +\n> +* The client cannot unbundle the bundles because the negative commit OIDs\n> + are not in the object database and there are no more\n> + `bundle..requires` links to follow.\n\nShould these result in warnings so that folks can diagnose slower\nconnections, or should they be squelched? (I'm thinking particularly\nof the `401 Not Authorized` case in combination with users never\nhaving had to use a credential helper before.)\n\n> +\n> +There are also situations that could be seen as wasteful, but are not\n> +error conditions:\n> +\n> +* The downloaded bundles contain more information than is requested by\n> + the clone or fetch request. A primary example is if the user requests\n> + a clone with `--single-branch` but downloads bundles that store every\n> + reachable commit from all `refs/heads/*` references. This might be\n> + initially wasteful, but perhaps these objects will become reachable by\n> + a later ref update that the client cares about.\n\nAh, this answers my --single-branch question. Still curious about the\n--shallow misfeature (yeah, I'm a bit opinionated) and how it\ninteracts, though.\n\n> +* A bundle download during a `git fetch` contains objects already in the\n> + object database. This is probably unavoidable if we are using bundles\n> + for fetches, since the client will almost always be slightly ahead of\n> + the bundle servers after performing its \"catch-up\" fetch to the remote\n> + server. This extra work is most wasteful when the client is fetching\n> + much more frequently than the server is computing bundles, such as if\n> + the client is using hourly prefetches with background maintenance, but\n> + the server is computing bundles weekly. For this reason, the client\n> + should not use bundle URIs for fetch unless the server has explicitly\n> + recommended it through the `bundle.tableOfContents.forFetch = true`\n> + value.\n\nMakes sense, and certainly reduces my worry about the \"transition\nperiod\" where users have existing clones that pre-dated the\nintroduction of the bundle URI feature. But I'm still kind of curious\nabout how we handle that transition for folks that have recommended\ntheir bundleUris for fetches.\n\n\n> +\n> +Implementation Plan\n> +-------------------\n> +\n> +This design document is being submitted on its own as an aspirational\n> +document, with the goal of implementing all of the mentioned client\n> +features over the course of several patch series. Here is a potential\n> +outline for submitting these features for full review:\n> +\n> +1. Update the `git bundle create` command to take a `--filter` option,\n> + allowing bundles to store packfiles restricted to an object filter.\n> + This is necessary for using bundle URIs to benefit partial clones.\n> +\n> +2. Integrate bundle URIs into `git clone` with a `--bundle-uri` option.\n> + This will include the full understanding of a table of contents, but\n> + will not integrate with `git fetch` or allow the server to advertise\n> + URIs.\n> +\n> +3. Integrate bundle URIs into `git fetch`, triggered by config values that\n> + are set during `git clone` if the server indicates that the bundle\n> + strategy works for fetches.\n> +\n> +4. Create a new \"recommended features\" capability in protocol v2 where the\n> + server can recommend features such as bundle URIs, partial clone, and\n> + sparse-checkout. These features will be extremely limited in scope and\n> + blocked by opt-in config options. The design for this portion could be\n> + replaced by a \"bundle-uri\" capability that only advertises bundle URIs\n> + and no other information.\n> +\n> +Related Work: Packfile URIs\n> +---------------------------\n> +\n> +The Git protocol already has a capability where the Git server can list\n> +a set of URLs along with the packfile response when serving a client\n> +request. The client is then expected to download the packfiles at those\n> +locations in order to have a complete understanding of the response.\n> +\n> +This mechanism is used by the Gerrit server (implemented with JGit) and\n> +has been effective at reducing CPU load and improving user performance for\n> +clones.\n> +\n> +A major downside to this mechanism is that the origin server needs to know\n> +_exactly_ what is in those packfiles, and the packfiles need to be available\n> +to the user for some time after the server has responded. This coupling\n> +between the origin and the packfile data is difficult to manage.\n> +\n> +Further, this implementation is extremely hard to make work with fetches.\n> +\n> +Related Work: GVFS Cache Servers\n> +--------------------------------\n> +\n> +The GVFS Protocol [2] is a set of HTTP endpoints designed independently of\n> +the Git project before Git's partial clone was created. One feature of this\n> +protocol is the idea of a \"cache server\" which can be colocated with build\n> +machines or developer offices to transfer Git data without overloading the\n> +central server.\n> +\n> +The endpoint that VFS for Git is famous for is the `GET /gvfs/objects/{oid}`\n> +endpoint, which allows downloading an object on-demand. This is a critical\n> +piece of the filesystem virtualization of that product.\n> +\n> +However, a more subtle need is the `GET /gvfs/prefetch?lastPackTimestamp=`\n> +endpoint. Given an optional timestamp, the cache server responds with a list\n> +of precomputed packfiles containing the commits and trees that were introduced\n> +in those time intervals.\n> +\n> +The cache server computes these \"prefetch\" packfiles using the following\n> +strategy:\n> +\n> +1. Every hour, an \"hourly\" pack is generated with a given timestamp.\n> +2. Nightly, the previous 24 hourly packs are rolled up into a \"daily\" pack.\n> +3. Nightly, all prefetch packs more than 30 days old are rolled up into\n> + one pack.\n> +\n> +When a user runs `gvfs clone` or `scalar clone` against a repo with cache\n> +servers, the client requests all prefetch packfiles, which is at most\n> +`24 + 30 + 1` packfiles downloading only commits and trees. The client\n> +then follows with a request to the origin server for the references, and\n> +attempts to checkout that tip reference. (There is an extra endpoint that\n> +helps get all reachable trees from a given commit, in case that commit\n> +was not already in a prefetch packfile.)\n> +\n> +During a `git fetch`, a hook requests the prefetch endpoint using the\n> +most-recent timestamp from a previously-downloaded prefetch packfile.\n> +Only the list of packfiles with later timestamps are downloaded. Most\n> +users fetch hourly, so they get at most one hourly prefetch pack. Users\n> +whose machines have been off or otherwise have not fetched in over 30 days\n> +might redownload all prefetch packfiles. This is rare.\n> +\n> +It is important to note that the clients always contact the origin server\n> +for the refs advertisement, so the refs are frequently \"ahead\" of the\n> +prefetched pack data. The missing objects are downloaded on-demand using\n> +the `GET gvfs/objects/{oid}` requests, when needed by a command such as\n> +`git checkout` or `git log`. Some Git optimizations disable checks that\n> +would cause these on-demand downloads to be too aggressive.\n> +\n> +See Also\n> +--------\n> +\n> +[1] https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n> + An earlier RFC for a bundle URI feature.\n> +\n> +[2] https://github.com/microsoft/VFSForGit/blob/master/Protocol.md\n> + The GVFS Protocol\n> \\ No newline at end of file\n> --\n> gitgitgadget\n>\n"},{"id":"450103","messageId":"4f5f4751-c047-b9de-28a7-6ee3c31826f0@github.com","threadId":"57465","inReplyTo":"CABPp-BEXgmGW=Lk5-JE6bc1F8RbGidDVjALAZraeZ-2_u476gg@mail.gmail.com","subject":"Re: [PATCH 01/25] docs: document bundle URI standard","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-02T14:06:53Z","receivedAt":"2022-03-02T14:07:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/1/2022 9:28 PM, Elijah Newren wrote:\n> On Wed, Feb 23, 2022 at 10:31 AM Derrick Stolee via GitGitGadget\n> wrote:\n>>\n>> From: Derrick Stolee \n>>\n>> Introduce the idea of bundle URIs to the Git codebase through an\n>> aspirational design document. This document includes the full design\n>> intended to include the feature in its fully-implemented form. This will\n>> take several steps as detailed in the Implementation Plan section.\n>>\n>> By committing this document now, it can be used to motivate changes\n>> necessary to reach these final goals. The design can still be altered as\n>> new information is discovered.\n>>\n>> Signed-off-by: Derrick Stolee \n>> ---\n>> Documentation/technical/bundle-uri.txt | 404 +++++++++++++++++++++++++\n>> 1 file changed, 404 insertions(+)\n>> create mode 100644 Documentation/technical/bundle-uri.txt\n>>\n>> diff --git a/Documentation/technical/bundle-uri.txt b/Documentation/technical/bundle-uri.txt\n>> new file mode 100644\n>> index 00000000000..5c0b9e8e3ef\n>> --- /dev/null\n>> +++ b/Documentation/technical/bundle-uri.txt\n>> @@ -0,0 +1,404 @@\n>> +Bundle URIs\n>> +===========\n>> +\n>> +Bundle URIs are locations where Git can download one or more bundles in\n>> +order to bootstrap the object database in advance of fetching the remaining\n>> +objects from a remote.\n>> +\n>> +One goal is to speed up clones and fetches for users with poor network\n>> +connectivity to the origin server. Another benefit is to allow heavy users,\n>> +such as CI build farms, to use local resources for the majority of Git data\n>> +and thereby reducing the load on the origin server.\n>> +\n>> +To enable the bundle URI feature, users can specify a bundle URI using\n>> +command-line options or the origin server can advertise one or more URIs\n>> +via a protocol v2 capability.\n>> +\n>> +Server requirements\n>> +-------------------\n>> +\n>> +To provide a server-side implementation of bundle servers, no other parts\n>> +of the Git protocol are required. This allows server maintainers to use\n>> +static content solutions such as CDNs in order to serve the bundle files.\n>> +\n>> +At the current scope of the bundle URI feature, all URIs are expected to\n>> +be HTTP(S) URLs where content is downloaded to a local file using a `GET`\n>> +request to that URL. The server could include authentication requirements\n>> +to those requests with the aim of triggering the configured credential\n>> +helper for secure access.\n> \n> So folks using ssh to clone, who have never configured a credential\n> helper before, might need to start doing so. This makes sense and I\n> don't think I see a way around it, but we might want to call it out a\n> bit more prominently. Cloning over https seems to be rare in the\n> various setups I've seen (I know there are others where it's common,\n> just noting that many users may never have had to use https for\n> cloning before), so this is a potentially big point for users to be\n> aware of in terms of setup.\n\nWe could even go so far as to skip the credential manager if the\nGit remote is SSH, requiring the bundle URIs to work only if\nunauthenticated. Likely, we will want clear knobs that the user\ncan toggle for how to behave when a bundle URI is advertised with\nmodes such as\n\n* always attempt with authentication\n* always attempt, but skip authentication when Git remote is not HTTP(S)\n* attempt only when Git remote is HTTP(S)\n* never attempt\n\nThese are all things that are separate from the bundle URI standard\nbeing documented here, but instead would be saved for the last set of\npatches that allow a server to advertise a bundle URI at clone time.\n\n>> +bundle.tableOfContents.version::\n>> + This value provides a version number for the table of contents. If\n>> + a future Git change enables a feature that needs the Git client to\n>> + react to a new key in the table of contents file, then this version\n>> + will increment. The only current version number is 1, and if any\n>> + other value is specified then Git will fail to use this file.\n> \n> What does \"Git will fail to use this file\" mean? Does it mean Git\n> will throw an error? clone/fetch without the aid of bundle uris but\n> show a warning? something else?\n\nI mean \"Git will continue as if the bundle URI was not specified\". It would\nshow a warning, probably. This could be converted into a failure if valuable\nfor the user, but I don't expect that will be the default behavior.\n\n>> +bundle..timestamp::\n>> + (Optional) This value is the number of seconds since Unix epoch\n>> + (UTC) that this bundle was created. This is used as an approximation\n>> + of a point in time that the bundle matches the data available at\n>> + the origin server.\n> \n> As an approximation, is there a risk of drift where the user has\n> timetamp A which is very close to B and makes decisions based upon it\n> which results in their not getting dependent objects they need? Or is\n> it just an optimization for them to only download certain bundles and\n> look at them, and then they'll iteratively go back and download more\n> (as per the 'requires' field below) if they don't have enough objects\n> to unbundle what they previously downloaded?\n\nThe user doesn't ever generate the timestamp. It saves the timestamp\nfrom the most-recent bundle it downloaded. The only risk of timestamp\ndrift is if the server has multiple machines generating different sets\nof bundles, and places those machines behind a load balancer.\n\nThis is something the server can control, likely by having one job\ngenerate the bundle set and then distributing them to various storage\nlocations.\n\n>> +Cloning with Bundle URIs\n>> +------------------------\n>> +\n>> +The primary need for bundle URIs is to speed up clones. The Git client\n>> +will interact with bundle URIs according to the following flow:\n>> +\n>> +1. The user specifies a bundle URI with the `--bundle-uri` command-line\n>> + option _or_ the client discovers a bundle URI that was advertised by\n>> + the remote server.\n>> +\n>> +2. The client downloads the file at the bundle URI. If it is a bundle, then\n>> + it is unbundled with the refs being stored in `refs/bundle/*`.\n>> +\n>> +3. If the file is instead a table of contents, then the bundles with\n>> + matching `filter` settings are sorted by `timestamp` (if present),\n>> + and the most-recent bundle is downloaded.\n>> +\n>> +4. If the current bundle header mentions negative commid OIDs that are not\n>> + in the object database, then download the `requires` bundle and try\n>> + again.\n>> +\n>> +5. After inspecting a bundle with no negative commit OIDs (or all OIDs are\n>> + already in the object database somehow), then unbundle all of the\n>> + bundles in reverse order, placing references within `refs/bundle/*`.\n>> +\n>> +6. The client performs a fetch negotiation with the origin server, using\n>> + the `refs/bundle/*` references as `have`s and the server's ref\n>> + advertisement as `want`s. This results in a pack-file containing the\n>> + remaining objects requested by the clone but not in the bundles.\n> \n> Does step 6 potentially involve a new, second connection to the origin\n> server? I'm wondering about timeouts closing the original connection\n> while the client is downloading the bundle uris. Will the client\n> handle that automatically, or will they potentially be forced to\n> re-issue the clone/fetch command? I'm also wondering if we want to be\n> \"nice\" and pre-emptively close the original connection to the server\n> while we fetch the bundles -- for example, some servers have a\n> threadpool for processing fetch/clone requests and will only serve a\n> limited number; IIRC Gerrit operates this way. I have no idea if\n> that's a good idea or a horrible one. If a second connection is tried\n> automatically, will the user potentially be forced to re-enter\n> connection credentials again? And is there a risk that after the\n> second connection, there are new bundle uris for the client to go\n> fetch (and/or a removal of the original ones, e.g. replacing the old\n> \"daily\" bundle with a new one)? Does this possibly cause us some\n> timestamp confusion as I noted earlier?\n\nIf the user is cloning over HTTPS, then the connections are stateless\nand this is not any different than how it works today.\n\nWhen using SSH, we will probably want to close the SSH connection on\nthe client and then reopen it to avoid keeping that connection open\nduring the download. The implementation in this RFC does _not_ do this,\nbut I think it would be valuable to do.\n\n>> +Note that during a clone we expect that all bundles will be required. The\n>> +client could be extended to download all bundles in parallel, though they\n>> +need to be unbundled in the correct order.\n> \n> What does required mean? That the origin server can refuse to service\n> requests if the client does not have commits found in said bundles?\n> That new enough Git clients are expected to download all the bundles\n> (and no config option will be provided to users to just do traditional\n> negotation without first downloading them)? Something else?\n\nThe assumption I'm making here is that all but on bundle in the table\nof contents contains a thin pack, depending on an \"earlier\" bundle.\nThe client would be unsuccessful unbundling any single bundle except\nthe earliest one first.\n\nThe benefit of this assumption is that we could also implement parallel\ndownloads of all bundles in the future.\n\nThis assumes that there is no way to organize the bundles to communicate\nthat a user might want only the objects reachable from the default branch,\nbut also some users want every reachable object. Such an organization\nwould require extra information to describe two \"parallel\" lists of\nbundles that could be selected for each of those categories. If such an\norganization is valuable, then the table of contents can be extended with\ninformation to communicate such an organization. The downside is that\nclients with this \"v1\" version would download extra data based on this\nassumption.\n\n> If users are doing a single-branch clone or a shallow clone, will the\n> requirements still hold? (I'm not a fan of shallow clones, but they\n> are sadly used in a number of places and I'm curious how the features\n> interact or conflict.)\n\nThe current specification does not focus on shallow clones. The TOC\ncould be extended to say \"this bundle is for a shallow clone of \ncommit \" if that was valuable.\n\nFor single-branch clones, my expectation is that the bundles will\ngive the user more information than they need for that clone. The\nnegotiation will find out what they need from that branch that was\nnot in the bundles, but the bundles will also contain a lot of\nobjects that are not reachable from that ref. (This is also up to\nthe discretion of the bundle server operator, since they could\nfocus on only objects reachable from a subset of refs, minimizing\nthe bundle data while increasing the potential size of that\nfollow-up fetch.)\n\n>> +If a table of contents is used and it contains\n>> +`bundle.tableOfContents.forFetch = true`, then the client can store a\n>> +config value indicating to reuse this URI for later `git fetch` commands.\n>> +In this case, the client will also want to store the maximum timestamp of\n>> +a downloaded bundle.\n>> +\n>> +Fetching with Bundle URIs\n>> +-------------------------\n>> +\n>> +When the client fetches new data, it can decide to fetch from bundle\n>> +servers before fetching from the origin remote. This could be done via\n>> +a command-line option, but it is more likely useful to use a config value\n>> +such as the one specified during the clone.\n>> +\n>> +The fetch operation follows the same procedure to download bundles from a\n>> +table of contents (although we do _not_ want to use parallel downloads\n>> +here). We expect that the process will end because all negative commit\n>> +OIDs in a thin bundle are already in the object database.\n>> +\n>> +A further optimization is that the client can avoid downloading any\n>> +bundles if their timestamps are not larger than the stored timestamp.\n>> +After fetching new bundles, this local timestamp value is updated.\n> \n> What about the transition period where repositories were cloned before\n> bundle URIs became a thing (or were turned on within an organization),\n> and the user then goes to fetch? Will git go and download a bunch of\n> useless large bundles (and maybe one small useful one) the day this\n> feature is turned on, making users think this is a bad feature?\n> \n> Should git perhaps treat the already-cloned case without a stored\n> timestamp as a request to store a timestamp of \"now\", making it ignore\n> the current bundles? (If so, are there races where it later goes to\n> grab a bundle slightly newer than \"now\" but which depends on an older\n> bundle that has some objects we are missing?)\n\nI expect that users who already cloned will never configure their\nrepositories to include a bundle server.\n\nThat said, if you run 'git bundle fetch ' in an existing\nrepository, then it will fetch only the newest bundle and see if you\nalready have all of its negative refs. If so, then it stops and that\nis the only bundle that is downloaded. Its timestamp is stored for\nthe next 'git bundle fetch'.\n\nIn the case where the server starts advertising a bundle URI, a\n'git fetch' will not start using that URI. That check only happens\nduring 'git clone' (as currently designed).\n\n>> +Error Conditions\n>> +----------------\n>> +\n>> +If the Git client discovers something unexpected while downloading\n>> +information according to a bundle URI or the table of contents found at\n>> +that location, then Git can ignore that data and continue as if it was not\n>> +given a bundle URI. The remote Git server is the ultimate source of truth,\n>> +not the bundle URI.\n> \n> This seems to contradict the earlier statement that for clones all\n> bundle URIs would be \"required\". I like the idea of bundle URIs only\n> being an optimization that can be ignored, just noting the potential\n> confusion.\n\nPerhaps I misnamed this section. These are things that could go wrong with\na bundle server connection, and in such a case Git should recover by\ntransitioning to the normal Git protocol to fetch the objects.\n\n>> +\n>> +Here are a few example error conditions:\n>> +\n>> +* The client fails to connect with a server at the given URI or a connection\n>> + is lost without any chance to recover.\n>> +\n>> +* The client receives a response other than `200 OK` (such as `404 Not Found`,\n>> + `401 Not Authorized`, or `500 Internal Server Error`).\n>> +\n>> +* The client receives data that is not parsable as a bundle or table of\n>> + contents.\n>> +\n>> +* The table of contents describes a directed cycle in the\n>> + `bundle..requires` links.\n>> +\n>> +* A bundle includes a filter that does not match expectations.\n>> +\n>> +* The client cannot unbundle the bundles because the negative commit OIDs\n>> + are not in the object database and there are no more\n>> + `bundle..requires` links to follow.\n> \n> Should these result in warnings so that folks can diagnose slower\n> connections, or should they be squelched? (I'm thinking particularly\n> of the `401 Not Authorized` case in combination with users never\n> having had to use a credential helper before.)\n\nThere is a lot of work to be done around polishing the user ergonomics\nhere, and that is an interesting thing to consider for a second round\nafter the basic standard is established. I appreciate that you are\nalready thinking about the user experience in these corner cases.\n\n>> +\n>> +There are also situations that could be seen as wasteful, but are not\n>> +error conditions:\n>> +\n>> +* The downloaded bundles contain more information than is requested by\n>> + the clone or fetch request. A primary example is if the user requests\n>> + a clone with `--single-branch` but downloads bundles that store every\n>> + reachable commit from all `refs/heads/*` references. This might be\n>> + initially wasteful, but perhaps these objects will become reachable by\n>> + a later ref update that the client cares about.\n> \n> Ah, this answers my --single-branch question. Still curious about the\n> --shallow misfeature (yeah, I'm a bit opinionated) and how it\n> interacts, though.\n\n(Hopefully my previous reply to this topic is helpful.)\n\n>> +* A bundle download during a `git fetch` contains objects already in the\n>> + object database. This is probably unavoidable if we are using bundles\n>> + for fetches, since the client will almost always be slightly ahead of\n>> + the bundle servers after performing its \"catch-up\" fetch to the remote\n>> + server. This extra work is most wasteful when the client is fetching\n>> + much more frequently than the server is computing bundles, such as if\n>> + the client is using hourly prefetches with background maintenance, but\n>> + the server is computing bundles weekly. For this reason, the client\n>> + should not use bundle URIs for fetch unless the server has explicitly\n>> + recommended it through the `bundle.tableOfContents.forFetch = true`\n>> + value.\n> \n> Makes sense, and certainly reduces my worry about the \"transition\n> period\" where users have existing clones that pre-dated the\n> introduction of the bundle URI feature. But I'm still kind of curious\n> about how we handle that transition for folks that have recommended\n> their bundleUris for fetches.\n\nBy \"that transition\" I believe you are talking about configuring bundle\nURIs on an existing repository. My earlier reply on that hopefully\neases your concerns somewhat. Ævar also has some ideas about downloading\nonly the header of a bundle and examining it to see if we need the rest\nof it, which would further reduce the amount of data necessary to do an\ninitial first fetch from a bundle URI.\n\nThanks for taking the time to read and give detailed thoughts on this\nproposal!\n\nThanks,\n-Stolee\n\n"},{"id":"450240","messageId":"CABPp-BFA4fPhrOpfC8ROQAqzofn+7LTiYXmz+AqOFEx5H_sYGA@mail.gmail.com","threadId":"57465","inReplyTo":"4f5f4751-c047-b9de-28a7-6ee3c31826f0@github.com","subject":"Re: [PATCH 01/25] docs: document bundle URI standard","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-03-03T02:19:22Z","receivedAt":"2022-03-03T02:19:40Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Mar 2, 2022 at 6:06 AM Derrick Stolee wrote:\n>\n[...]\n> >> +bundle..timestamp::\n> >> + (Optional) This value is the number of seconds since Unix epoch\n> >> + (UTC) that this bundle was created. This is used as an approximation\n> >> + of a point in time that the bundle matches the data available at\n> >> + the origin server.\n> >\n> > As an approximation, is there a risk of drift where the user has\n> > timetamp A which is very close to B and makes decisions based upon it\n> > which results in their not getting dependent objects they need? Or is\n> > it just an optimization for them to only download certain bundles and\n> > look at them, and then they'll iteratively go back and download more\n> > (as per the 'requires' field below) if they don't have enough objects\n> > to unbundle what they previously downloaded?\n>\n> The user doesn't ever generate the timestamp. It saves the timestamp\n> from the most-recent bundle it downloaded. The only risk of timestamp\n> drift is if the server has multiple machines generating different sets\n> of bundles, and places those machines behind a load balancer.\n>\n> This is something the server can control, likely by having one job\n> generate the bundle set and then distributing them to various storage\n> locations.\n\nMy first thought here was \"can, or must?\"...and if it's not \"must\",\nare there ramifications we have to consider in the design of how the\nGit client behaves? However, reading through the rest of the document\nand your response, I don't think there is no problem here because\nbelow you mention that when fetching, the client can grab the latest\nbundle, and if it lacks some of the necessary pre-requisite objects,\nit can just iteratively download previous bundles until they have\nenough.\n\nAlso, the main thing that prompted my question was my wondering about\nhow to get pre-existing clones to start taking advantage of bundle\nURIs for future fetches, and this timestamp approximation seemed\nproblematic. But in addition to iteratively grabbing more bundles,\nyou also suggested that such users would just be forced to reclone,\nwhich would also avoid timestamp approximation issues.\n\nAnd, just now, I noticed the little \"Optional\" you put in the\ndescription that I somehow missed previously. I just can't read.\nAnyway, it sounds like the timestamp isn't used for correctness for\nthe client to know what to grab, it's more just an aid for grabbing a\nuseful initial set.\n\n> >> +Cloning with Bundle URIs\n> >> +------------------------\n> >> +\n> >> +The primary need for bundle URIs is to speed up clones. The Git client\n> >> +will interact with bundle URIs according to the following flow:\n> >> +\n> >> +1. The user specifies a bundle URI with the `--bundle-uri` command-line\n> >> + option _or_ the client discovers a bundle URI that was advertised by\n> >> + the remote server.\n> >> +\n> >> +2. The client downloads the file at the bundle URI. If it is a bundle, then\n> >> + it is unbundled with the refs being stored in `refs/bundle/*`.\n> >> +\n> >> +3. If the file is instead a table of contents, then the bundles with\n> >> + matching `filter` settings are sorted by `timestamp` (if present),\n> >> + and the most-recent bundle is downloaded.\n> >> +\n> >> +4. If the current bundle header mentions negative commid OIDs that are not\n> >> + in the object database, then download the `requires` bundle and try\n> >> + again.\n> >> +\n> >> +5. After inspecting a bundle with no negative commit OIDs (or all OIDs are\n> >> + already in the object database somehow), then unbundle all of the\n> >> + bundles in reverse order, placing references within `refs/bundle/*`.\n> >> +\n> >> +6. The client performs a fetch negotiation with the origin server, using\n> >> + the `refs/bundle/*` references as `have`s and the server's ref\n> >> + advertisement as `want`s. This results in a pack-file containing the\n> >> + remaining objects requested by the clone but not in the bundles.\n> >\n> > Does step 6 potentially involve a new, second connection to the origin\n> > server? I'm wondering about timeouts closing the original connection\n> > while the client is downloading the bundle uris. Will the client\n> > handle that automatically, or will they potentially be forced to\n> > re-issue the clone/fetch command? I'm also wondering if we want to be\n> > \"nice\" and pre-emptively close the original connection to the server\n> > while we fetch the bundles -- for example, some servers have a\n> > threadpool for processing fetch/clone requests and will only serve a\n> > limited number; IIRC Gerrit operates this way. I have no idea if\n> > that's a good idea or a horrible one. If a second connection is tried\n> > automatically, will the user potentially be forced to re-enter\n> > connection credentials again? And is there a risk that after the\n> > second connection, there are new bundle uris for the client to go\n> > fetch (and/or a removal of the original ones, e.g. replacing the old\n> > \"daily\" bundle with a new one)? Does this possibly cause us some\n> > timestamp confusion as I noted earlier?\n>\n> If the user is cloning over HTTPS, then the connections are stateless\n> and this is not any different than how it works today.\n>\n> When using SSH, we will probably want to close the SSH connection on\n> the client and then reopen it to avoid keeping that connection open\n> during the download. The implementation in this RFC does _not_ do this,\n> but I think it would be valuable to do.\n\nRight, I was thinking of ssh.\n\n> >> +Note that during a clone we expect that all bundles will be required. The\n> >> +client could be extended to download all bundles in parallel, though they\n> >> +need to be unbundled in the correct order.\n> >\n> > What does required mean? That the origin server can refuse to service\n> > requests if the client does not have commits found in said bundles?\n> > That new enough Git clients are expected to download all the bundles\n> > (and no config option will be provided to users to just do traditional\n> > negotation without first downloading them)? Something else?\n>\n> The assumption I'm making here is that all but on bundle in the table\n> of contents contains a thin pack, depending on an \"earlier\" bundle.\n> The client would be unsuccessful unbundling any single bundle except\n> the earliest one first.\n>\n> The benefit of this assumption is that we could also implement parallel\n> downloads of all bundles in the future.\n>\n> This assumes that there is no way to organize the bundles to communicate\n> that a user might want only the objects reachable from the default branch,\n> but also some users want every reachable object. Such an organization\n> would require extra information to describe two \"parallel\" lists of\n> bundles that could be selected for each of those categories. If such an\n> organization is valuable, then the table of contents can be extended with\n> information to communicate such an organization. The downside is that\n> clients with this \"v1\" version would download extra data based on this\n> assumption.\n\nThat makes sense, and seems to suggest, as you clarify below, that no\nbundles are required. However, that makes your earlier sentence \"Note\nthat during a clone we expect that all bundles will be required\" a bit\njarring and perhaps in need of rewording to clarify the intent (e.g.\n\"Note that during a clone we expect that clients will download and use\nall bundles, not just a subset of them.\").\n\n> > If users are doing a single-branch clone or a shallow clone, will the\n> > requirements still hold? (I'm not a fan of shallow clones, but they\n> > are sadly used in a number of places and I'm curious how the features\n> > interact or conflict.)\n>\n> The current specification does not focus on shallow clones. The TOC\n> could be extended to say \"this bundle is for a shallow clone of\n> commit \" if that was valuable.\n\nI guess it could, though I suspect the easier answer is that clients\nignore the bundle URIs for shallow clones, because doing otherwise is\njust a nightmare for managing bundles with various depths. (As a side\nnote, is the likely result of this series that shallow clones become\nthe bad citizens that put the heaviest load on the origin server?)\n\n> For single-branch clones, my expectation is that the bundles will\n> give the user more information than they need for that clone. The\n> negotiation will find out what they need from that branch that was\n> not in the bundles, but the bundles will also contain a lot of\n> objects that are not reachable from that ref. (This is also up to\n> the discretion of the bundle server operator, since they could\n> focus on only objects reachable from a subset of refs, minimizing\n> the bundle data while increasing the potential size of that\n> follow-up fetch.)\n\nMakes sense to me.\n\n[...]\n> I expect that users who already cloned will never configure their\n> repositories to include a bundle server.\n>\n> That said, if you run 'git bundle fetch ' in an existing\n> repository, then it will fetch only the newest bundle and see if you\n> already have all of its negative refs. If so, then it stops and that\n> is the only bundle that is downloaded. Its timestamp is stored for\n> the next 'git bundle fetch'.\n>\n> In the case where the server starts advertising a bundle URI, a\n> 'git fetch' will not start using that URI. That check only happens\n> during 'git clone' (as currently designed).\n\nSo people have to reclone to take advantage of this performance improvement?\n\nIt probably doesn't matter to me; our repository sizes are such that I\nsuspect we may only use this feature for clones anyway. I'm just\ncurious.\n\n[...]\n> >> +Here are a few example error conditions:\n> >> +\n> >> +* The client fails to connect with a server at the given URI or a connection\n> >> + is lost without any chance to recover.\n> >> +\n> >> +* The client receives a response other than `200 OK` (such as `404 Not Found`,\n> >> + `401 Not Authorized`, or `500 Internal Server Error`).\n[...]\n> >\n> > Should these result in warnings so that folks can diagnose slower\n> > connections, or should they be squelched? (I'm thinking particularly\n> > of the `401 Not Authorized` case in combination with users never\n> > having had to use a credential helper before.)\n>\n> There is a lot of work to be done around polishing the user ergonomics\n> here, and that is an interesting thing to consider for a second round\n> after the basic standard is established. I appreciate that you are\n> already thinking about the user experience in these corner cases.\n\nThanks, but I think this particular situation (using ssh for clone and\npossibly not having credential helpers setup before) isn't so much of\na corner case:\n * Setting up cloning via ssh is easy (on Linux and maybe Mac),\nwhereas https involves more hoops. So, at my previous employer,\ncloning was only available via ssh. (No idea if they've since changed\nor adopted a Git repository manager of some sort.)\n * At current employer, ssh is default, and has at various times been\nthe _only_ option:\n * Back when we used gitolite, gitolite was intrinsically tied to\nssh (probably still is, right?)\n * For years during part of the time we used Gerrit, https cloning\nwas disabled. This was probably a configuration oversight at first,\nbut I inherited the system and didn't notice this for years, likely\nbecause of having migrated from gitolite. I think I enabled it, and\npossibly disabled it again because Gerrit liked to default to https\nfor download links if it was a possibility. Or maybe I disabled it\nagain due to the authentication issues that also affected GHE\n(mentioned below).\n * For a few years during the early usage of GHE, https cloning was\ndisabled. This was not an oversight, but an unfortunate fallout of\nsome kind of issues in our then-authentication scheme (salesforce,\nduo, who knows what else). Something in that stack didn't work\ntogether in a way that somehow only adversely affected cloning (at\nleast to my knowledge), but since everyone was used to cloning solely\nvia ssh anyway, the folks running GHE simply disabled cloning via\nhttps. We've since changed authentication schemes, and it appears\nhttps was re-enabled somewhere along the way (I just tried to verify).\n\nSo, I expect many users here have never cloned any of our internal\nrepositories with https, and thus likely have not set up a credential\nhelper. And my previous employer would have been in the same boat and\nit would not surprise me if they still are. Combine the above with\nthe following:\n * The Infosec team here will not allow unauthenticated source code\naccess; so the bundle access would have to be authenticated here.\n * I'm pretty certain my former employer would have felt the same.\n\nSo, to me at least, this particular case (using ssh for clone and\npossibly not having credential helpers setup) feels pretty common.\nNot that you're doing anything wrong for this case, just noting that\nwe need to make sure such folks are aware of the authentication\nconsiderations.\n"},{"id":"450241","messageId":"d9b87ffa-bbb2-c48b-020c-73068154c8b5@github.com","threadId":"57465","inReplyTo":"CABPp-BFA4fPhrOpfC8ROQAqzofn+7LTiYXmz+AqOFEx5H_sYGA@mail.gmail.com","subject":"Re: [PATCH 01/25] docs: document bundle URI standard","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-03T02:44:53Z","receivedAt":"2022-03-03T02:44:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2022 9:19 PM, Elijah Newren wrote:\n> On Wed, Mar 2, 2022 at 6:06 AM Derrick Stolee wrote:\n>>\n> [...]\n>>>> +bundle..timestamp::\n>>>> + (Optional) This value is the number of seconds since Unix epoch\n>>>> + (UTC) that this bundle was created. This is used as an approximation\n>>>> + of a point in time that the bundle matches the data available at\n>>>> + the origin server.\n>>>\n>>> As an approximation, is there a risk of drift where the user has\n>>> timetamp A which is very close to B and makes decisions based upon it\n>>> which results in their not getting dependent objects they need? Or is\n>>> it just an optimization for them to only download certain bundles and\n>>> look at them, and then they'll iteratively go back and download more\n>>> (as per the 'requires' field below) if they don't have enough objects\n>>> to unbundle what they previously downloaded?\n>>\n>> The user doesn't ever generate the timestamp. It saves the timestamp\n>> from the most-recent bundle it downloaded. The only risk of timestamp\n>> drift is if the server has multiple machines generating different sets\n>> of bundles, and places those machines behind a load balancer.\n>>\n>> This is something the server can control, likely by having one job\n>> generate the bundle set and then distributing them to various storage\n>> locations.\n> \n> My first thought here was \"can, or must?\"...and if it's not \"must\",\n> are there ramifications we have to consider in the design of how the\n> Git client behaves? However, reading through the rest of the document\n> and your response, I don't think there is no problem here because\n> below you mention that when fetching, the client can grab the latest\n> bundle, and if it lacks some of the necessary pre-requisite objects,\n> it can just iteratively download previous bundles until they have\n> enough.\n> \n> Also, the main thing that prompted my question was my wondering about\n> how to get pre-existing clones to start taking advantage of bundle\n> URIs for future fetches, and this timestamp approximation seemed\n> problematic. But in addition to iteratively grabbing more bundles,\n> you also suggested that such users would just be forced to reclone,\n> which would also avoid timestamp approximation issues.\n> \n> And, just now, I noticed the little \"Optional\" you put in the\n> description that I somehow missed previously. I just can't read.\n> Anyway, it sounds like the timestamp isn't used for correctness for\n> the client to know what to grab, it's more just an aid for grabbing a\n> useful initial set.\n\nExactly. It's a heuristic, but it is less noisy than it sounds.\n\nWe could easily replace it with \"generation\" to give an increasing\nnumber (how many times has the bundle server has generated bundles?),\nbut using a timestamp provides some amount of human-understandable\nmeaning to the number.\n\n>>>> +Cloning with Bundle URIs\n>>>> +------------------------\n>>>> +\n>>>> +The primary need for bundle URIs is to speed up clones. The Git client\n>>>> +will interact with bundle URIs according to the following flow:\n>>>> +\n>>>> +1. The user specifies a bundle URI with the `--bundle-uri` command-line\n>>>> + option _or_ the client discovers a bundle URI that was advertised by\n>>>> + the remote server.\n>>>> +\n>>>> +2. The client downloads the file at the bundle URI. If it is a bundle, then\n>>>> + it is unbundled with the refs being stored in `refs/bundle/*`.\n>>>> +\n>>>> +3. If the file is instead a table of contents, then the bundles with\n>>>> + matching `filter` settings are sorted by `timestamp` (if present),\n>>>> + and the most-recent bundle is downloaded.\n>>>> +\n>>>> +4. If the current bundle header mentions negative commid OIDs that are not\n>>>> + in the object database, then download the `requires` bundle and try\n>>>> + again.\n>>>> +\n>>>> +5. After inspecting a bundle with no negative commit OIDs (or all OIDs are\n>>>> + already in the object database somehow), then unbundle all of the\n>>>> + bundles in reverse order, placing references within `refs/bundle/*`.\n>>>> +\n>>>> +6. The client performs a fetch negotiation with the origin server, using\n>>>> + the `refs/bundle/*` references as `have`s and the server's ref\n>>>> + advertisement as `want`s. This results in a pack-file containing the\n>>>> + remaining objects requested by the clone but not in the bundles.\n>>>\n>>> Does step 6 potentially involve a new, second connection to the origin\n>>> server? I'm wondering about timeouts closing the original connection\n>>> while the client is downloading the bundle uris. Will the client\n>>> handle that automatically, or will they potentially be forced to\n>>> re-issue the clone/fetch command? I'm also wondering if we want to be\n>>> \"nice\" and pre-emptively close the original connection to the server\n>>> while we fetch the bundles -- for example, some servers have a\n>>> threadpool for processing fetch/clone requests and will only serve a\n>>> limited number; IIRC Gerrit operates this way. I have no idea if\n>>> that's a good idea or a horrible one. If a second connection is tried\n>>> automatically, will the user potentially be forced to re-enter\n>>> connection credentials again? And is there a risk that after the\n>>> second connection, there are new bundle uris for the client to go\n>>> fetch (and/or a removal of the original ones, e.g. replacing the old\n>>> \"daily\" bundle with a new one)? Does this possibly cause us some\n>>> timestamp confusion as I noted earlier?\n>>\n>> If the user is cloning over HTTPS, then the connections are stateless\n>> and this is not any different than how it works today.\n>>\n>> When using SSH, we will probably want to close the SSH connection on\n>> the client and then reopen it to avoid keeping that connection open\n>> during the download. The implementation in this RFC does _not_ do this,\n>> but I think it would be valuable to do.\n> \n> Right, I was thinking of ssh.\n> \n>>>> +Note that during a clone we expect that all bundles will be required. The\n>>>> +client could be extended to download all bundles in parallel, though they\n>>>> +need to be unbundled in the correct order.\n>>>\n>>> What does required mean? That the origin server can refuse to service\n>>> requests if the client does not have commits found in said bundles?\n>>> That new enough Git clients are expected to download all the bundles\n>>> (and no config option will be provided to users to just do traditional\n>>> negotation without first downloading them)? Something else?\n>>\n>> The assumption I'm making here is that all but on bundle in the table\n>> of contents contains a thin pack, depending on an \"earlier\" bundle.\n>> The client would be unsuccessful unbundling any single bundle except\n>> the earliest one first.\n>>\n>> The benefit of this assumption is that we could also implement parallel\n>> downloads of all bundles in the future.\n>>\n>> This assumes that there is no way to organize the bundles to communicate\n>> that a user might want only the objects reachable from the default branch,\n>> but also some users want every reachable object. Such an organization\n>> would require extra information to describe two \"parallel\" lists of\n>> bundles that could be selected for each of those categories. If such an\n>> organization is valuable, then the table of contents can be extended with\n>> information to communicate such an organization. The downside is that\n>> clients with this \"v1\" version would download extra data based on this\n>> assumption.\n> \n> That makes sense, and seems to suggest, as you clarify below, that no\n> bundles are required. However, that makes your earlier sentence \"Note\n> that during a clone we expect that all bundles will be required\" a bit\n> jarring and perhaps in need of rewording to clarify the intent (e.g.\n> \"Note that during a clone we expect that clients will download and use\n> all bundles, not just a subset of them.\").\n\nYour suggestion to reword is a good one.\n\n>>> If users are doing a single-branch clone or a shallow clone, will the\n>>> requirements still hold? (I'm not a fan of shallow clones, but they\n>>> are sadly used in a number of places and I'm curious how the features\n>>> interact or conflict.)\n>>\n>> The current specification does not focus on shallow clones. The TOC\n>> could be extended to say \"this bundle is for a shallow clone of\n>> commit \" if that was valuable.\n> \n> I guess it could, though I suspect the easier answer is that clients\n> ignore the bundle URIs for shallow clones, because doing otherwise is\n> just a nightmare for managing bundles with various depths. (As a side\n> note, is the likely result of this series that shallow clones become\n> the bad citizens that put the heaviest load on the origin server?)\n> \n>> For single-branch clones, my expectation is that the bundles will\n>> give the user more information than they need for that clone. The\n>> negotiation will find out what they need from that branch that was\n>> not in the bundles, but the bundles will also contain a lot of\n>> objects that are not reachable from that ref. (This is also up to\n>> the discretion of the bundle server operator, since they could\n>> focus on only objects reachable from a subset of refs, minimizing\n>> the bundle data while increasing the potential size of that\n>> follow-up fetch.)\n> \n> Makes sense to me.\n> \n> [...]\n>> I expect that users who already cloned will never configure their\n>> repositories to include a bundle server.\n>>\n>> That said, if you run 'git bundle fetch ' in an existing\n>> repository, then it will fetch only the newest bundle and see if you\n>> already have all of its negative refs. If so, then it stops and that\n>> is the only bundle that is downloaded. Its timestamp is stored for\n>> the next 'git bundle fetch'.\n>>\n>> In the case where the server starts advertising a bundle URI, a\n>> 'git fetch' will not start using that URI. That check only happens\n>> during 'git clone' (as currently designed).\n> \n> So people have to reclone to take advantage of this performance improvement?\n\nTo auto-discover bundle URIs from the origin server, in v1, yes.\n\nIt is possible that we would want fetches to see that the\nrecommended features have been updated, and to have the client\n\"upgrade\" to these features in appropriate cases. But I don't\nanticipate putting that into the first version. This is especially\nimportant for features like partial clone where it doesn't make a\nlot of sense to change the filter (until the refiltering topic is\nfinalized) and maybe not even to toggle sparse-checkout after the\ninitial clone. These kinds of complications are solvable, but are\nprobably too complicated to work out completely in v1.\n\n> It probably doesn't matter to me; our repository sizes are such that I\n> suspect we may only use this feature for clones anyway. I'm just\n> curious.\n\nI anticipate that this will primarily be used for clones except\nfor some very large repos where fetches longer than a day or two\nbehind the latest can be costly.\n\n> [...]\n>>>> +Here are a few example error conditions:\n>>>> +\n>>>> +* The client fails to connect with a server at the given URI or a connection\n>>>> + is lost without any chance to recover.\n>>>> +\n>>>> +* The client receives a response other than `200 OK` (such as `404 Not Found`,\n>>>> + `401 Not Authorized`, or `500 Internal Server Error`).\n> [...]\n>>>\n>>> Should these result in warnings so that folks can diagnose slower\n>>> connections, or should they be squelched? (I'm thinking particularly\n>>> of the `401 Not Authorized` case in combination with users never\n>>> having had to use a credential helper before.)\n>>\n>> There is a lot of work to be done around polishing the user ergonomics\n>> here, and that is an interesting thing to consider for a second round\n>> after the basic standard is established. I appreciate that you are\n>> already thinking about the user experience in these corner cases.\n> \n> Thanks, but I think this particular situation (using ssh for clone and\n> possibly not having credential helpers setup before) isn't so much of\n> a corner case:\n> * Setting up cloning via ssh is easy (on Linux and maybe Mac),\n> whereas https involves more hoops. So, at my previous employer,\n> cloning was only available via ssh. (No idea if they've since changed\n> or adopted a Git repository manager of some sort.)\n> * At current employer, ssh is default, and has at various times been\n> the _only_ option:\n> * Back when we used gitolite, gitolite was intrinsically tied to\n> ssh (probably still is, right?)\n> * For years during part of the time we used Gerrit, https cloning\n> was disabled. This was probably a configuration oversight at first,\n> but I inherited the system and didn't notice this for years, likely\n> because of having migrated from gitolite. I think I enabled it, and\n> possibly disabled it again because Gerrit liked to default to https\n> for download links if it was a possibility. Or maybe I disabled it\n> again due to the authentication issues that also affected GHE\n> (mentioned below).\n> * For a few years during the early usage of GHE, https cloning was\n> disabled. This was not an oversight, but an unfortunate fallout of\n> some kind of issues in our then-authentication scheme (salesforce,\n> duo, who knows what else). Something in that stack didn't work\n> together in a way that somehow only adversely affected cloning (at\n> least to my knowledge), but since everyone was used to cloning solely\n> via ssh anyway, the folks running GHE simply disabled cloning via\n> https. We've since changed authentication schemes, and it appears\n> https was re-enabled somewhere along the way (I just tried to verify).\n> \n> So, I expect many users here have never cloned any of our internal\n> repositories with https, and thus likely have not set up a credential\n> helper. And my previous employer would have been in the same boat and\n> it would not surprise me if they still are. Combine the above with\n> the following:\n> * The Infosec team here will not allow unauthenticated source code\n> access; so the bundle access would have to be authenticated here.\n> * I'm pretty certain my former employer would have felt the same.\n> \n> So, to me at least, this particular case (using ssh for clone and\n> possibly not having credential helpers setup) feels pretty common.\n> Not that you're doing anything wrong for this case, just noting that\n> we need to make sure such folks are aware of the authentication\n> considerations.\n\nI completely agree that SSH clones are _not_ a corner case and\nshould be front-and-center in our minds when moving to the server-\nadvertised bundles. If a user specifies a --bundle-uri argument\nto the 'git clone', then we can expect that they will have proper\ncredential managers configured to handle the supplied URIs.\n\nI was thinking more about the cases where a user is having other\nkinds of network issues to the bundle URIs that are not due to\nthis auth issue, but you mention that as your main example, so I\ncan now see how my statement looks like I'm referring to SSH.\n\nThanks,\n-Stolee\n"},{"id":"450395","messageId":"a6981d6e-16b0-b0e1-a94d-a87ec20871bd@github.com","threadId":"57465","inReplyTo":"15aed4cc-2d16-0b3f-5235-f7858a705c52@github.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-04T13:30:29Z","receivedAt":"2022-03-04T13:30:34Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/24/2022 9:11 AM, Derrick Stolee wrote:\n> On 2/23/2022 5:17 PM, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:\n\n>>> There have been several suggestions to improve Git clone speeds and\n>>> reliability by supplementing the Git protocol with static content. The\n>>> Packfile URI [0] feature lets the Git response include URIs that point to\n>>> packfiles that the client must download to complete the request.\n>>>\n>>> Last year, Ævar suggested using bundles instead of packfiles [1] [2]. This\n>>> design has the same benefits to the packfile URI feature because it offloads\n>>> most object downloads to static content fetches. The main advantage over\n>>> packfile URIs is that the remote Git server does not need to know what is in\n>>> those bundles. The Git client tells the server what it downloaded during the\n>>> fetch negotiation afterwards. This includes any chance that the client did\n>>> not have access to those bundles or otherwise failed to access them. I\n>>> agreed that this was a much more desirable way to serve static content, but\n>>> had concerns about the flexibility of that design [3]. I have not heard more\n>>> on the topic since October, so I started investigating this idea myself in\n>>> December, resulting in this RFC.\n>>\n>> This timing is both quite fortunate & unfortunate for me, since I'd been\n>> blocked / waiting on various things until very recently to submit a\n>> non-RFC re-roll of (a larger version of) that series you mentioned from\n>> October.\n>>\n>> I guess the good news is that we'll have at least one guaranteed very\n>> interested reviewer for each other's patches, and that the design that\n>> makes it into git.git in the end will definitely be well hashed out :)\n>>\n>> I won't be able to review this in any detail right at this hour, but\n>> will be doing so. I'd also like to submit what I've got in some form\n>> soon for hashing the two out.\n>>\n>> It will be some 50+ patches on the ML in total though related to this\n>> topic, so I think the two of us coming up with some way to manage all of\n>> that for both ourselves & others would be nice. Perhaps we could also\n>> have an off-list (video) chat in real time to clarify/discuss various\n>> thing related to this.\n> \n> I look forward to seeing your full implementation. There are many things\n> about your RFC that left me confused and not fully understanding your\n> vision.\n\nI am genuinely curious to see your full implementation of bundle URIs.\nI've been having trouble joining the Git IRC chats, but I saw from the\nlogs that you are working on getting patches together.\n\nDo you have an expected timeline on that progress?\n\nI would like to move forward in getting bundle URIs submitted as a full\nfeature, but it is important to see your intended design so we can take\nthe best parts of both to create a version that satisfies us both.\n\nThanks,\n-Stolee\n"},{"id":"450401","messageId":"220304.86a6e5g44z.gmgdl@evledraar.gmail.com","threadId":"57465","inReplyTo":"a6981d6e-16b0-b0e1-a94d-a87ec20871bd@github.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T14:49:58Z","receivedAt":"2022-03-04T14:54:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Mar 04 2022, Derrick Stolee wrote:\n\n> On 2/24/2022 9:11 AM, Derrick Stolee wrote:\n>> On 2/23/2022 5:17 PM, Ævar Arnfjörð Bjarmason wrote:\n>>>\n>>> On Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:\n>\n>>>> There have been several suggestions to improve Git clone speeds and\n>>>> reliability by supplementing the Git protocol with static content. The\n>>>> Packfile URI [0] feature lets the Git response include URIs that point to\n>>>> packfiles that the client must download to complete the request.\n>>>>\n>>>> Last year, Ævar suggested using bundles instead of packfiles [1] [2]. This\n>>>> design has the same benefits to the packfile URI feature because it offloads\n>>>> most object downloads to static content fetches. The main advantage over\n>>>> packfile URIs is that the remote Git server does not need to know what is in\n>>>> those bundles. The Git client tells the server what it downloaded during the\n>>>> fetch negotiation afterwards. This includes any chance that the client did\n>>>> not have access to those bundles or otherwise failed to access them. I\n>>>> agreed that this was a much more desirable way to serve static content, but\n>>>> had concerns about the flexibility of that design [3]. I have not heard more\n>>>> on the topic since October, so I started investigating this idea myself in\n>>>> December, resulting in this RFC.\n>>>\n>>> This timing is both quite fortunate & unfortunate for me, since I'd been\n>>> blocked / waiting on various things until very recently to submit a\n>>> non-RFC re-roll of (a larger version of) that series you mentioned from\n>>> October.\n>>>\n>>> I guess the good news is that we'll have at least one guaranteed very\n>>> interested reviewer for each other's patches, and that the design that\n>>> makes it into git.git in the end will definitely be well hashed out :)\n>>>\n>>> I won't be able to review this in any detail right at this hour, but\n>>> will be doing so. I'd also like to submit what I've got in some form\n>>> soon for hashing the two out.\n>>>\n>>> It will be some 50+ patches on the ML in total though related to this\n>>> topic, so I think the two of us coming up with some way to manage all of\n>>> that for both ourselves & others would be nice. Perhaps we could also\n>>> have an off-list (video) chat in real time to clarify/discuss various\n>>> thing related to this.\n>> \n>> I look forward to seeing your full implementation. There are many things\n>> about your RFC that left me confused and not fully understanding your\n>> vision.\n>\n> I am genuinely curious to see your full implementation of bundle URIs.\n> I've been having trouble joining the Git IRC chats, but I saw from the\n> logs that you are working on getting patches together.\n>\n> Do you have an expected timeline on that progress?\n>\n> I would like to move forward in getting bundle URIs submitted as a full\n> feature, but it is important to see your intended design so we can take\n> the best parts of both to create a version that satisfies us both.\n\nHi. Very sorry about the late reply. I relly meant to have something\nread to send at the end of this week, but it's been a bit of a\nwhirlwhind of random things coming up & distracting me too much. Sorry.\n\nI'm also totally on board with that goal, are you OK with waiting until\nthe end of next week at the latest?\n\nAlso, as noted in the upthread\n<220224.86czjdb22l.gmgdl@evledraar.gmail.com> it might be useful to chat\nin a more voice/video medium in parallel (maybe mid-next-week) about the\nhigh-level ideas & to get a feel for our goals, conflicts etc. Doing\nthat over very long E-Mail exchanges (and the fault of \"long\" there is\nmostly on my side:) can be a bit harder...\n"},{"id":"450410","messageId":"1469e420-63e5-e2db-21d5-c70674ab04d5@github.com","threadId":"57465","inReplyTo":"220304.86a6e5g44z.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-04T15:12:55Z","receivedAt":"2022-03-04T15:13:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/4/2022 9:49 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Mar 04 2022, Derrick Stolee wrote:\n> \n>> On 2/24/2022 9:11 AM, Derrick Stolee wrote:\n>>> I look forward to seeing your full implementation. There are many things\n>>> about your RFC that left me confused and not fully understanding your\n>>> vision.\n>>\n>> I am genuinely curious to see your full implementation of bundle URIs.\n>> I've been having trouble joining the Git IRC chats, but I saw from the\n>> logs that you are working on getting patches together.\n>>\n>> Do you have an expected timeline on that progress?\n>>\n>> I would like to move forward in getting bundle URIs submitted as a full\n>> feature, but it is important to see your intended design so we can take\n>> the best parts of both to create a version that satisfies us both.\n> \n> Hi. Very sorry about the late reply. I relly meant to have something\n> read to send at the end of this week, but it's been a bit of a\n> whirlwhind of random things coming up & distracting me too much. Sorry.\n> \n> I'm also totally on board with that goal, are you OK with waiting until\n> the end of next week at the latest?\n\nI'm OK with waiting, especially when I have a timeline in mind.\n \n> Also, as noted in the upthread\n> <220224.86czjdb22l.gmgdl@evledraar.gmail.com> it might be useful to chat\n> in a more voice/video medium in parallel (maybe mid-next-week) about the\n> high-level ideas & to get a feel for our goals, conflicts etc. Doing\n> that over very long E-Mail exchanges (and the fault of \"long\" there is\n> mostly on my side:) can be a bit harder...\n\nI agree. I we can work out a time in a private thread and I can send\nyou a video call invite.\n\nThanks,\n-Stolee\n"},{"id":"450714","messageId":"20220308081838.29282-1-dyroneteng@gmail.com","threadId":"57465","inReplyTo":"220224.86czjdb22l.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-03-08T08:18:38Z","receivedAt":"2022-03-08T08:18:47Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote on Wed, 23 Feb 2022 23:17:22 +0100:\n\n>[Note: The E-Mail address you CC'd for me (presumably, dropped in this\n>reply) is not my E-Mail address, this one is]\n>\n>[Also CC-ing some people who have expressed interest in this are, and\n>would probably like to be kept in the loop going forward]\n\nAppreciate for the CC. \n\nI'm attractive on this for a while. On a earlier time, I had posted a\npatchset about to extend \"packfile-uri\" for common or similar reasons,\nbut after I saw the idea and RFC from Ævar Arnfjörð Bjarmason, I suspended\nit.\n\nReally looking forward the solution after you guys reach the consensuses and\nI'm glad to attend and listen to your meetings, if possible (I looked at the\ncontext a little bit late, maybe already missed it).\n\nThanks.\n"},{"id":"450760","messageId":"ddebc223-1e13-e758-f9b1-d3f23961e459@github.com","threadId":"57465","inReplyTo":"1469e420-63e5-e2db-21d5-c70674ab04d5@github.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-08T17:15:29Z","receivedAt":"2022-03-08T17:15:40Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/4/2022 10:12 AM, Derrick Stolee wrote:\n> On 3/4/2022 9:49 AM, Ævar Arnfjörð Bjarmason wrote:\n>> Also, as noted in the upthread\n>> <220224.86czjdb22l.gmgdl@evledraar.gmail.com> it might be useful to chat\n>> in a more voice/video medium in parallel (maybe mid-next-week) about the\n>> high-level ideas & to get a feel for our goals, conflicts etc. Doing\n>> that over very long E-Mail exchanges (and the fault of \"long\" there is\n>> mostly on my side:) can be a bit harder...\n> \n> I agree. I we can work out a time in a private thread and I can send\n> you a video call invite.\n\nÆvar and I just finished our chat and came away with these two\naction items:\n\n1. Ævar will finish prepping his RFC as-is and send it to the list.\n It contains several deeply technical optimizations that are\n critical to how his model works, but could also be used to\n improve scenarios in the table of contents model.\n\n2. Ævar will then do a round of taking both series and combining\n them in a way that allows the union of possible functionality\n to work.\n\n3. As these things come out, I will make it a priority to read the\n patches and provide feedback focusing on high-level concepts\n and ways we can split the future, non-RFC series into chunks\n that provide incremental functionality while keeping review\n easier than reading the whole series.\n\nThanks,\n-Stolee\n"},{"id":"451025","messageId":"nycvar.QRO.7.76.6.2203101545140.357@tvgsbejvaqbjf.bet","threadId":"57465","inReplyTo":"ddebc223-1e13-e758-f9b1-d3f23961e459@github.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-03-10T14:45:45Z","receivedAt":"2022-03-10T14:54:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stolee & Ævar,\n\nOn Tue, 8 Mar 2022, Derrick Stolee wrote:\n\n> On 3/4/2022 10:12 AM, Derrick Stolee wrote:\n> > On 3/4/2022 9:49 AM, Ævar Arnfjörð Bjarmason wrote:\n> >> Also, as noted in the upthread\n> >> <220224.86czjdb22l.gmgdl@evledraar.gmail.com> it might be useful to chat\n> >> in a more voice/video medium in parallel (maybe mid-next-week) about the\n> >> high-level ideas & to get a feel for our goals, conflicts etc. Doing\n> >> that over very long E-Mail exchanges (and the fault of \"long\" there is\n> >> mostly on my side:) can be a bit harder...\n> >\n> > I agree. I we can work out a time in a private thread and I can send\n> > you a video call invite.\n>\n> Ævar and I just finished our chat and came away with these two\n> action items:\n>\n> 1. Ævar will finish prepping his RFC as-is and send it to the list.\n> It contains several deeply technical optimizations that are\n> critical to how his model works, but could also be used to\n> improve scenarios in the table of contents model.\n>\n> 2. Ævar will then do a round of taking both series and combining\n> them in a way that allows the union of possible functionality\n> to work.\n>\n> 3. As these things come out, I will make it a priority to read the\n> patches and provide feedback focusing on high-level concepts\n> and ways we can split the future, non-RFC series into chunks\n> that provide incremental functionality while keeping review\n> easier than reading the whole series.\n\nI very much look forward to see the combined work soon!\n\nThank you, both,\nJohannes\n"},{"id":"451127","messageId":"220311.86pmmshahy.gmgdl@evledraar.gmail.com","threadId":"57465","inReplyTo":"7ec7ba3f328f9d6fa7f919ce18a138840d44ee49.1645641063.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 15/25] config: add git_config_get_timestamp()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T13:32:33Z","receivedAt":"2022-03-11T13:41:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n>\n> The existing config parsing methods do not include a way to consistently\n> parse timestamps across all platforms. Recall that \"unsigned long\" is\n> 32 bits on 64-bit Windows, so git_config_get_ulong() is insufficient.\n\nThey do. I think you've just missed the slightly non-obviously-named\nexpiry-date type:\n\n $ for t in $(date +%s) 2.days.ago 1.week.ago; do echo $t = $(git -c x.y=$t config --type expiry-date --get x.y); done\n 1647005519 = 1647005519\n 2.days.ago = 1646832719\n 1.week.ago = 1646400719\n\nAFAICT there's no need for these methods, except perhaps as some wrapper\nfor the existing git_config_expiry_date(), perhaps to have a flag passed\ndown to only allow epochs, and not the human-readable format?\n\nBut for the TOC proposal I don't see why it wouldn't allow the\nhuman-readable format. You can arguably shoot yourself in the foot with\n2.days.ago, but you can also provide absolute times in that format.\n\nIn any case, peeling out this patch from this series to fast-track it if\nyou do need it sounds like a good idea, it would need the relevant bits\nof the CLI UX implemented & docs added.\n\nThere's also tests that you'll find by grepping for \"expiry-date\" to\nextend.\n\n> RFC-QUESTION: Would this be better to use uintmax_t, which could be cast\n> to timestamp_t or other types more robust than \"unsigned long\"?\n\nI think it's certainly legitimate to re-visit that when we're talking\nabout making it part of a network format.\n\nBut I think we should just go for timestamp_t, it will break 32 bit\nclients around the year 2038, but I think that probably the intersection\nbetween clients who'll find this useful and those still using 32 bit\nsystems then will be pretty much zero :)\n\nSo I think the only thing we need for bundle-uri if it has a TOC with a\ntimestamp is to have it die/fallback properly if it can't parse the\nvalues.\n\n> [...]\n> +\t\t*dest = git_config_timestamp(key, value);\n> +\t\treturn 0;\n> +\t} else\n> +\t\treturn 1;\n\nNit: missing {}\n\n>[...]\n> --- a/config.h\n> +++ b/config.h\n> @@ -206,6 +206,7 @@ int config_with_options(config_fn_t fn, void *,\n> \n> int git_parse_ssize_t(const char *, ssize_t *);\n> int git_parse_ulong(const char *, unsigned long *);\n> +int git_parse_timestamp(const char *, timestamp_t *);\n> \n> /**\n> * Same as `git_config_bool`, except that it returns -1 on error rather\n> @@ -226,6 +227,11 @@ int64_t git_config_int64(const char *, const char *);\n> */\n> unsigned long git_config_ulong(const char *, const char *);\n> \n> +/**\n> + * Identical to `git_config_int`, but for (unsigned) timestamps.\n> + */\n> +timestamp_t git_config_timestamp(const char *name, const char *value);\n> +\n> ssize_t git_config_ssize_t(const char *, const char *);\n> \n> /**\n> @@ -469,6 +475,7 @@ int git_configset_get_string(struct config_set *cs, const char *key, char **dest\n> int git_configset_get_string_tmp(struct config_set *cs, const char *key, const char **dest);\n> int git_configset_get_int(struct config_set *cs, const char *key, int *dest);\n> int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest);\n> +int git_configset_get_timestamp(struct config_set *cs, const char *key, timestamp_t *dest);\n> int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);\n> int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);\n> int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);\n> @@ -489,6 +496,8 @@ int repo_config_get_int(struct repository *repo,\n> \t\t\tconst char *key, int *dest);\n> int repo_config_get_ulong(struct repository *repo,\n> \t\t\t const char *key, unsigned long *dest);\n> +int repo_config_get_timestamp(struct repository *repo,\n> +\t\t\t const char *key, timestamp_t *dest);\n> int repo_config_get_bool(struct repository *repo,\n> \t\t\t const char *key, int *dest);\n> int repo_config_get_bool_or_int(struct repository *repo,\n> @@ -558,6 +567,11 @@ int git_config_get_int(const char *key, int *dest);\n> */\n> int git_config_get_ulong(const char *key, unsigned long *dest);\n> \n> +/**\n> + * Similar to `git_config_get_int` but for (unsigned) timestamps.\n> + */\n> +int git_config_get_timestamp(const char *key, timestamp_t *dest);\n> +\n\nNot your fault, but it's really tiresome that whenever we need to add\nstuff to this API it's this mostly copy/pasting of related methods. I\nhad some recent-ish patches that did something similar & it was a chore\n:)\n"},{"id":"451128","messageId":"220311.86lexghafe.gmgdl@evledraar.gmail.com","threadId":"57465","inReplyTo":"bdd5c664cde070a93d7e698aff2ab11ce7557ee2.1645641063.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 11/25] bundle: allow relative URLs in table of contents","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T13:42:53Z","receivedAt":"2022-03-11T13:43:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> [...]\n> @@ -460,6 +463,10 @@ static int cmd_bundle_fetch(int argc, const char **argv, const char *prefix)\n> \t\t\t (!info->filter_str || strcasecmp(filter, info->filter_str)))\n> \t\t\t\tcontinue;\n> \n> +\t\t\told_uri = info->uri;\n> +\t\t\tinfo->uri = relative_url(bundle_uri, info->uri, NULL);\n> +\t\t\tfree(old_uri);\n> +\n\nI had it on my TODO to look into how to do this, and hadn't dug yet,\nit's really pleasing that we have an API to make it this simple.\n"},{"id":"451129","messageId":"220311.86h784habh.gmgdl@evledraar.gmail.com","threadId":"57465","inReplyTo":"fa0e83b39dd3c3bb16e932c39aff080a1fdf8fe4.1645641063.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 10/25] bundle: add --filter option to 'fetch'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T13:44:36Z","receivedAt":"2022-03-11T13:45:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> [...]\n> +\t\t\t/* Skip if filter does not match. */\n> +\t\t\tif (!filter && info->filter_str)\n> +\t\t\t\tcontinue;\n> +\t\t\tif (filter &&\n> +\t\t\t (!info->filter_str || strcasecmp(filter, info->filter_str)))\n> +\t\t\t\tcontinue;\n\nGeneral API comment: Do we have a need for strcasecmp() now anywhere\nwhen dealing with filters?\n\n $ git clone --no-checkout --filter=BlOb:nOnE https://github.com/git/git\n fatal: invalid filter-spec 'BlOb:nOnE'\n"},{"id":"451130","messageId":"220311.86czisha13.gmgdl@evledraar.gmail.com","threadId":"57465","inReplyTo":"a9bcb6065523e2d2307fdc5b7bc92b04aba012e8.1645641063.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/25] bundle: alphabetize subcommands better","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T13:47:18Z","receivedAt":"2022-03-11T13:51:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n>\n> The usage strings for the 'git bundle' subcommands are not alphabetical.\n> This also applies to their inspection within cmd_bundle(). Fix this\n> ordering before we insert a new subcommand.\n>\n> This change does not reorder the cmd_bundle_*() methods to avoid moving\n> lines that are more likely wanted in a future 'git blame' call. It is\n> fine that those longer methods are not ordered alphabetically.\n\nIf we're moving these around it makes sense to make them use the pattern\nof macros we use in commit-graph.c. I could have sworn I changed that\nhere already, but obviously not...\n\nAnyway, I think as general UX I thought having these in alphabetical\norder was a non-goal. Doesn't it make more for these commands in general\nto list themost frequently used commands first.\n\nIn this case though there's pretty much a mapping between that and what\nhappens to be alphabetical order.\n\nBut I'd think if e.g. we implemented an \"add-ref\" or whatever to\nin-place munge a bundle file to add more data to it such an obscure\nthing would be better off last, before create/unbundle/list-heads/verify\netc.\n\nJut my 0.02, but for this change I really don't mind it, other than the\nmacro suggestion...\n"},{"id":"453286","messageId":"1b63a8eb-9a6d-a07e-9070-9cdc0c4d5a1c@github.com","threadId":"57465","inReplyTo":"ddebc223-1e13-e758-f9b1-d3f23961e459@github.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-04-07T19:08:16Z","receivedAt":"2022-04-07T19:08:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/8/2022 12:15 PM, Derrick Stolee wrote:\n> On 3/4/2022 10:12 AM, Derrick Stolee wrote:\n>> On 3/4/2022 9:49 AM, Ævar Arnfjörð Bjarmason wrote:\n>>> Also, as noted in the upthread\n>>> <220224.86czjdb22l.gmgdl@evledraar.gmail.com> it might be useful to chat\n>>> in a more voice/video medium in parallel (maybe mid-next-week) about the\n>>> high-level ideas & to get a feel for our goals, conflicts etc. Doing\n>>> that over very long E-Mail exchanges (and the fault of \"long\" there is\n>>> mostly on my side:) can be a bit harder...\n>>\n>> I agree. I we can work out a time in a private thread and I can send\n>> you a video call invite.\n> \n> Ævar and I just finished our chat and came away with these two\n> action items:\n> \n> 1. Ævar will finish prepping his RFC as-is and send it to the list.\n> It contains several deeply technical optimizations that are\n> critical to how his model works, but could also be used to\n> improve scenarios in the table of contents model.\n\nÆvar: I'm still waiting on the full version of this. While you\nupdated [1] your original RFC [2], it was incomplete. I am still\nlooking forward to seeing your full vision of how it works with\nincremental fetch and how your optimizations to download only the\nheaders of the bundles will work.\n\n[1] [RFC PATCH v2 00/13] bundle-uri: a \"dumb CDN\" for git\n https://lore.kernel.org/git/RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com/\n\n[2] [RFC PATCH 00/13] Add bundle-uri: resumably clones, static \"dumb\" CDN etc.\n https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n\n> 2. Ævar will then do a round of taking both series and combining\n> them in a way that allows the union of possible functionality\n> to work.\n\nOr perhaps you are jumping straight to this part?\n\n> 3. As these things come out, I will make it a priority to read the\n> patches and provide feedback focusing on high-level concepts\n> and ways we can split the future, non-RFC series into chunks\n> that provide incremental functionality while keeping review\n> easier than reading the whole series.\n\nI'm still looking forward to seeing progress in this area. Please\nlet me know what your plan is here.\n\nThanks,\n-Stolee\n"},{"id":"453323","messageId":"220408.86zgkwaq2i.gmgdl@evledraar.gmail.com","threadId":"57465","inReplyTo":"1b63a8eb-9a6d-a07e-9070-9cdc0c4d5a1c@github.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-08T09:15:25Z","receivedAt":"2022-04-08T09:21:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 07 2022, Derrick Stolee wrote:\n\n> On 3/8/2022 12:15 PM, Derrick Stolee wrote:\n>> On 3/4/2022 10:12 AM, Derrick Stolee wrote:\n>>> On 3/4/2022 9:49 AM, Ævar Arnfjörð Bjarmason wrote:\n>>>> Also, as noted in the upthread\n>>>> <220224.86czjdb22l.gmgdl@evledraar.gmail.com> it might be useful to chat\n>>>> in a more voice/video medium in parallel (maybe mid-next-week) about the\n>>>> high-level ideas & to get a feel for our goals, conflicts etc. Doing\n>>>> that over very long E-Mail exchanges (and the fault of \"long\" there is\n>>>> mostly on my side:) can be a bit harder...\n>>>\n>>> I agree. I we can work out a time in a private thread and I can send\n>>> you a video call invite.\n>> \n>> Ævar and I just finished our chat and came away with these two\n>> action items:\n>> \n>> 1. Ævar will finish prepping his RFC as-is and send it to the list.\n>> It contains several deeply technical optimizations that are\n>> critical to how his model works, but could also be used to\n>> improve scenarios in the table of contents model.\n>\n> Ævar: I'm still waiting on the full version of this. While you\n> updated [1] your original RFC [2], it was incomplete. I am still\n> looking forward to seeing your full vision of how it works with\n> incremental fetch and how your optimizations to download only the\n> headers of the bundles will work.\n>\n> [1] [RFC PATCH v2 00/13] bundle-uri: a \"dumb CDN\" for git\n> https://lore.kernel.org/git/RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com/\n>\n> [2] [RFC PATCH 00/13] Add bundle-uri: resumably clones, static \"dumb\" CDN etc.\n> https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n>\n>> 2. Ævar will then do a round of taking both series and combining\n>> them in a way that allows the union of possible functionality\n>> to work.\n>\n> Or perhaps you are jumping straight to this part?\n\nYeah, that was part of it...\n\n>> 3. As these things come out, I will make it a priority to read the\n>> patches and provide feedback focusing on high-level concepts\n>> and ways we can split the future, non-RFC series into chunks\n>> that provide incremental functionality while keeping review\n>> easier than reading the whole series.\n>\n> I'm still looking forward to seeing progress in this area. Please\n> let me know what your plan is here.\n\nHi. I'm sorry about the delay, I ran into various life/software things,\nand found that this topic required a lot of continuous \"sit down for a\nday and work on it\" attention from me v.s. some other topics where I'd\ndeal with interruption better.\n\nThen I was hoping that the merger of your bundle.c changes would come a\nbit earlier before the rc, but they pretty much coincided, and since the\nrc dropped I've been hesitant to send a very large topic to the list\n(c.f. e.g. [1]).\n\nMaybe I should just bite the bullet and submit it anyway, what do you\nthink?\n\n1. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2204071407160.347@tvgsbejvaqbjf.bet/\n"},{"id":"453332","messageId":"b4f85515-945b-51e1-285f-85345124e9b4@github.com","threadId":"57465","inReplyTo":"220408.86zgkwaq2i.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-04-08T13:13:40Z","receivedAt":"2022-04-08T13:16:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/8/2022 5:15 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Thu, Apr 07 2022, Derrick Stolee wrote:\n>> Ævar: I'm still waiting on the full version of this. While you\n>> updated [1] your original RFC [2], it was incomplete. I am still\n>> looking forward to seeing your full vision of how it works with\n>> incremental fetch and how your optimizations to download only the\n>> headers of the bundles will work.\n>>\n>> [1] [RFC PATCH v2 00/13] bundle-uri: a \"dumb CDN\" for git\n>> https://lore.kernel.org/git/RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com/\n>>\n>> [2] [RFC PATCH 00/13] Add bundle-uri: resumably clones, static \"dumb\" CDN etc.\n>> https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n>>\n>>> 2. Ævar will then do a round of taking both series and combining\n>>> them in a way that allows the union of possible functionality\n>>> to work.\n>>\n>> Or perhaps you are jumping straight to this part?\n> \n> Yeah, that was part of it...\n> \n>>> 3. As these things come out, I will make it a priority to read the\n>>> patches and provide feedback focusing on high-level concepts\n>>> and ways we can split the future, non-RFC series into chunks\n>>> that provide incremental functionality while keeping review\n>>> easier than reading the whole series.\n>>\n>> I'm still looking forward to seeing progress in this area. Please\n>> let me know what your plan is here.\n> \n> Hi. I'm sorry about the delay, I ran into various life/software things,\n> and found that this topic required a lot of continuous \"sit down for a\n> day and work on it\" attention from me v.s. some other topics where I'd\n> deal with interruption better.\n> \n> Then I was hoping that the merger of your bundle.c changes would come a\n> bit earlier before the rc, but they pretty much coincided, and since the\n> rc dropped I've been hesitant to send a very large topic to the list\n> (c.f. e.g. [1]).\n> \n> Maybe I should just bite the bullet and submit it anyway, what do you\n> think?\n> \n> 1. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2204071407160.347@tvgsbejvaqbjf.bet/\n\nWe could err on the side of not distracting from the list. I tend to\nthink the release phase can be a good time to talk about an RFC, since\nJunio can ignore the thread until it is actually ready for full review.\n\nAt minimum, I appreciate knowing your progress. I can be patient for a\nfew more weeks as long as I have confidence that it will be shared as\nsoon as external blockers are cleared.\n\nThanks!\n-Stolee\n"},{"id":"453344","messageId":"xmqqlewfmnx2.fsf@gitster.g","threadId":"57465","inReplyTo":"b4f85515-945b-51e1-285f-85345124e9b4@github.com","subject":"Re: [PATCH 00/25] [RFC] Bundle URIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-08T18:26:49Z","receivedAt":"2022-04-08T18:26:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> We could err on the side of not distracting from the list. I tend to\n> think the release phase can be a good time to talk about an RFC, since\n\nYes, I think that it is a good thing to do. I'd need to be careful\nnot to miss updates and issues critical to the upcoming release but\na large topic that clearly marks itself as RFC would not get in the\nway, I would think.\n\n> Junio can ignore the thread until it is actually ready for full review.\n\nIt is true for both during -rc freeze and outside ;-)\n"}]}