{"thread":{"id":"56782","subject":"[PATCH 0/3] bundle-uri: \"dumb\" static CDN offloading, spec & server implementation","startedAt":"2021-10-25T21:26:10Z","lastAt":"2022-04-22T09:37:55Z","messageCount":77,"participants":["Ævar Arnfjörð Bjarmason","Derrick Stolee","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"439593","messageId":"cover-0.3-00000000000-20211025T211159Z-avarab@gmail.com","threadId":"56782","inReplyTo":null,"subject":"[PATCH 0/3] bundle-uri: \"dumb\" static CDN offloading, spec & server implementation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-25T21:25:43Z","receivedAt":"2021-10-25T21:26:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This implements a new \"bundle-uri\" protocol v2 extension, which allows\nservers to advertise *.bundle files which clients can pre-seed their\nfull \"clone\"'s or incremental \"fetch\"'s from.\n\nThis is both an alternative to, and complimentary to the existing\n\"packfile-uri\" mechanism, i.e. servers and/or clients can pick one or\nboth, but would generally pick one over the other.\n\nThis \"bundle-uri\" mechanism has the advantage of being dumber, and\noffloads more complexity from the server side to the client\nside.\n\nUnlike with packfile-uri a conforming server doesn't need produce a\nPACK that (hopefully, otherwise there's not much point) excludes OIDs\nthat it knows it'll provide via a packfile-uri.\n\nTo the server a \"bundle-uri\" negotiation the same as a \"normal\" one,\nthe client just happens to provide OIDs it found in bundles as \"have\"\nlines.\n\nIn my WIP client patches I even have a (trivial to implement) mode\nwhere a client can choose to pretend that a server reported that a\ngiven set of bundle URIs can be used to pre-seed its \"clone\" or\n\"fetch\".\n\nA client can thus use use a CDN it controls to optimistically pre-seed\na clone from a server that knows nothing about \"bundle-uri\", sort of\nlike a \"git clone --reference --dissociate\", except with a\n instead of a .\n\nNeed re-clone a bunch of large repositories on CI boxes from\ngit.example.com, but git.example.com doesn't support \"bundle-uri\", and\nyou've got a slow outbound connection? Just point to a pre-seeding CDN\nyou control.\n\nThere are disadvantages to this over packfile-uri, JGit has a mature\nimplementation of it, and I doubt that e.g. Google will ever want to\nuse this, since that feature was tailor-made for their use case.\n\nE.g. a repository that has a *.pack sitting on disk can't re-use and\nstream it out with sendfile() as it could with a \"packfile-uri\",\ninstead it would need to point to some duplicate of that data in\n*.bundle form (or on-the-fly generate a header for the *.pack).\n\nThe goal of this feature isn't to win over packfile-uri users, but to\ngive users who wouldn't consider it due to its tight coupling to have\naccess to CDN offloading.\n\nThe error optimistic recovery of \"bundle-uri\" and looseer coupling\nbetween server and CDN means that it should be easy to use this for\nuse where the CDN is something like say Debian's mirror network.\n\nWe're coming up on 2.34.0-rc0, so this certainly won't be in 2.34.0,\nbut I'm submitting this now per discussion during '#git-devel' standup\ntoday.\n\nThere was a discussion on the RFC version of the larger series of\npatches to implement this \"bundle-uri\"[1].\n\nI've updated the protocol-v2.txt changes in 2/3 a lot in response to\nthat, in particular I've specified and implemented early client\ndisconnection behavior, so bundle-uri SHOULD never cause\nclient<->server dialog to hang (at most we'll need to re-connect, if\nwe need to fall back from a failed bundle-uri).\n\n1. https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n\nÆvar Arnfjörð Bjarmason (3):\n leak tests: mark t5701-git-serve.sh as passing SANITIZE=leak\n protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"\n bundle-uri client: add \"bundle-uri\" parsing + tests\n\n Documentation/technical/protocol-v2.txt | 209 ++++++++++++++++++++++++\n Makefile | 2 +\n bundle-uri.c | 179 ++++++++++++++++++++\n bundle-uri.h | 30 ++++\n serve.c | 6 +\n t/helper/test-bundle-uri.c | 83 ++++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t5701-git-serve.sh | 125 +++++++++++++-\n t/t5750-bundle-uri-parse.sh | 153 +++++++++++++++++\n 10 files changed, 788 insertions(+), 1 deletion(-)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n create mode 100644 t/helper/test-bundle-uri.c\n create mode 100755 t/t5750-bundle-uri-parse.sh\n\n-- \n2.33.1.1511.gd15d1b313a6\n\n"},{"id":"439594","messageId":"patch-1.3-7639b9bbac5-20211025T211159Z-avarab@gmail.com","threadId":"56782","inReplyTo":"cover-0.3-00000000000-20211025T211159Z-avarab@gmail.com","subject":"[PATCH 1/3] leak tests: mark t5701-git-serve.sh as passing SANITIZE=leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-25T21:25:44Z","receivedAt":"2021-10-25T21:26:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"t5701-git-serve.sh\" test passes when run under a git compiled\nwith SANITIZE=leak, let's mark it as such to add it to the\n\"linux-leaks\" CI job.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5701-git-serve.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\nindex aa1827d841d..1896f671cb3 100755\n--- a/t/t5701-git-serve.sh\n+++ b/t/t5701-git-serve.sh\n@@ -5,6 +5,7 @@ test_description='test protocol v2 server commands'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'test capability advertisement' '\n-- \n2.33.1.1511.gd15d1b313a6\n\n"},{"id":"439595","messageId":"patch-2.3-3ac0539c053-20211025T211159Z-avarab@gmail.com","threadId":"56782","inReplyTo":"cover-0.3-00000000000-20211025T211159Z-avarab@gmail.com","subject":"[PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-25T21:25:45Z","receivedAt":"2021-10-25T21:26:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a server-side implementation of a new \"bundle-uri\" command to\nprotocol v2. As discussed in the updated \"protocol-v2.txt\" this will\nallow conforming clients to optionally seed their initial clones or\nincremental fetches from URLs containing \"*.bundle\" files created with\n\"git bundle create\".\n\nThe use-cases are similar to those of the existing \"Packfile URIs\",\nand the two feature can be combined within a single request, but\n\"bundle-uri\" has a few advantages over packfile-uris in some some\ncommon scenarios, discussed below.\n\nThis change does not give us a working \"bundle-uri\" client. I have\nthose patches as a follow-up, but let's first establish what the\nprotocol for this should be like first. The client implementation will\nthen implement this specification.\n\nWith this change when the uploadpack.bundleURI config is set to a\nURI (or URIs, if set >1 times), advertise a \"bundle-uri\" command. Then\nwhen the client requests \"bundle-uri\" emit those URIs back at them.\n\nDifferences between this and the existing packfile-uri facility:\n\n A. There is no \"real\" support for packfile-uri in git.git. The\n uploadpack.blobPackfileUri setting allows carving out a list of\n blobs (actually any OIDs), but as alluded to in bfc2a36ff2a (Doc:\n clarify contents of packfile sent as URI, 2021-01-20) the only\n \"real\" implementation is JGit based.\n\n B. The uploadpack.blobPackfileUri is a MUST where this is a\n \"CAN\". I.e. once a client says they support packfile-uri of given\n list of protocols the server will send them a PACK response\n assuming they've downloaded the URI they client was sent, if the\n client doesn't do that they don't have a valid repository.\n\n Pointing at a bundle and having the client send us \"have\"\n lines (or not, maybe they couldn't fetch it, or decided they\n didn't want to) is more flexible, and can gracefully recover\n e.g. if the CDN isn't reachable (maybe you do support \"https\", but\n the CDN provider is down, or blocked your whole country).\n\n C. The client, after executing \"ls-refs\" will disconnect if it has\n also grabbed the \"bundle-uris\" and knows the server won't send it\n anything it doesn't already have (or expect to have, if it's\n downloading the bundles concurrent to an early disconnect).\n\n This is in (small) contrast to packfile-uri where a client would\n enter a negotiation dialog, which may or may not result in a\n packfile-uri and/or an inline PACK.\n\n D. Because of \"C\" clients can, if the bundles are up-to-date, get an\n up-to-date repository with just \"bundle-uri\" and \"ls-refs\" commands,\n with no need to enter a dialog with \"git upload-pack\".\n\n That small dialog is unlikely to matter for performance purposes,\n this section is just noting differences between \"bundle-uri\" and\n \"packfile-uri\".\n\nAs noted above the features are compatible, a client that supports\n\"bundle-uri\" and \"packfile-uri\" might download a bundle, and then\nproceed with a \"fetch\" dialog, that dialog might then result in\n\"packfile-uri\" response.\n\nIn practice server operators are unlikely to want to mix the two,\nsince the main benefit of either approach is the ability to offload\nlarge \"clone\" responses to CDNs. A server operator would have little\nreason not to go with one approach or the other.\n\nThere was a suggestion of implementing a similar feature long ago[1]\nby Jeff King. The main difference between it and this approach is that\nwe've since gained protocol v2, so we can add this as an optional path\nin the dialog between client and server. The 2011 implementation\nhooked into the transport mechanism to try to clone from a bundle\ndirectly. See also [2] and [3] for some later mentions of that\napproach.\n\nSee also [4] for the series that implemented\nuploadpack.blobPackfileUri, and [5] for a series on top that did the\n.gitmodules check in that context. See [6] for the \"ls-refs unborn\"\nfeature which modified code in similar areas of the request flow.\n\n1. https://lore.kernel.org/git/20111110074330.GA27925@sigill.intra.peff.net/\n2. https://lore.kernel.org/git/20190514092900.GA11679@sigill.intra.peff.net/\n3. https://lore.kernel.org/git/YFJWz5yIGng+a16k@coredump.intra.peff.net/\n4. https://lore.kernel.org/git/cover.1591821067.git.jonathantanmy@google.com/\n Merged as 34e849b05a4 (Merge branch 'jt/cdn-offload', 2020-06-25)\n5. https://lore.kernel.org/git/cover.1614021092.git.jonathantanmy@google.com/\n Merged as 6ee353d42f3 (Merge branch 'jt/transfer-fsck-across-packs',\n 2021-03-01)\n6. 69571dfe219 (Merge branch 'jt/clone-unborn-head', 2021-02-17)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/technical/protocol-v2.txt | 209 ++++++++++++++++++++++++\n Makefile | 1 +\n bundle-uri.c | 55 +++++++\n bundle-uri.h | 14 ++\n serve.c | 6 +\n t/t5701-git-serve.sh | 124 +++++++++++++-\n 6 files changed, 408 insertions(+), 1 deletion(-)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 21e8258ccf3..4bc15a976cd 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -566,3 +566,212 @@ and associated requested information, each separated by a single space.\n \tattr = \"size\"\n \n \tobj-info = obj-id SP obj-size\n+\n+bundle-uri\n+~~~~~~~~~~\n+\n+If the 'bundle-uri' capability is advertised, the server supports the\n+`bundle-uri' command.\n+\n+The capability is currently advertised with no value (i.e. not\n+\"bundle-uri=somevalue\"), a value may be added in the future for\n+supporting command-wide extensions. Clients MUST ignore any unknown\n+capability values and proceed with the 'bundle-uri` dialog they\n+support.\n+\n+The 'bundle-uri' command is intended to be issued before `fetch` to\n+get URIs to bundle files (see linkgit:git-bundle[1]) to \"seed\" and\n+inform the subsequent `fetch` command.\n+\n+The client CAN issue `bundle-uri` before or after any other valid\n+command. To be useful to clients it's expected that it'll be issued\n+after an `ls-refs` and before `fetch`, but CAN be issued at any time\n+in the dialog.\n+\n+DISCUSSION of bundle-uri\n+^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+The intent of the feature is optimize for server resource consumption\n+in the common case by changing the common case of fetching a very\n+large PACK during linkgit:git-clone[1] into a smaller incremental\n+fetch.\n+\n+It also allows servers to achieve better caching in combination with\n+an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n+\n+By having new clones or fetches be a more predictable and common\n+negotiation against the tips of recently produces *.bundle file(s).\n+Servers might even pre-generate the results of such negotiations for\n+the `uploadpack.packObjectsHook` as new pushes come in.\n+\n+I.e. the server would anticipate that fresh clones will download a\n+known bundle, followed by catching up to the current state of the\n+repository using ref tips found in that bundle (or bundles).\n+\n+PROTOCOL for bundle-uri\n+^^^^^^^^^^^^^^^^^^^^^^^\n+\n+A `bundle-uri` request takes no arguments, and as noted above does not\n+currently advertise a capability value. Both may be added in the\n+future.\n+\n+When the client issues a `command=bundle-uri` the response is a list\n+of URIs the server would like the client to fetch out-of-bounds before\n+proceeding with the `fetch` request in this format:\n+\n+\toutput = bundle-uri-line\n+\t\t bundle-uri-line* flush-pkt\n+\n+\tbundle-uri-line = PKT-LINE(bundle-uri)\n+\t\t\t *(SP bundle-feature-key *(=bundle-feature-val))\n+\t\t\t LF\n+\n+\tbundle-uri = A URI such as a https://, ssh:// etc. URI\n+\n+\tbundle-feature-key = Any printable ASCII characters except SP or \"=\"\n+\tbundle-feature-val = Any printable ASCII characters except SP or \"=\"\n+\n+No `bundle-feature-key`=`bundle-feature-value` fields are currently\n+defined. See the discussion of features below.\n+\n+Clients are still expected to fully parse the line according to the\n+above format, lines that do not conform to the format SHOULD be\n+discarded. The user MAY be warned in such a case.\n+\n+bundle-uri CLIENT AND SERVER EXPECTATIONS\n+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+\".bundle\" FORMAT\n+++++++++++++++++\n+\n+The advertised bundle(s) MUST be in a format that \"git bundle verify\"\n+would accept. I.e. they MUST contain one or more reference tips for\n+use by the client, MUST indicate prerequisites (in any) with standard\n+\"-\" prefixes, and MUST indicate their \"object-format\", if\n+applicable. Create \"*.bundle\" files with \"git bundle create\".\n+\n+bundle-uri CLIENT ERROR RECOVERY\n+++++++++++++++++++++++++++++++++\n+\n+A client MUST above all gracefully degrade on errors, whether that\n+error is because of bad missing/data in the bundle URI(s), because\n+that client is too dumb to e.g. understand and fully parse out bundle\n+headers and their prerequisite relationships, or something else.\n+\n+Server operators should feel confident in turning on \"bundle-uri\" and\n+not worry if e.g. their CDN goes down that clones or fetches will run\n+into hard failures. Even if the server bundle bundle(s) are\n+incomplete, or bad in some way the client should still end up with a\n+functioning repository, just as if it had chosen not to use this\n+protocol extension.\n+\n+All subsequent discussion on client and server interaction MUST keep\n+this in mind.\n+\n+bundle-uri SERVER TO CLIENT\n++++++++++++++++++++++++++++\n+\n+The ordering of the returned bundle uris is not significant. Clients\n+MUST parse their headers to discover their contained OIDS and\n+prerequisites. A client MUST consider the content of the bundle(s)\n+themselves and their header as the ultimate source of truth.\n+\n+A server MAY even return bundle(s) that don't have any direct\n+relationship to the repository being cloned (either through accident,\n+or intentional \"clever\" configuration), and expect a client to sort\n+out what data they'd like from the bundle(s), if any.\n+\n+bundle-uri CLIENT TO SERVER\n++++++++++++++++++++++++++++\n+\n+The client SHOULD provide reference tips found in the bundle header(s)\n+as 'have' lines in any subsequent `fetch` request. A client MAY also\n+ignore the bundle(s) entirely if doing so is deemed worse for some\n+reason, e.g. if the bundles can't be downloaded, it doesn't like the\n+tips it finds etc.\n+\n+WHEN ADVERTISED BUNDLE(S) REQUIRE NO FURTHER NEGOTIATION\n+++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n+\n+If after issuing `bundle-uri` and `ls-refs`, and getting the header(s)\n+of the bundle(s) the client finds that the ref tips it wants can be\n+retrieved entirety from advertised bundle(s), it MAY disconnect. The\n+results of such a 'clone' or 'fetch' should be indistinguishable from\n+the state attained without using bundle-uri.\n+\n+EARLY CLIENT DISCONNECTIONS AND ERROR RECOVERY\n+++++++++++++++++++++++++++++++++++++++++++++++\n+\n+A client MAY perform an early disconnect while still downloading the\n+bundle(s) (having streamed and parsed their headers). In such a case\n+the client MUST gracefully recover from any errors related to\n+finishing the download and validation of the bundle(s).\n+\n+I.e. a client might need to re-connect and issue a 'fetch' command,\n+and possibly fall back to not making use of 'bundle-uri' at all.\n+\n+This \"MAY\" behavior is specified as such (and not a \"SHOULD\") on the\n+assumption that a server advertising bundle uris is more likely than\n+not to be serving up a relatively large repository, and to be pointing\n+to URIs that have a good chance of being in working order. A client\n+MAY e.g. look at the payload size of the bundles as a heuristic to see\n+if an early disconnect is worth it, should falling back on a full\n+\"fetch\" dialog be necessary.\n+\n+WHEN ADVERTISED BUNDLE(S) REQUIRE FURTHER NEGOTIATION\n++++++++++++++++++++++++++++++++++++++++++++++++++++++\n+\n+A client SHOULD commence a negotiation of a PACK from the server via\n+the \"fetch\" command using the OID tips found in advertised bundles,\n+even if's still in the process of downloading those bundle(s).\n+\n+This allows for aggressive early disconnects from any interactive\n+server dialog. The client blindly trusts that the advertised OID tips\n+are relevant, and issues them as 'have' lines, it then requests any\n+tips it would like (usually from the \"ls-refs\" advertisement) via\n+'want' lines. The server will then compute a (hopefully small) PACK\n+with the expected difference between the tips from the bundle(s) and\n+the data requested.\n+\n+The only connection the client then needs to keep active is to the\n+concurrently downloading static bundle(s), when those and the\n+incremental PACK are retrieved they should be inflated and\n+validated. Any errors at this point should be gracefully recovered\n+from, see above.\n+\n+bundle-uri PROTOCOL FEATURES\n+^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+As noted above no `bundle-feature-key`=`bundle-feature-value` fields\n+are currently defined.\n+\n+They are intended for future per-URI metadata which older clients MUST\n+ignore and gracefully degrade on. Any fields they do recognize they\n+CAN also ignore.\n+\n+Any backwards-incompatible addition of pre-URI key-value will be\n+guarded by a new value or values in 'bundle-uri' capability\n+advertisement itself, and/or by new future `bundle-uri` request\n+arguments.\n+\n+While no per-URI key-value are currently supported currently they're\n+intended to support future features such as:\n+\n+ * Add a \"hash=\" or \"size=\" advertise the expected hash or\n+ size of the bundle file.\n+\n+ * Advertise that one or more bundle files are the same (to e.g. have\n+ clients round-robin or otherwise choose one of N possible files).\n+\n+ * A \"oid=\" shortcut and \"prerequisite=\" shortcut. For\n+ expressing the common case of a bundle with one tip and no\n+ prerequisites, or one tip and one prerequisite.\n++\n+This would allow for optimizing the common case of servers who'd like\n+to provide one \"big bundle\" containing only their \"main\" branch,\n+and/or incremental updates thereof.\n++\n+A client receiving such a a response MAY assume that they can skip\n+retrieving the header from a bundle at the indicated URI, and thus\n+save themselves and the server(s) the request(s) needed to inspect the\n+headers of that bundle or bundles.\ndiff --git a/Makefile b/Makefile\nindex 381bed2c1d2..e41ac60829d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -846,6 +846,7 @@ LIB_OBJS += blob.o\n LIB_OBJS += bloom.o\n LIB_OBJS += branch.o\n LIB_OBJS += bulk-checkin.o\n+LIB_OBJS += bundle-uri.o\n LIB_OBJS += bundle.o\n LIB_OBJS += cache-tree.o\n LIB_OBJS += cbtree.o\ndiff --git a/bundle-uri.c b/bundle-uri.c\nnew file mode 100644\nindex 00000000000..ff054ddc690\n--- /dev/null\n+++ b/bundle-uri.c\n@@ -0,0 +1,55 @@\n+#include \"cache.h\"\n+#include \"bundle-uri.h\"\n+#include \"pkt-line.h\"\n+#include \"config.h\"\n+\n+static void send_bundle_uris(struct packet_writer *writer,\n+\t\t\t struct string_list *uris)\n+{\n+\tstruct string_list_item *item;\n+\n+\tfor_each_string_list_item(item, uris)\n+\t\tpacket_writer_write(writer, \"%s\", item->string);\n+}\n+\n+static int advertise_bundle_uri = -1;\n+static struct string_list bundle_uris = STRING_LIST_INIT_DUP;\n+static int bundle_uri_config(const char *var, const char *value, void *data)\n+{\n+\tif (!strcmp(var, \"uploadpack.bundleuri\")) {\n+\t\tadvertise_bundle_uri = 1;\n+\t\tstring_list_append(&bundle_uris, value);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+int bundle_uri_advertise(struct repository *r, struct strbuf *value)\n+{\n+\tif (advertise_bundle_uri != -1)\n+\t\tgoto cached;\n+\n+\tgit_config(bundle_uri_config, NULL);\n+\tadvertise_bundle_uri = !!bundle_uris.nr;\n+\n+cached:\n+\treturn advertise_bundle_uri;\n+}\n+\n+int bundle_uri_command(struct repository *r,\n+\t\t struct packet_reader *request)\n+{\n+\tstruct packet_writer writer;\n+\tpacket_writer_init(&writer, 1);\n+\n+\twhile (packet_reader_read(request) == PACKET_READ_NORMAL)\n+\t\tdie(_(\"bundle-uri: unexpected argument: '%s'\"), request->line);\n+\tif (request->status != PACKET_READ_FLUSH)\n+\t\tdie(_(\"bundle-uri: expected flush after arguments\"));\n+\n+\tsend_bundle_uris(&writer, &bundle_uris);\n+\n+\tpacket_writer_flush(&writer);\n+\n+\treturn 0;\n+}\ndiff --git a/bundle-uri.h b/bundle-uri.h\nnew file mode 100644\nindex 00000000000..b8762e6a8e4\n--- /dev/null\n+++ b/bundle-uri.h\n@@ -0,0 +1,14 @@\n+#ifndef BUNDLE_URI_H\n+#define BUNDLE_URI_H\n+\n+struct repository;\n+struct packet_reader;\n+struct packet_writer;\n+\n+/**\n+ * API used by serve.[ch].\n+ */\n+int bundle_uri_advertise(struct repository *r, struct strbuf *value);\n+int bundle_uri_command(struct repository *r, struct packet_reader *request);\n+\n+#endif /* BUNDLE_URI_H */\ndiff --git a/serve.c b/serve.c\nindex b3fe9b5126a..f3e0203d2c6 100644\n--- a/serve.c\n+++ b/serve.c\n@@ -8,6 +8,7 @@\n #include \"protocol-caps.h\"\n #include \"serve.h\"\n #include \"upload-pack.h\"\n+#include \"bundle-uri.h\"\n \n static int advertise_sid = -1;\n static int client_hash_algo = GIT_HASH_SHA1;\n@@ -136,6 +137,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 = \"bundle-uri\",\n+\t\t.advertise = bundle_uri_advertise,\n+\t\t.command = bundle_uri_command,\n+\t},\n };\n \n void protocol_v2_advertise_capabilities(void)\ndiff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\nindex 1896f671cb3..9d053f77a93 100755\n--- a/t/t5701-git-serve.sh\n+++ b/t/t5701-git-serve.sh\n@@ -13,7 +13,7 @@ test_expect_success 'test capability advertisement' '\n \twrong_algo sha1:sha256\n \twrong_algo sha256:sha1\n \tEOF\n-\tcat >expect <<-EOF &&\n+\tcat >expect.base <<-EOF &&\n \tversion 2\n \tagent=git/$(git version | cut -d\" \" -f3)\n \tls-refs=unborn\n@@ -21,8 +21,11 @@ test_expect_success 'test capability advertisement' '\n \tserver-option\n \tobject-format=$(test_oid algo)\n \tobject-info\n+\tEOF\n+\tcat >expect.trailer <<-EOF &&\n \t0000\n \tEOF\n+\tcat expect.base expect.trailer >expect &&\n \n \tGIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \\\n \t\t--advertise-capabilities >out &&\n@@ -342,4 +345,123 @@ test_expect_success 'basics of object-info' '\n \ttest_cmp expect actual\n '\n \n+# Test the basics of bundle-uri\n+#\n+test_expect_success 'test capability advertisement with uploadpack.bundleURI' '\n+\ttest_config uploadpack.bundleURI FAKE &&\n+\n+\tcat >expect.extra <<-EOF &&\n+\tbundle-uri\n+\tEOF\n+\tcat expect.base \\\n+\t expect.extra \\\n+\t expect.trailer >expect &&\n+\n+\tGIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \\\n+\t\t--advertise-capabilities >out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: dies if not enabled' '\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\t0000\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\tfatal: invalid command '\"'\"'bundle-uri'\"'\"'\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\tERR serve: invalid command '\"'\"'bundle-uri'\"'\"'\n+\tEOF\n+\n+\ttest_must_fail test-tool serve-v2 --stateless-rpc out 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_must_be_empty out\n+'\n+\n+\n+test_expect_success 'basics of bundle-uri: enabled with single URI' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/repo.bdl &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0000\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\thttps://cdn.example.com/repo.bdl\n+\t0000\n+\tEOF\n+\n+\ttest-tool serve-v2 --stateless-rpc out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: enabled with single URI' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/repo.bdl &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0000\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\thttps://cdn.example.com/repo.bdl\n+\t0000\n+\tEOF\n+\n+\ttest-tool serve-v2 --stateless-rpc out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: enabled with two URIs' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/repo.bdl &&\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/recent.bdl --add &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0000\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\thttps://cdn.example.com/repo.bdl\n+\thttps://cdn.example.com/recent.bdl\n+\t0000\n+\tEOF\n+\n+\ttest-tool serve-v2 --stateless-rpc out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: unknown future feature(s)' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/fake.bdl &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0001\n+\tsome-feature\n+\twe-do-not\n+\tknow=about\n+\t0000\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\tfatal: bundle-uri: unexpected argument: '\"'\"'some-feature'\"'\"'\n+\tEOF\n+\n+\ttest_must_fail test-tool serve-v2 --stateless-rpc out 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_must_be_empty out\n+'\n+\n test_done\n-- \n2.33.1.1511.gd15d1b313a6\n\n"},{"id":"439596","messageId":"patch-3.3-64224ec2cba-20211025T211159Z-avarab@gmail.com","threadId":"56782","inReplyTo":"cover-0.3-00000000000-20211025T211159Z-avarab@gmail.com","subject":"[PATCH 3/3] bundle-uri client: add \"bundle-uri\" parsing + tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-25T21:25:46Z","receivedAt":"2021-10-25T21:26:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a \"test-tool bundle-uri parse\" which parses the format defined in\nthe newly specified \"bundle-uri\" command.\n\nAs note in the \"bundle-uri\" section in protocol-v2.txt we haven't\nspecified any key-values yet, just URI lines, but we should parse\ntheir format for conformity with the spec.\n\nWe need to make sure our future client doesn't die if this optional\ndata is ever provided by the server, and that we've covered all the\nedge cases with these key-values in our specification. Let's add and\ntest a bundle_uri_parse_line() to do that.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n bundle-uri.c | 124 +++++++++++++++++++++++++++++\n bundle-uri.h | 16 ++++\n t/helper/test-bundle-uri.c | 83 +++++++++++++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t5750-bundle-uri-parse.sh | 153 ++++++++++++++++++++++++++++++++++++\n 7 files changed, 379 insertions(+)\n create mode 100644 t/helper/test-bundle-uri.c\n create mode 100755 t/t5750-bundle-uri-parse.sh\n\ndiff --git a/Makefile b/Makefile\nindex e41ac60829d..de66a016c78 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -691,6 +691,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n TEST_BUILTINS_OBJS += test-advise.o\n TEST_BUILTINS_OBJS += test-bitmap.o\n TEST_BUILTINS_OBJS += test-bloom.o\n+TEST_BUILTINS_OBJS += test-bundle-uri.o\n TEST_BUILTINS_OBJS += test-chmtime.o\n TEST_BUILTINS_OBJS += test-config.o\n TEST_BUILTINS_OBJS += test-crontab.o\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex ff054ddc690..9827fc5da17 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -53,3 +53,127 @@ int bundle_uri_command(struct repository *r,\n \n \treturn 0;\n }\n+\n+/**\n+ * General API for {transport,connect}.c etc.\n+ */\n+int bundle_uri_parse_line(struct string_list *bundle_uri, const char *line)\n+{\n+\tsize_t i;\n+\tstruct string_list columns = STRING_LIST_INIT_DUP;\n+\tconst char *uri;\n+\tstruct string_list *uri_columns = NULL;\n+\tint ret = 0;\n+\n+\tif (!strlen(line))\n+\t\treturn error(_(\"bundle-uri: got an empty line\"));\n+\n+\t/*\n+\t * Right now we don't understand anything beyond the first SP,\n+\t * but let's be tolerant and ignore any future unknown\n+\t * fields. See the \"MUST\" note about \"bundle-feature-key\" in\n+\t * Documentation/technical/protocol-v2.txt\n+\t */\n+\tif (string_list_split(&columns, line, ' ', -1) < 1)\n+\t\treturn error(_(\"bundle-uri: line not in SP-delimited format: %s\"), line);\n+\n+\t/*\n+\t * We represent a \"[ ...]\" line with the URI\n+\t * being the .string in a string list, and the .util being an\n+\t * optional string list of key (.string) and values\n+\t * (.util). If the top-level .util is NULL there's no\n+\t * key-value pairs....\n+\t */\n+\turi = columns.items[0].string;\n+\tif (!strlen(uri)) {\n+\t\tret = error(_(\"bundle-uri: got an empty URI component\"));\n+\t\tgoto cleanup;\n+\t}\n+\n+\t/*\n+\t * ... we're going to need that non-NULL .util .\n+\t */\n+\tif (columns.nr > 1) {\n+\t\turi_columns = xcalloc(1, sizeof(struct string_list));\n+\t\tstring_list_init_dup(uri_columns);\n+\t}\n+\n+\t/*\n+\t * Let's parse the optional \"kv\" format, even if we don't\n+\t * understand any of the keys or values yet.\n+\t */\n+\tfor (i = 1; i < columns.nr; i++) {\n+\t\tstruct string_list kv = STRING_LIST_INIT_DUP;\n+\t\tconst char *arg = columns.items[i].string;\n+\t\tint fields = string_list_split(&kv, arg, '=', 2);\n+\t\tint err = 0;\n+\n+\t\tswitch (fields) {\n+\t\tcase 0:\n+\t\t\tBUG(\"should have no fields=0\");\n+\t\tcase 1:\n+\t\t\tif (!strlen(arg)) {\n+\t\t\t\terr = error(\"bundle-uri: column %lu: got an empty attribute (full line was '%s')\",\n+\t\t\t\t\t i, line);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\t/*\n+\t\t\t * We could dance around with\n+\t\t\t * string_list_append_nodup() and skip\n+\t\t\t * string_list_clear(&kv, 0) here, but let's\n+\t\t\t * keep it simple.\n+\t\t\t */\n+\t\t\tstring_list_append(uri_columns, arg);\n+\t\t\tbreak;\n+\t\tcase 2:\n+\t\t{\n+\t\t\tconst char *k = kv.items[0].string;\n+\t\t\tconst char *v = kv.items[1].string;\n+\n+\t\t\tstring_list_append(uri_columns, k)->util = xstrdup(v);\n+\t\t\tbreak;\n+\t\t}\n+\t\tdefault:\n+\t\t\terr = error(\"bundle-uri: column %lu: '%s' more than one '=' character (full line was '%s')\",\n+\t\t\t\t i, arg, line);\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tstring_list_clear(&kv, 0);\n+\t\tif (err) {\n+\t\t\tret = err;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\n+\t/*\n+\t * Per the spec we'll only consider bundle-uri lines OK if\n+\t * there were no parsing problems, even if the problems were\n+\t * with attributes whose content we don't understand.\n+\t */\n+\tif (ret && uri_columns) {\n+\t\tstring_list_clear(uri_columns, 1);\n+\t\tfree(uri_columns);\n+\t} else if (!ret) {\n+\t\tstring_list_append(bundle_uri, uri)->util = uri_columns;\n+\t}\n+\n+cleanup:\n+\tstring_list_clear(&columns, 0);\n+\treturn ret;\n+}\n+\n+static void bundle_uri_string_list_clear_cb(void *util, const char *string)\n+{\n+\tstruct string_list *fields = util;\n+\tif (!fields)\n+\t\treturn;\n+\tstring_list_clear(fields, 1);\n+\tfree(fields);\n+}\n+\n+void bundle_uri_string_list_clear(struct string_list *bundle_uri)\n+{\n+\tstring_list_clear_func(bundle_uri, bundle_uri_string_list_clear_cb);\n+}\ndiff --git a/bundle-uri.h b/bundle-uri.h\nindex b8762e6a8e4..c23d7316555 100644\n--- a/bundle-uri.h\n+++ b/bundle-uri.h\n@@ -4,6 +4,7 @@\n struct repository;\n struct packet_reader;\n struct packet_writer;\n+struct string_list;\n \n /**\n * API used by serve.[ch].\n@@ -11,4 +12,19 @@ struct packet_writer;\n int bundle_uri_advertise(struct repository *r, struct strbuf *value);\n int bundle_uri_command(struct repository *r, struct packet_reader *request);\n \n+/**\n+ * General API for {transport,connect}.c etc.\n+ */\n+\n+/**\n+ * bundle_uri_parse_line() returns 0 when a valid bundle-uri has been\n+ * added to `bundle_uri`, <0 on error.\n+ */\n+int bundle_uri_parse_line(struct string_list *bundle_uri, const char *line);\n+\n+/**\n+ * Clear the `bundle_uri` list. Just a very thin wrapper on\n+ * string_list_clear().\n+ */\n+void bundle_uri_string_list_clear(struct string_list *bundle_uri);\n #endif /* BUNDLE_URI_H */\ndiff --git a/t/helper/test-bundle-uri.c b/t/helper/test-bundle-uri.c\nnew file mode 100644\nindex 00000000000..805a86c0130\n--- /dev/null\n+++ b/t/helper/test-bundle-uri.c\n@@ -0,0 +1,83 @@\n+#include \"test-tool.h\"\n+#include \"parse-options.h\"\n+#include \"bundle-uri.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+\n+static int cmd__bundle_uri_parse(int argc, const char **argv)\n+{\n+\tconst char *usage[] = {\n+\t\t\"test-tool bundle-uri parse util;\n+\n+\t\tfprintf(stdout, \"%s\", item->string);\n+\t\tif (!kv) {\n+\t\t\tfprintf(stdout, \"\\n\");\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfor_each_string_list_item(kv_item, kv) {\n+\t\t\tconst char *k = kv_item->string;\n+\t\t\tconst char *v = kv_item->util;\n+\n+\t\t\tif (v)\n+\t\t\t\tfprintf(stdout, \" [kv: %s => %s]\", k, v);\n+\t\t\telse\n+\t\t\t\tfprintf(stdout, \" [attr: %s]\", k);\n+\t\t}\n+\t\tfprintf(stdout, \"\\n\");\n+\t}\n+\tstrbuf_release(&sb);\n+\n+\tbundle_uri_string_list_clear(&list);\n+\n+\treturn err < 0 ? 1 : 0;\n+usage:\n+\tusage_with_options(usage, options);\n+}\n+\n+int cmd__bundle_uri(int argc, const char **argv)\n+{\n+\tconst char *usage[] = {\n+\t\t\"test-tool bundle-uri []\",\n+\t\tNULL\n+\t};\n+\tstruct option options[] = {\n+\t\tOPT_END(),\n+\t};\n+\n+\targc = parse_options(argc, argv, NULL, options, usage,\n+\t\t\t PARSE_OPT_STOP_AT_NON_OPTION |\n+\t\t\t PARSE_OPT_KEEP_ARGV0);\n+\tif (argc == 1)\n+\t\tgoto usage;\n+\n+\tif (!strcmp(argv[1], \"parse\"))\n+\t\treturn cmd__bundle_uri_parse(argc - 1, argv + 1);\n+\terror(\"there is no test-tool bundle-uri tool '%s'\", argv[1]);\n+\n+usage:\n+\tusage_with_options(usage, options);\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 3ce5585e53a..b6e1ee7b253 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -17,6 +17,7 @@ static struct test_cmd cmds[] = {\n \t{ \"advise\", cmd__advise_if_enabled },\n \t{ \"bitmap\", cmd__bitmap },\n \t{ \"bloom\", cmd__bloom },\n+\t{ \"bundle-uri\", cmd__bundle_uri },\n \t{ \"chmtime\", cmd__chmtime },\n \t{ \"config\", cmd__config },\n \t{ \"crontab\", cmd__crontab },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 9f0f5228508..ef839ac7262 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -7,6 +7,7 @@\n int cmd__advise_if_enabled(int argc, const char **argv);\n int cmd__bitmap(int argc, const char **argv);\n int cmd__bloom(int argc, const char **argv);\n+int cmd__bundle_uri(int argc, const char **argv);\n int cmd__chmtime(int argc, const char **argv);\n int cmd__config(int argc, const char **argv);\n int cmd__crontab(int argc, const char **argv);\ndiff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh\nnew file mode 100755\nindex 00000000000..70fd1b398e9\n--- /dev/null\n+++ b/t/t5750-bundle-uri-parse.sh\n@@ -0,0 +1,153 @@\n+#!/bin/sh\n+\n+test_description=\"Test bundle-uri bundle_uri_parse_line()\"\n+\n+TEST_NO_CREATE_REPO=1\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+\n+test_expect_success 'bundle_uri_parse_line() just URIs' '\n+\tcat >in <<-\\EOF &&\n+\thttp://example.com/bundle.bdl\n+\thttps://example.com/bundle.bdl\n+\tfile:///usr/share/git/bundle.bdl\n+\tEOF\n+\n+\t# For the simple case\n+\tcp in expect &&\n+\n+\ttest-tool bundle-uri parse actual 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bundle_uri_parse_line() with attributes' '\n+\tcat >in <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl attr\n+\thttp://example.com/bundle2.bdl ibute\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl [attr: attr]\n+\thttp://example.com/bundle2.bdl [attr: ibute]\n+\tEOF\n+\n+\ttest-tool bundle-uri parse actual 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bundle_uri_parse_line() with attributes and key-value attributes' '\n+\tcat >in <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl x a=b y c=d z e=f a=b\n+\tEOF\n+\n+\n+\tcat >expect <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl [attr: x] [kv: a => b] [attr: y] [kv: c => d] [attr: z] [kv: e => f] [kv: a => b]\n+\tEOF\n+\n+\ttest-tool bundle-uri parse actual 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bundle_uri_parse_line() parsing edge cases: extra SP' '\n+\tcat >in <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl one-space\n+\thttp://example.com/bundle2.bdl two-space\n+\thttp://example.com/bundle3.bdl three-space\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\terror: bundle-uri: column 1: got an empty attribute (full line was '\\''http://example.com/bundle2.bdl two-space'\\'')\n+\terror: bad line: '\\''http://example.com/bundle2.bdl two-space'\\''\n+\terror: bundle-uri: column 1: got an empty attribute (full line was '\\''http://example.com/bundle3.bdl three-space'\\'')\n+\terror: bad line: '\\''http://example.com/bundle3.bdl three-space'\\''\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl [attr: one-space]\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse actual 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bundle_uri_parse_line() parsing edge cases: empty lines' '\n+\tcat >in <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl\n+\n+\thttp://example.com/bundle2.bdl a=b\n+\n+\thttp://example.com/bundle3.bdl\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\terror: bundle-uri: got an empty line\n+\terror: bad line: '\\'''\\''\n+\terror: bundle-uri: got an empty line\n+\terror: bad line: '\\'''\\''\n+\tEOF\n+\n+\t# We fail, but try to continue parsing regardless\n+\tcat >expect <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl\n+\thttp://example.com/bundle2.bdl [kv: a => b]\n+\thttp://example.com/bundle3.bdl\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse actual 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bundle_uri_parse_line() parsing edge cases: empty URIs' '\n+\tsed \"s/> //\" >in <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl\n+\t> a=b\n+\thttp://example.com/bundle3.bdl a=b\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\terror: bundle-uri: got an empty URI component\n+\terror: bad line: '\\'' a=b'\\''\n+\tEOF\n+\n+\t# We fail, but try to continue parsing regardless\n+\tcat >expect <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl\n+\thttp://example.com/bundle3.bdl [kv: a => b]\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse actual 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bundle_uri_parse_line() parsing edge cases: multiple = in key-values' '\n+\tcat >in <<-\\EOF &&\n+\thttp://example.com/bundle1.bdl k=v=extra\n+\thttp://example.com/bundle2.bdl a=b k=v=extra c=d\n+\thttp://example.com/bundle3.bdl kv=ok\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\terror: bundle-uri: column 1: '\\''k=v=extra'\\'' more than one '\\''='\\'' character (full line was '\\''http://example.com/bundle1.bdl k=v=extra'\\'')\n+\terror: bad line: '\\''http://example.com/bundle1.bdl k=v=extra'\\''\n+\terror: bundle-uri: column 2: '\\''k=v=extra'\\'' more than one '\\''='\\'' character (full line was '\\''http://example.com/bundle2.bdl a=b k=v=extra c=d'\\'')\n+\terror: bad line: '\\''http://example.com/bundle2.bdl a=b k=v=extra c=d'\\''\n+\tEOF\n+\n+\t# We fail, but try to continue parsing regardless\n+\tcat >expect <<-\\EOF &&\n+\thttp://example.com/bundle3.bdl [kv: kv => ok]\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse actual 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.33.1.1511.gd15d1b313a6\n\n"},{"id":"439645","messageId":"f2269fc7-1688-d62e-02bb-01c5b5e33143@gmail.com","threadId":"56782","inReplyTo":"patch-2.3-3ac0539c053-20211025T211159Z-avarab@gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-26T14:00:10Z","receivedAt":"2021-10-26T14:00:17Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n> diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\n> +bundle-uri CLIENT ERROR RECOVERY\n> +++++++++++++++++++++++++++++++++\n> +\n> +A client MUST above all gracefully degrade on errors, whether that\n> +error is because of bad missing/data in the bundle URI(s), because\n> +that client is too dumb to e.g. understand and fully parse out bundle\n> +headers and their prerequisite relationships, or something else.\n\n\"too dumb\" seems a bit informal to me, especially because you\nimmediately elaborate on its meaning. You could rewrite as follows:\n\n ...because\n that client can't understand or fully parse out bundle\n headers and their prerequisite relationships, or something else.\n\n> +Server operators should feel confident in turning on \"bundle-uri\" and\n> +not worry if e.g. their CDN goes down that clones or fetches will run\n> +into hard failures. Even if the server bundle bundle(s) are\n> +incomplete, or bad in some way the client should still end up with a\n> +functioning repository, just as if it had chosen not to use this\n> +protocol extension.\n\nAlso, insertions of \"e.g.\" in the middle of a sentence don't flow well.\n\n Server operators should feel confident in turning on \"bundle-uri\" and\n not worry that failures such as the CDN being unavailable will cause\n clones or fetches to have hard failures. Even if the server bundle(s)\n are invalid, the client should still end up with a functioning\n repository, just as if it had chosen not to use this protocol extension.\n\n(Note: I also removed a \"bundle bundle(s)\" that was split across a line\nbreak.)\n\n> +bundle-uri SERVER TO CLIENT\n> ++++++++++++++++++++++++++++\n> +\n> +The ordering of the returned bundle uris is not significant. Clients\n\nI'm late to noticing, but shouldn't \"URI\" be all-caps when not used in\nthe literal capability string \"bundle-uri\"?\n\n> +bundle-uri CLIENT TO SERVER\n> ++++++++++++++++++++++++++++\n> +\n> +The client SHOULD provide reference tips found in the bundle header(s)\n> +as 'have' lines in any subsequent `fetch` request. A client MAY also\n> +ignore the bundle(s) entirely if doing so is deemed worse for some\n> +reason, e.g. if the bundles can't be downloaded, it doesn't like the\n> +tips it finds etc.\n\nI would just stop after \"is deemed worse for some reason.\" because one\nexample is obvious and the other is unclear how the client would detect\nthat situation. (Maybe: tip commit timestamps are really old?)\n\n> +\n> +WHEN ADVERTISED BUNDLE(S) REQUIRE NO FURTHER NEGOTIATION\n> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> +\n> +If after issuing `bundle-uri` and `ls-refs`, and getting the header(s)\n> +of the bundle(s) the client finds that the ref tips it wants can be\n> +retrieved entirety from advertised bundle(s), it MAY disconnect. The\n\ns/entirety/entirely/\n\n> +results of such a 'clone' or 'fetch' should be indistinguishable from\n> +the state attained without using bundle-uri.\n> +\n> +EARLY CLIENT DISCONNECTIONS AND ERROR RECOVERY\n> +++++++++++++++++++++++++++++++++++++++++++++++\n> +\n> +A client MAY perform an early disconnect while still downloading the\n> +bundle(s) (having streamed and parsed their headers). In such a case\n> +the client MUST gracefully recover from any errors related to\n> +finishing the download and validation of the bundle(s).\n> +\n> +I.e. a client might need to re-connect and issue a 'fetch' command,\n> +and possibly fall back to not making use of 'bundle-uri' at all.\n\nUse \"For example,\" over starting a sentence with \"i.e.\". The examples\nof \"i.e.\" and \"e.g.\" already in this document show proper use, which\ninvolves parentheses.\n\n> +This \"MAY\" behavior is specified as such (and not a \"SHOULD\") on the\n> +assumption that a server advertising bundle uris is more likely than\n> +not to be serving up a relatively large repository, and to be pointing\n> +to URIs that have a good chance of being in working order. A client\n> +MAY e.g. look at the payload size of the bundles as a heuristic to see\n\nAgain, here, the entire sentence is an example. This \"e.g.\" can be\nremoved with no loss of meaning.\n\n> +if an early disconnect is worth it, should falling back on a full\n> +\"fetch\" dialog be necessary.\n\n\n> +While no per-URI key-value are currently supported currently they're\n> +intended to support future features such as:\n> +\n> + * Add a \"hash=\" or \"size=\" advertise the expected hash or\n> + size of the bundle file.\n\nI suppose if one wanted to add this server-to-bundle coupling, then some\nclients might appreciate it.\n\n> + * Advertise that one or more bundle files are the same (to e.g. have\n> + clients round-robin or otherwise choose one of N possible files).\n\n * Advertise that one or more bundle files are the same, to allow for\n redundancy without causing duplicated effort.\n\n> +static void send_bundle_uris(struct packet_writer *writer,\n> +\t\t\t struct string_list *uris)\n> +{\n> +\tstruct string_list_item *item;\n> +\n> +\tfor_each_string_list_item(item, uris)\n> +\t\tpacket_writer_write(writer, \"%s\", item->string);\n> +}\n> +\n> +static int advertise_bundle_uri = -1;\n> +static struct string_list bundle_uris = STRING_LIST_INIT_DUP;\n\nI see you put send_bundle_uris() before the global bundle_uris so\nit can be independent, but do you expect anyone to call send_bundle_uris()\nvia a different list?\n\nShould we find a different place to store this data?\n\n> +static int bundle_uri_config(const char *var, const char *value, void *data)\n> +{\n> +\tif (!strcmp(var, \"uploadpack.bundleuri\")) {\n> +\t\tadvertise_bundle_uri = 1;\n> +\t\tstring_list_append(&bundle_uris, value);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n\nHere, we are dictating that the URI list is available as a multi-valued\nconfig \"uploadpack.bundleuri\".\n\n1. Should this be updated in Documentation/config/uploadpack.txt?\n\n2. This seems difficult to extend to your possible future features as\n listed in the protocol docs, mainly because this can only store the\n flat URI string. To add things like hash values, sizes, and prereqs,\n you would need more data included and grouped on a per-URI basis.\n What plans do you have to make extensions here while remaining\n somewhat compatible with downgrading Git versions?\n\n> @@ -136,6 +137,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 = \"bundle-uri\",\n> +\t\t.advertise = bundle_uri_advertise,\n> +\t\t.command = bundle_uri_command,\n> +\t},\n> };\n\nI really appreciate that it is this simple to extend protocol v2.\n\n> +test_expect_success 'basics of bundle-uri: dies if not enabled' '\n> +\ttest-tool pkt-line pack >in <<-EOF &&\n> +\tcommand=bundle-uri\n> +\t0000\n> +\tEOF\n> +\n> +\tcat >err.expect <<-\\EOF &&\n> +\tfatal: invalid command '\"'\"'bundle-uri'\"'\"'\n> +\tEOF\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\tERR serve: invalid command '\"'\"'bundle-uri'\"'\"'\n> +\tEOF\n> +\n> +\ttest_must_fail test-tool serve-v2 --stateless-rpc out 2>err.actual &&\n> +\ttest_cmp err.expect err.actual &&\n> +\ttest_must_be_empty out\n> +'\n> +\n> +\n\nhyper-nit: double newline.\n\nThe implementation seems simple enough, which I like. I'm a bit\nyour current use of Git config as the back-end, only because it is\ndifficult to be future-proof. As the functionality stands today, the\ncurrent config design works just fine. Perhaps we don't need to\nworry about the future, because we can design a new, complementary\nstorage for that extra data. It seems worth exploring for a little\nwhile, though. Perhaps we should take a page out of 'git-remote'\nand how it stores named remotes with sub-items for metadata. The\nnames probably don't need to ever be exposed to users, but it could\nbe beneficial to anyone implementing this scheme.\n\n[bundle \"main\"]\n\turi = https://example.com/my-bundle\n\turi = https://redundant-cdn.com/my-bundle\n\tsize = 120523\n\tsha256 = {64hexchars}\n\n[bundle \"fork\"]\n\turi = https://cdn.org/my-fork\n\tsize = 334\n\tsha256 = {...}\n\tprereq = {oid}\n\nThis kind of layout has an immediate grouping of data that should\nhelp any future plan. Notice how I included multiple \"uri\" lines\nin the \"main\", which helps with your plan for duplicate URIs.\n\nThanks,\n-Stolee\n"},{"id":"439646","messageId":"3de22cec-55db-6ff3-c009-a150647fef38@gmail.com","threadId":"56782","inReplyTo":"patch-3.3-64224ec2cba-20211025T211159Z-avarab@gmail.com","subject":"Re: [PATCH 3/3] bundle-uri client: add \"bundle-uri\" parsing + tests","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-26T14:05:30Z","receivedAt":"2021-10-26T14:05:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n> Add a \"test-tool bundle-uri parse\" which parses the format defined in\n> the newly specified \"bundle-uri\" command.\n> \n> As note in the \"bundle-uri\" section in protocol-v2.txt we haven't\n> specified any key-values yet, just URI lines, but we should parse\n> their format for conformity with the spec.\n> \n> We need to make sure our future client doesn't die if this optional\n> data is ever provided by the server, and that we've covered all the\n> edge cases with these key-values in our specification. Let's add and\n> test a bundle_uri_parse_line() to do that.\n\nWhile this implementation is interesting, and the tests available are\nuseful for validating the protocol, I would like to see how this\nintegrates into a full 'git clone' operation to be sure that the API\nis correct. How much more work is it to implement the full end-to-end\nscenario?\n\nThanks,\n-Stolee\n"},{"id":"439651","messageId":"211026.86cznrzu8g.gmgdl@evledraar.gmail.com","threadId":"56782","inReplyTo":"f2269fc7-1688-d62e-02bb-01c5b5e33143@gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-26T15:00:08Z","receivedAt":"2021-10-26T15:22:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 26 2021, Derrick Stolee wrote:\n\n> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>> diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\n>> +bundle-uri CLIENT ERROR RECOVERY\n>> +++++++++++++++++++++++++++++++++\n>> +\n>> +A client MUST above all gracefully degrade on errors, whether that\n>> +error is because of bad missing/data in the bundle URI(s), because\n>> +that client is too dumb to e.g. understand and fully parse out bundle\n>> +headers and their prerequisite relationships, or something else.\n>\n> \"too dumb\" seems a bit informal to me, especially because you\n> immediately elaborate on its meaning. You could rewrite as follows:\n>\n> ...because\n> that client can't understand or fully parse out bundle\n> headers and their prerequisite relationships, or something else.\n\nThanks, I've snipped all your subsequent comments on\nphrasing/clarifications etc, except insofar as they have questions I\nneed to address (as opposed to just my bad grammar/phrasing etc).\n\nThanks a lot for them, will go through them closely for any subsequent\nre-roll & address them.\n\n> [...]\n>> +While no per-URI key-value are currently supported currently they're\n>> +intended to support future features such as:\n>> +\n>> + * Add a \"hash=\" or \"size=\" advertise the expected hash or\n>> + size of the bundle file.\n>\n> I suppose if one wanted to add this server-to-bundle coupling, then some\n> clients might appreciate it.\n\nFor packfile-uri there's a hard dependency on the server transferring\nthe hash of the PACK file.\n\nI've intentionally omitted it, the reasons are covered in [1], which I\nrealize now should really be part of this early series.\n\nBasically having it as a hard requirement isn't necessary for security\nor payload validation. Any server who's worried about their transport\nintegrity would point to a https URI under their control, any\nchecksumming and validation we'll need we'll get from the transport\nlayer and the client's reachability check.\n\nHaving it would mean that you need closer cooperation by default between\nserver and CDN than I'm aiming for, i.e. a server should be able to\npoint to some URI somewhere updated by a dumb hourly cronjob, without\nneeding to pass information back & forth about what the \"current\" URL\nis. The client will discover all that.\n\nBut I left that \"hash=*\" in because it could be optionally added, in\ncase someone really wants it for some reason...\n\n1. https://lore.kernel.org/git/RFC-patch-13.13-1e657ed27a-20210805T150534Z-avarab@gmail.com/\n\n>> + * Advertise that one or more bundle files are the same (to e.g. have\n>> + clients round-robin or otherwise choose one of N possible files).\n>\n> * Advertise that one or more bundle files are the same, to allow for\n> redundancy without causing duplicated effort.\n\n*nod*\n\n>> +static void send_bundle_uris(struct packet_writer *writer,\n>> +\t\t\t struct string_list *uris)\n>> +{\n>> +\tstruct string_list_item *item;\n>> +\n>> +\tfor_each_string_list_item(item, uris)\n>> +\t\tpacket_writer_write(writer, \"%s\", item->string);\n>> +}\n>> +\n>> +static int advertise_bundle_uri = -1;\n>> +static struct string_list bundle_uris = STRING_LIST_INIT_DUP;\n>\n> I see you put send_bundle_uris() before the global bundle_uris so\n> it can be independent, but do you expect anyone to call send_bundle_uris()\n> via a different list?\n\nNo, I'll move that around or rather fold it into bundle_uri_command()\ndirectly.\n\nI think I'd originally copied the structure of send_ref() and ls_refs()\nfrom ls-refs.c, but it doesn't make much sense anymore here for this\n2-line function. Thanks.\n\n> Should we find a different place to store this data?\n>\n>> +static int bundle_uri_config(const char *var, const char *value, void *data)\n>> +{\n>> +\tif (!strcmp(var, \"uploadpack.bundleuri\")) {\n>> +\t\tadvertise_bundle_uri = 1;\n>> +\t\tstring_list_append(&bundle_uris, value);\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>\n> Here, we are dictating that the URI list is available as a multi-valued\n> config \"uploadpack.bundleuri\".\n>\n> 1. Should this be updated in Documentation/config/uploadpack.txt?\n\nDefinitely. I'll either incorporate that or re-structure this leading\nseries so that it's more design-doc/protocol focused, in any case all of\nthis ends up documented in the right places eventually...\n\n> 2. This seems difficult to extend to your possible future features as\n> listed in the protocol docs, mainly because this can only store the\n> flat URI string. To add things like hash values, sizes, and prereqs,\n> you would need more data included and grouped on a per-URI basis.\n> What plans do you have to make extensions here while remaining\n> somewhat compatible with downgrading Git versions?\n\n[...addressed below...]\n\n>> @@ -136,6 +137,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 = \"bundle-uri\",\n>> +\t\t.advertise = bundle_uri_advertise,\n>> +\t\t.command = bundle_uri_command,\n>> +\t},\n>> };\n>\n> I really appreciate that it is this simple to extend protocol v2.\n\nYeah! FWIW I've got some WIP patches to make it easier still, i.e. some\nfurther simplification & validation in the serve.[ch] API.\n\n>> +test_expect_success 'basics of bundle-uri: dies if not enabled' '\n>> +\ttest-tool pkt-line pack >in <<-EOF &&\n>> +\tcommand=bundle-uri\n>> +\t0000\n>> +\tEOF\n>> +\n>> +\tcat >err.expect <<-\\EOF &&\n>> +\tfatal: invalid command '\"'\"'bundle-uri'\"'\"'\n>> +\tEOF\n>> +\n>> +\tcat >expect <<-\\EOF &&\n>> +\tERR serve: invalid command '\"'\"'bundle-uri'\"'\"'\n>> +\tEOF\n>> +\n>> +\ttest_must_fail test-tool serve-v2 --stateless-rpc out 2>err.actual &&\n>> +\ttest_cmp err.expect err.actual &&\n>> +\ttest_must_be_empty out\n>> +'\n>> +\n>> +\n>\n> hyper-nit: double newline.\n>\n> The implementation seems simple enough, which I like. I'm a bit\n\nI mentally inserted the missing \"skeptical/uncertain\" etc. here :)\n\n> your current use of Git config as the back-end, only because it is\n> difficult to be future-proof. As the functionality stands today, the\n> current config design works just fine. Perhaps we don't need to\n> worry about the future, because we can design a new, complementary\n> storage for that extra data. It seems worth exploring for a little\n> while, though. Perhaps we should take a page out of 'git-remote'\n> and how it stores named remotes with sub-items for metadata. The\n> names probably don't need to ever be exposed to users, but it could\n> be beneficial to anyone implementing this scheme.\n>\n> [bundle \"main\"]\n> \turi = https://example.com/my-bundle\n> \turi = https://redundant-cdn.com/my-bundle\n> \tsize = 120523\n> \tsha256 = {64hexchars}\n>\n> [bundle \"fork\"]\n> \turi = https://cdn.org/my-fork\n> \tsize = 334\n> \tsha256 = {...}\n> \tprereq = {oid}\n>\n> This kind of layout has an immediate grouping of data that should\n> help any future plan. Notice how I included multiple \"uri\" lines\n> in the \"main\", which helps with your plan for duplicate URIs.\n\nAt first sight I like that config schema much better than my current\none, in particular how it makes the future-proofed \"these N urls are one\nlogical URL\" case simpler.\n\nBut overall I'm a bit on the fence, and leaning towards keeping what I\nhave, not out of any lazynes or wanting to just keep what I have mind\nyou.\n\nBut rather that the main benefit of the current one is that it's a 1=1\nmapping to the line-based protocol, and you can say update your URLs as:\n\n git config --replace-all uploadpack.bundleUri \"$first_url\" &&\n git config --add uploadpack.bundleUri \"$second_url\"\n\nHaving usually you'd know the URL you'd like to replace, so you can use\nthe [value-pattern] of --replace-all, if it's a named section or other\nsplit-out structure that become a two-step lookup.\n\nAlso for testing I've got a (trivial) plumbing tool I'll submit called\n\"git ls-remote-bundle-uri\" (could be folded into something else I guess)\nto dump the server-side config, it's nice that you can pretty much\ndirectly copy/paste it into config without needing to adjust it.\n\nHaving said all that I'm not sure I feel strongly about it either way,\nwhat do you think given the above?\n\nI think most \"real\" server operators will use this as\nGIT_CONFIG_COUNT= GIT_CONFIG_{KEY,VALUE}_<1..n>, which can of course\nwork with any config schema, but if you've got code generating it on the\nother side naming the targets is probably a slight hassle / confusing.\n\nThere's also the small matter of it being consistent with the\npackfile-uri config in its current form, but that shouldn't be a reason\nnot to come up with something better. If anything any better suggestion\n(if we go for that) could be supported by it too...\n"},{"id":"439705","messageId":"b22b894b-af03-c108-a3d5-0072c252f513@gmail.com","threadId":"56782","inReplyTo":"211026.86cznrzu8g.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-27T01:55:29Z","receivedAt":"2021-10-27T01:55:37Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/26/2021 11:00 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Tue, Oct 26 2021, Derrick Stolee wrote:\n>> The implementation seems simple enough, which I like. I'm a bit\n> \n> I mentally inserted the missing \"skeptical/uncertain\" etc. here :)\n\nMore \"uncertain\" than \"skeptical\". The current plan works perfectly\nfor the current implementation, so there is an element of YAGNI that\ncould easily lead us to avoid overthinking this.\n \n>> your current use of Git config as the back-end, only because it is\n>> difficult to be future-proof. As the functionality stands today, the\n>> current config design works just fine. Perhaps we don't need to\n>> worry about the future, because we can design a new, complementary\n>> storage for that extra data. It seems worth exploring for a little\n>> while, though. Perhaps we should take a page out of 'git-remote'\n>> and how it stores named remotes with sub-items for metadata. The\n>> names probably don't need to ever be exposed to users, but it could\n>> be beneficial to anyone implementing this scheme.\n>>\n>> [bundle \"main\"]\n>> \turi = https://example.com/my-bundle\n>> \turi = https://redundant-cdn.com/my-bundle\n>> \tsize = 120523\n>> \tsha256 = {64hexchars}\n>>\n>> [bundle \"fork\"]\n>> \turi = https://cdn.org/my-fork\n>> \tsize = 334\n>> \tsha256 = {...}\n>> \tprereq = {oid}\n>>\n>> This kind of layout has an immediate grouping of data that should\n>> help any future plan. Notice how I included multiple \"uri\" lines\n>> in the \"main\", which helps with your plan for duplicate URIs.\n> \n> At first sight I like that config schema much better than my current\n> one, in particular how it makes the future-proofed \"these N urls are one\n> logical URL\" case simpler.\n> \n> But overall I'm a bit on the fence, and leaning towards keeping what I\n> have, not out of any lazynes or wanting to just keep what I have mind\n> you.\n> \n> But rather that the main benefit of the current one is that it's a 1=1\n> mapping to the line-based protocol, and you can say update your URLs as:\n> \n> git config --replace-all uploadpack.bundleUri \"$first_url\" &&\n> git config --add uploadpack.bundleUri \"$second_url\"\n> \n> Having usually you'd know the URL you'd like to replace, so you can use\n> the [value-pattern] of --replace-all, if it's a named section or other\n> split-out structure that become a two-step lookup.\n\nDon't forget to use --fixed-value for exact string matching instead of\nregex matching!\n\n> Also for testing I've got a (trivial) plumbing tool I'll submit called\n> \"git ls-remote-bundle-uri\" (could be folded into something else I guess)\n> to dump the server-side config, it's nice that you can pretty much\n> directly copy/paste it into config without needing to adjust it.\n\nWith the appropriate helper structs and methods in the product code,\nsuch helper tools will still be simple without being a second place\nthat is directly aware of how the values are stored to disk. I don't\njudge your prototype work that helps you build the feature, but it's\nsimultaneously not a reason to stick to a design.\n\n> Having said all that I'm not sure I feel strongly about it either way,\n> what do you think given the above?\n\nI'm not feeling too strong about it right now. The current design\ndoes not need anything extra, but it also purposefully leaves certain\nthings open for extension in the future.\n\nThe thing I worry about is that there will be two supported ways to\nstore a list of bundle URIs: a flat list of URIs in the multi-valued\nuploadPack.bundleURI config value, but then also a second option that\nallows the extensions that arise. It's a layer of complication that\nwould be nice to avoid if there was an easy way to do it, but the\nschema I sketched earlier isn't simple enough to merit a switch right\nnow. Perhaps someone else will have an idea that accomplishes the\nsame goal, but also is less complicated?\n \n> I think most \"real\" server operators will use this as\n> GIT_CONFIG_COUNT= GIT_CONFIG_{KEY,VALUE}_<1..n>, which can of course\n> work with any config schema, but if you've got code generating it on the\n> other side naming the targets is probably a slight hassle / confusing.\n\nYou'd really overload the environment this way? That's not how I would\napproach it, but maybe there is a benefit over writing to the repository's\nconfig file. I suppose that you could store the data in a database and\nlink it to the repository at runtime instead.\n\n> There's also the small matter of it being consistent with the\n> packfile-uri config in its current form, but that shouldn't be a reason\n> not to come up with something better. If anything any better suggestion\n> (if we go for that) could be supported by it too...\n\nWhat do you mean about being consistent with packfile-uri? This layer\nthat we care about isn't even implemented in git.git.\n\nThanks,\n-Stolee\n"},{"id":"439706","messageId":"7fab28bf-54e7-d0e9-110a-53fad6244cc9@gmail.com","threadId":"56782","inReplyTo":"patch-2.3-3ac0539c053-20211025T211159Z-avarab@gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-27T02:01:15Z","receivedAt":"2021-10-27T02:02:39Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n> Add a server-side implementation of a new \"bundle-uri\" command to\n> protocol v2. As discussed in the updated \"protocol-v2.txt\" this will\n> allow conforming clients to optionally seed their initial clones or\n> incremental fetches from URLs containing \"*.bundle\" files created with\n> \"git bundle create\".\n\n...\n\n> +DISCUSSION of bundle-uri\n> +^^^^^^^^^^^^^^^^^^^^^^^^\n> +\n> +The intent of the feature is optimize for server resource consumption\n> +in the common case by changing the common case of fetching a very\n> +large PACK during linkgit:git-clone[1] into a smaller incremental\n> +fetch.\n> +\n> +It also allows servers to achieve better caching in combination with\n> +an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n> +\n> +By having new clones or fetches be a more predictable and common\n> +negotiation against the tips of recently produces *.bundle file(s).\n> +Servers might even pre-generate the results of such negotiations for\n> +the `uploadpack.packObjectsHook` as new pushes come in.\n> +\n> +I.e. the server would anticipate that fresh clones will download a\n> +known bundle, followed by catching up to the current state of the\n> +repository using ref tips found in that bundle (or bundles).\n> +\n> +PROTOCOL for bundle-uri\n> +^^^^^^^^^^^^^^^^^^^^^^^\n> +\n> +A `bundle-uri` request takes no arguments, and as noted above does not\n> +currently advertise a capability value. Both may be added in the\n> +future.\n\nOne thing I realized was missing from this proposal is any interaction\nwith partial clone. It would be disappointing if we could not advertise\nbundles of commit-and-tree packfiles for blobless partial clones.\n\nThere is currently no way for the client to signal the filter type\nduring this command. Not having any way to extend to include that\nseems like an oversight we should remedy before committing to a\nprotocol that can't be extended.\n\n(This also seems like a good enough reason to group the URIs into a\nstruct-like storage, because the filter type could be stored next to\nthe URI.)\n\nThanks,\n-Stolee\n"},{"id":"439746","messageId":"211027.86r1c6yh52.gmgdl@evledraar.gmail.com","threadId":"56782","inReplyTo":"7fab28bf-54e7-d0e9-110a-53fad6244cc9@gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-27T08:29:35Z","receivedAt":"2021-10-27T09:02:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 26 2021, Derrick Stolee wrote:\n\n> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>> Add a server-side implementation of a new \"bundle-uri\" command to\n>> protocol v2. As discussed in the updated \"protocol-v2.txt\" this will\n>> allow conforming clients to optionally seed their initial clones or\n>> incremental fetches from URLs containing \"*.bundle\" files created with\n>> \"git bundle create\".\n>\n> ...\n>\n>> +DISCUSSION of bundle-uri\n>> +^^^^^^^^^^^^^^^^^^^^^^^^\n>> +\n>> +The intent of the feature is optimize for server resource consumption\n>> +in the common case by changing the common case of fetching a very\n>> +large PACK during linkgit:git-clone[1] into a smaller incremental\n>> +fetch.\n>> +\n>> +It also allows servers to achieve better caching in combination with\n>> +an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n>> +\n>> +By having new clones or fetches be a more predictable and common\n>> +negotiation against the tips of recently produces *.bundle file(s).\n>> +Servers might even pre-generate the results of such negotiations for\n>> +the `uploadpack.packObjectsHook` as new pushes come in.\n>> +\n>> +I.e. the server would anticipate that fresh clones will download a\n>> +known bundle, followed by catching up to the current state of the\n>> +repository using ref tips found in that bundle (or bundles).\n>> +\n>> +PROTOCOL for bundle-uri\n>> +^^^^^^^^^^^^^^^^^^^^^^^\n>> +\n>> +A `bundle-uri` request takes no arguments, and as noted above does not\n>> +currently advertise a capability value. Both may be added in the\n>> +future.\n>\n> One thing I realized was missing from this proposal is any interaction\n> with partial clone. It would be disappointing if we could not advertise\n> bundles of commit-and-tree packfiles for blobless partial clones.\n>\n> There is currently no way for the client to signal the filter type\n> during this command. Not having any way to extend to include that\n> seems like an oversight we should remedy before committing to a\n> protocol that can't be extended.\n>\n> (This also seems like a good enough reason to group the URIs into a\n> struct-like storage, because the filter type could be stored next to\n> the URI.)\n\nI'll update the docs to note that. I'd definitely like to leave out any\nimplementation of filter/shallow for an initial iteration of this for\nsimplicity, but the protocol keyword/behavior is open-ended enough to\npermit any extension.\n\nI.e. the server can start advertising \"bundle-uri=shallow\", and future\nclients can request arbitrary key-value pairs in addition to just\n\"bundle-uri\" now.\n\nHaving said that I think that *probably* this is something that'll never\nbe implemented, but maybe I'll eat my words there.\n\nThe reason is that once we're in the \"fetch\" dialog with the server, as\nwe are with \"filter\" and \"shallow\" I'd think that we'd be better of just\nsending a packfile-uri, since that's tailor-made for that use-case.\n\nBut I suppose we could also advertise e.g.:\n\n tip= depth=1\n\nWhich a client that noticed that it noticed say the --single-branch at\n but with depth=1 could use before it ever got to \"fetch\".\n\nBut (and I haven't looked into this really) I'd think that would quickly\nget you into having a bundle with a PACK payload that wouldn't be\nrepresentable with the current bundle header format, which I think we'd\nalways want a 1=1 mapping of. I.e. you can specify a prereq, but not\nleave out trees/blobs etc.\n\nSo thoughts on that most welcome, in particular how it could be made\nfuture-proof.\n"},{"id":"439791","messageId":"756524c1-a3b9-29b5-bb72-f75a0c76ea1f@gmail.com","threadId":"56782","inReplyTo":"211027.86r1c6yh52.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-27T16:31:20Z","receivedAt":"2021-10-27T16:31:26Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/27/2021 4:29 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Tue, Oct 26 2021, Derrick Stolee wrote:\n> \n>> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>>> Add a server-side implementation of a new \"bundle-uri\" command to\n>>> protocol v2. As discussed in the updated \"protocol-v2.txt\" this will\n>>> allow conforming clients to optionally seed their initial clones or\n>>> incremental fetches from URLs containing \"*.bundle\" files created with\n>>> \"git bundle create\".\n>>\n>> ...\n>>\n>>> +DISCUSSION of bundle-uri\n>>> +^^^^^^^^^^^^^^^^^^^^^^^^\n>>> +\n>>> +The intent of the feature is optimize for server resource consumption\n>>> +in the common case by changing the common case of fetching a very\n>>> +large PACK during linkgit:git-clone[1] into a smaller incremental\n>>> +fetch.\n>>> +\n>>> +It also allows servers to achieve better caching in combination with\n>>> +an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n>>> +\n>>> +By having new clones or fetches be a more predictable and common\n>>> +negotiation against the tips of recently produces *.bundle file(s).\n>>> +Servers might even pre-generate the results of such negotiations for\n>>> +the `uploadpack.packObjectsHook` as new pushes come in.\n>>> +\n>>> +I.e. the server would anticipate that fresh clones will download a\n>>> +known bundle, followed by catching up to the current state of the\n>>> +repository using ref tips found in that bundle (or bundles).\n>>> +\n>>> +PROTOCOL for bundle-uri\n>>> +^^^^^^^^^^^^^^^^^^^^^^^\n>>> +\n>>> +A `bundle-uri` request takes no arguments, and as noted above does not\n>>> +currently advertise a capability value. Both may be added in the\n>>> +future.\n>>\n>> One thing I realized was missing from this proposal is any interaction\n>> with partial clone. It would be disappointing if we could not advertise\n>> bundles of commit-and-tree packfiles for blobless partial clones.\n>>\n>> There is currently no way for the client to signal the filter type\n>> during this command. Not having any way to extend to include that\n>> seems like an oversight we should remedy before committing to a\n>> protocol that can't be extended.\n>>\n>> (This also seems like a good enough reason to group the URIs into a\n>> struct-like storage, because the filter type could be stored next to\n>> the URI.)\n> \n> I'll update the docs to note that. I'd definitely like to leave out any\n> implementation of filter/shallow for an initial iteration of this for\n> simplicity, but the protocol keyword/behavior is open-ended enough to\n> permit any extension.\n\nIt would be good to be explicit about how this would work. Looking at\nit fresh, it seems that the server could send multiple bundle URIs with\nthe extra metadata to say which ones have a filter (and what that filter\nis). The client could then check if a bundle matches the given filter.\n\nBut this is a bit inverted: the filter mechanism currently has the client\nrequest a given filter and the server responds with _at least_ that much\ndata. This allows the server to ignore things like pathspec-filters or\ncertain size-based filters. If the client just ignores a bundle URI\nbecause it doesn't match the exact filter, this could lead the client to\nask for the data without a bundle, even if it would be faster to just\ndownload the advertised bundle.\n\nFor this reason, I think it would be valuable for the client to tell\nthe server the intended filter, and the server responds with bundle\nURIs that contain a subset of the information that would be provided\nby a later fetch request with that filter.\n\n> I.e. the server can start advertising \"bundle-uri=shallow\", and future\n> clients can request arbitrary key-value pairs in addition to just\n> \"bundle-uri\" now.\n> \n> Having said that I think that *probably* this is something that'll never\n> be implemented, but maybe I'll eat my words there.\n\nYou continue focusing on the shallow option, which I agree is not\nimportant. The filter option, specifically --filter=blob:none, seems\nto be critical to have a short-term plan for implementing with this\nin mind.\n\nThanks,\n-Stolee\n"},{"id":"439800","messageId":"211027.86mtmuxsey.gmgdl@evledraar.gmail.com","threadId":"56782","inReplyTo":"b22b894b-af03-c108-a3d5-0072c252f513@gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-27T17:49:31Z","receivedAt":"2021-10-27T17:56:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 26 2021, Derrick Stolee wrote:\n\n> On 10/26/2021 11:00 AM, Ævar Arnfjörð Bjarmason wrote:\n\n[I'll reply to the rest later, either here or in related threads. I.e. I\nmight end up entirely revamping the config etc. format]\n\n>> There's also the small matter of it being consistent with the\n>> packfile-uri config in its current form, but that shouldn't be a reason\n>> not to come up with something better. If anything any better suggestion\n>> (if we go for that) could be supported by it too...\n>\n> What do you mean about being consistent with packfile-uri? This layer\n> that we care about isn't even implemented in git.git.\n\nIt's rather limited, but we do support a uploadpack.BlobPackFileUri as a\nserver-side feature for upload-pack. I.e.:\n\n uploadpack.BlobPackFileUri= \n\nSee Documentation/technical/packfile-uri.txt.\n\nThe is part of the protocol, but the is just an aid to\nupload-pack to peel out that OID when it serves up the PACK, the \nbeing what you get from the URI.\n\nIn terms of server implementation it's rather proof-of-concept-ish,\ni.e. it's not really all that useful unless your use case is carving out\na small number of really big blobs. JGit's is much more mature, and\nthere's some patches on-list recently to make the git.git one more\npractically useful[1].\n\n1. https://lore.kernel.org/git/cover.1634634814.git.tenglong@alibaba-inc.com/\n\n"},{"id":"439813","messageId":"211027.86ilxixoxz.gmgdl@evledraar.gmail.com","threadId":"56782","inReplyTo":"756524c1-a3b9-29b5-bb72-f75a0c76ea1f@gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-27T18:01:47Z","receivedAt":"2021-10-27T19:11:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 27 2021, Derrick Stolee wrote:\n\n> On 10/27/2021 4:29 AM, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Tue, Oct 26 2021, Derrick Stolee wrote:\n>> \n>>> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>>>> Add a server-side implementation of a new \"bundle-uri\" command to\n>>>> protocol v2. As discussed in the updated \"protocol-v2.txt\" this will\n>>>> allow conforming clients to optionally seed their initial clones or\n>>>> incremental fetches from URLs containing \"*.bundle\" files created with\n>>>> \"git bundle create\".\n>>>\n>>> ...\n>>>\n>>>> +DISCUSSION of bundle-uri\n>>>> +^^^^^^^^^^^^^^^^^^^^^^^^\n>>>> +\n>>>> +The intent of the feature is optimize for server resource consumption\n>>>> +in the common case by changing the common case of fetching a very\n>>>> +large PACK during linkgit:git-clone[1] into a smaller incremental\n>>>> +fetch.\n>>>> +\n>>>> +It also allows servers to achieve better caching in combination with\n>>>> +an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n>>>> +\n>>>> +By having new clones or fetches be a more predictable and common\n>>>> +negotiation against the tips of recently produces *.bundle file(s).\n>>>> +Servers might even pre-generate the results of such negotiations for\n>>>> +the `uploadpack.packObjectsHook` as new pushes come in.\n>>>> +\n>>>> +I.e. the server would anticipate that fresh clones will download a\n>>>> +known bundle, followed by catching up to the current state of the\n>>>> +repository using ref tips found in that bundle (or bundles).\n>>>> +\n>>>> +PROTOCOL for bundle-uri\n>>>> +^^^^^^^^^^^^^^^^^^^^^^^\n>>>> +\n>>>> +A `bundle-uri` request takes no arguments, and as noted above does not\n>>>> +currently advertise a capability value. Both may be added in the\n>>>> +future.\n>>>\n>>> One thing I realized was missing from this proposal is any interaction\n>>> with partial clone. It would be disappointing if we could not advertise\n>>> bundles of commit-and-tree packfiles for blobless partial clones.\n>>>\n>>> There is currently no way for the client to signal the filter type\n>>> during this command. Not having any way to extend to include that\n>>> seems like an oversight we should remedy before committing to a\n>>> protocol that can't be extended.\n>>>\n>>> (This also seems like a good enough reason to group the URIs into a\n>>> struct-like storage, because the filter type could be stored next to\n>>> the URI.)\n>> \n>> I'll update the docs to note that. I'd definitely like to leave out any\n>> implementation of filter/shallow for an initial iteration of this for\n>> simplicity, but the protocol keyword/behavior is open-ended enough to\n>> permit any extension.\n>\n> It would be good to be explicit about how this would work. Looking at\n> it fresh, it seems that the server could send multiple bundle URIs with\n> the extra metadata to say which ones have a filter (and what that filter\n> is). The client could then check if a bundle matches the given filter.\n>\n> But this is a bit inverted: the filter mechanism currently has the client\n> request a given filter and the server responds with _at least_ that much\n> data. This allows the server to ignore things like pathspec-filters or\n> certain size-based filters. If the client just ignores a bundle URI\n> because it doesn't match the exact filter, this could lead the client to\n> ask for the data without a bundle, even if it would be faster to just\n> download the advertised bundle.\n>\n> For this reason, I think it would be valuable for the client to tell\n> the server the intended filter, and the server responds with bundle\n> URIs that contain a subset of the information that would be provided\n> by a later fetch request with that filter.\n>\n>> I.e. the server can start advertising \"bundle-uri=shallow\", and future\n>> clients can request arbitrary key-value pairs in addition to just\n>> \"bundle-uri\" now.\n>> \n>> Having said that I think that *probably* this is something that'll never\n>> be implemented, but maybe I'll eat my words there.\n\nI didn't mean to elide past \"filter\", but was just using \"shallow\" as a\nshort-hand for one thing in the \"fetch\" dialog that a client can mention\nthat'll impact PACK generation, just like filter.\n\nHaving thought about this a bit more, I think it should be an invariant\nin any bundle-uri design that the server shouldn't communicate any\nside-channel information whatsoever about a bundle it advertises, if\nthat information can't be discovered in the header of that bundle file.\n\nMind you, this means throwing out large parts of my current proposed\nover-the-wire design, but I think for the better. I.e. the whole\nresponse part where we communicate:\n\n (bundle-uri (SP bundle-feature-key (=bundle-feature-val)?)* LF)*\n flush-pkt\n\nWould just become something like:\n\n (bundle-uri delim-pkt bundle-header? delim-pkt)*\n flush-pkt\n\nI.e. we'd optionally transfer the content of the bundle header (content\nup to the first \"\\n\\n\") to the client, but *only* ever as a shorthand\nfor saving the client a roundtrip.\n\nThe pointed-to bundle is still 100% the source of truth, and when\nretrieving the bundle-uri we'd ignore whatever \"bundle-header\" we got\nearlier (except insofar as we'd like to say emit a warning() if the two\ndon't match).\n\n(I'd not thought too carefully about these shallow/filter etc. edge\ncases, my main intended use-case has been pre-seeding full clones, and\nhaving this feedback to make me think about it is very valuable).\n\n> You continue focusing on the shallow option, which I agree is not\n> important. The filter option, specifically --filter=blob:none, seems\n> to be critical to have a short-term plan for implementing with this\n> in mind.\n\nPer the above this then just becomes a question of \"how do we produce a\nbundle with those attributes?\".\n\nI *think* that currently there isn't a way to do that, i.e. the PACK\npayload of a bundle is the output of \"git pack-objects\", but due to it\nincluding refs, tips and prerequisites.\n\nI don't think you can say \"this bundle has no blobs\". The\n\"prerequisites\" hard map to the same thing you could put on a\n\"want/have\" line during PACK negotiation.\n\nI think we could/should fix that, i.e. we can bump the bundle format\nversion and have it encode some extended prerequisites/filter/shallow\netc information. You'd then have a 1=1 match between the features of\ngit-upload-pack and what you can transfer via the bundle side-channel.\n\nBut the more I think about it, the more strongly I feel that we should\nalways add that to the bundle *format*, and not as some side-channel\ninformation in this \"bundle-uri\" protocol keyword.\n\nTo me *the* point of this feature is to have servers provide a shorthand\nfor something that's been a well-established trick you can do today, and\nof which there are any number of pre-existing implementations.\n\nI'm not trying to break any new ground here, just make \"git\n[fetch|clone]\" support a well-known trick as a first-class feature via\nprotocol v2.\n\nI'm not the first person to whip up some custom\n\"git-clone-via-bundle.sh\" that takes bundle URI(s) and a repo URI,\nwget's the bundle, calls \"git bundle unbundle\", updates ref tips, and\nthen does a \"git fetch\".\n\nThe benefit of making that a first-class protocol feature over a full\nnegotiation is essentially synonymous with how it's easier in practice\nto widely deploy static assets on CDNs v.s. guaranteeing the same\nnetwork locality, caching etc. when serving up the same payload by\nrunning a custom binary.\n\nOne reason not to add any side-channel information not found in the\nbundle header(s) is that we can also guarantee that there won't be any\nfeature gap between the \"transfer.injectBundleURI\" config key I've\nalready got implemented (and is in the earlier RFC version of this\nseries). I.e. you can do:\n\n # You can specify this N number of times to inject N bundles\n git clone \\\n\t-c transfer.injectBundleURI=\"https://something.local/some-repo.bundle\" \\\n https://example.com/some-repo.git\n\nTo inject CDN support to any remote server that doesn't know about\n\"bundle-uri\", or add to the bundles of a server that does. That URI can\neven be a file:// if you add \"-c fetch.uriProtocols=file\".\n\nI realize that all of the above does *not* answer part of your question\nabout filters, which I think I can accurately rephrase as:\n\n Ok, so you can dump a static list of bundle URIs from config, but\n that's always going to be a small list, what about the combinatorial\n explosion arbitrary upload-pack options? Filters, shallow,\n include-tag etc.\n\nMy main answer to that is that YAGNI. If you need to spew out an URL for\na PACK after a client describes any of its arbitrary wants, needs,\nfilters etc. you've exactly re-invented what \"packfile-uri\" is today. I\nthink that feature is very useful, and I've got no intention of trying\nto replace it.\n\nI think the sweet spot for \"bundle-uri\" is to advertise a small number\nof bundles that encompass common clone/fetch patterns. I.e. something\nlike a bundle for a full clone with the repo data up to today, and maybe\na couple of other bundles covering a time period that clients would be\nlikely to incrementally update within, e.g. 1 week ago..today &&\nyesterday..now.\n\nI agree that adding say \"full clone, --depth=1\" and \"full clone, no\nblobs\" etc. to that might be *very* useful for some deployments, but per\nthe above I think we should really add that to the bundle format first,\nnot protocol v2.\n"},{"id":"439815","messageId":"e3c923b4-cce0-372c-31f4-9f091239ad8b@gmail.com","threadId":"56782","inReplyTo":"211027.86ilxixoxz.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-27T19:23:22Z","receivedAt":"2021-10-27T19:23:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/27/2021 2:01 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Oct 27 2021, Derrick Stolee wrote:\n> \n>> On 10/27/2021 4:29 AM, Ævar Arnfjörð Bjarmason wrote:\n>>>\n>>> On Tue, Oct 26 2021, Derrick Stolee wrote:\n>>>\n>>>> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>>>>> Add a server-side implementation of a new \"bundle-uri\" command to\n>>>>> protocol v2. As discussed in the updated \"protocol-v2.txt\" this will\n>>>>> allow conforming clients to optionally seed their initial clones or\n>>>>> incremental fetches from URLs containing \"*.bundle\" files created with\n>>>>> \"git bundle create\".\n>>>>\n>>>> ...\n>>>>\n>>>>> +DISCUSSION of bundle-uri\n>>>>> +^^^^^^^^^^^^^^^^^^^^^^^^\n>>>>> +\n>>>>> +The intent of the feature is optimize for server resource consumption\n>>>>> +in the common case by changing the common case of fetching a very\n>>>>> +large PACK during linkgit:git-clone[1] into a smaller incremental\n>>>>> +fetch.\n>>>>> +\n>>>>> +It also allows servers to achieve better caching in combination with\n>>>>> +an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n>>>>> +\n>>>>> +By having new clones or fetches be a more predictable and common\n>>>>> +negotiation against the tips of recently produces *.bundle file(s).\n>>>>> +Servers might even pre-generate the results of such negotiations for\n>>>>> +the `uploadpack.packObjectsHook` as new pushes come in.\n>>>>> +\n>>>>> +I.e. the server would anticipate that fresh clones will download a\n>>>>> +known bundle, followed by catching up to the current state of the\n>>>>> +repository using ref tips found in that bundle (or bundles).\n>>>>> +\n>>>>> +PROTOCOL for bundle-uri\n>>>>> +^^^^^^^^^^^^^^^^^^^^^^^\n>>>>> +\n>>>>> +A `bundle-uri` request takes no arguments, and as noted above does not\n>>>>> +currently advertise a capability value. Both may be added in the\n>>>>> +future.\n>>>>\n>>>> One thing I realized was missing from this proposal is any interaction\n>>>> with partial clone. It would be disappointing if we could not advertise\n>>>> bundles of commit-and-tree packfiles for blobless partial clones.\n>>>>\n>>>> There is currently no way for the client to signal the filter type\n>>>> during this command. Not having any way to extend to include that\n>>>> seems like an oversight we should remedy before committing to a\n>>>> protocol that can't be extended.\n>>>>\n>>>> (This also seems like a good enough reason to group the URIs into a\n>>>> struct-like storage, because the filter type could be stored next to\n>>>> the URI.)\n>>>\n>>> I'll update the docs to note that. I'd definitely like to leave out any\n>>> implementation of filter/shallow for an initial iteration of this for\n>>> simplicity, but the protocol keyword/behavior is open-ended enough to\n>>> permit any extension.\n>>\n>> It would be good to be explicit about how this would work. Looking at\n>> it fresh, it seems that the server could send multiple bundle URIs with\n>> the extra metadata to say which ones have a filter (and what that filter\n>> is). The client could then check if a bundle matches the given filter.\n>>\n>> But this is a bit inverted: the filter mechanism currently has the client\n>> request a given filter and the server responds with _at least_ that much\n>> data. This allows the server to ignore things like pathspec-filters or\n>> certain size-based filters. If the client just ignores a bundle URI\n>> because it doesn't match the exact filter, this could lead the client to\n>> ask for the data without a bundle, even if it would be faster to just\n>> download the advertised bundle.\n>>\n>> For this reason, I think it would be valuable for the client to tell\n>> the server the intended filter, and the server responds with bundle\n>> URIs that contain a subset of the information that would be provided\n>> by a later fetch request with that filter.\n>>\n>>> I.e. the server can start advertising \"bundle-uri=shallow\", and future\n>>> clients can request arbitrary key-value pairs in addition to just\n>>> \"bundle-uri\" now.\n>>>\n>>> Having said that I think that *probably* this is something that'll never\n>>> be implemented, but maybe I'll eat my words there.\n> \n> I didn't mean to elide past \"filter\", but was just using \"shallow\" as a\n> short-hand for one thing in the \"fetch\" dialog that a client can mention\n> that'll impact PACK generation, just like filter.\n> \n> Having thought about this a bit more, I think it should be an invariant\n> in any bundle-uri design that the server shouldn't communicate any\n> side-channel information whatsoever about a bundle it advertises, if\n> that information can't be discovered in the header of that bundle file.\n> \n> Mind you, this means throwing out large parts of my current proposed\n> over-the-wire design, but I think for the better. I.e. the whole\n> response part where we communicate:\n> \n> (bundle-uri (SP bundle-feature-key (=bundle-feature-val)?)* LF)*\n> flush-pkt\n> \n> Would just become something like:\n> \n> (bundle-uri delim-pkt bundle-header? delim-pkt)*\n> flush-pkt\n> \n> I.e. we'd optionally transfer the content of the bundle header (content\n> up to the first \"\\n\\n\") to the client, but *only* ever as a shorthand\n> for saving the client a roundtrip.\n\nIt still seems like we're better off letting the client request a\nfilter and have the server present URIs that the client can use,\nand the server can choose to ignore the filter or provide URIs that\nare specific to that filter. Sending the full list and making the\nclient decide what it wants seems like it will be more complicated\nthan necessary.\n\nHowever, I'll withhold complete judgement until I see a full proposal\nin a v2.\n\n> I think the sweet spot for \"bundle-uri\" is to advertise a small number\n> of bundles that encompass common clone/fetch patterns. I.e. something\n> like a bundle for a full clone with the repo data up to today, and maybe\n> a couple of other bundles covering a time period that clients would be\n> likely to incrementally update within, e.g. 1 week ago..today &&\n> yesterday..now.\n> \n> I agree that adding say \"full clone, --depth=1\" and \"full clone, no\n> blobs\" etc. to that might be *very* useful for some deployments, but per\n> the above I think we should really add that to the bundle format first,\n> not protocol v2.\n \nI'm focusing my interest in this topic not on \"how can we make what we\nalready do faster?\" but \"how can we unlock scale not previously\npossible?\" Allowing blobless clones is an important part of this, in\nmy opinion, so it is my _default_ mode of operating.\n\nThanks,\n-Stolee\n"},{"id":"439831","messageId":"211027.86a6iuxk3x.gmgdl@evledraar.gmail.com","threadId":"56782","inReplyTo":"e3c923b4-cce0-372c-31f4-9f091239ad8b@gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-27T20:22:28Z","receivedAt":"2021-10-27T20:56:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 27 2021, Derrick Stolee wrote:\n\n> On 10/27/2021 2:01 PM, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Wed, Oct 27 2021, Derrick Stolee wrote:\n>> \n>>> On 10/27/2021 4:29 AM, Ævar Arnfjörð Bjarmason wrote:\n>>>>\n>>>> On Tue, Oct 26 2021, Derrick Stolee wrote:\n>>>>\n>>>>> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>>>>>> Add a server-side implementation of a new \"bundle-uri\" command to\n>>>>>> protocol v2. As discussed in the updated \"protocol-v2.txt\" this will\n>>>>>> allow conforming clients to optionally seed their initial clones or\n>>>>>> incremental fetches from URLs containing \"*.bundle\" files created with\n>>>>>> \"git bundle create\".\n>>>>>\n>>>>> ...\n>>>>>\n>>>>>> +DISCUSSION of bundle-uri\n>>>>>> +^^^^^^^^^^^^^^^^^^^^^^^^\n>>>>>> +\n>>>>>> +The intent of the feature is optimize for server resource consumption\n>>>>>> +in the common case by changing the common case of fetching a very\n>>>>>> +large PACK during linkgit:git-clone[1] into a smaller incremental\n>>>>>> +fetch.\n>>>>>> +\n>>>>>> +It also allows servers to achieve better caching in combination with\n>>>>>> +an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n>>>>>> +\n>>>>>> +By having new clones or fetches be a more predictable and common\n>>>>>> +negotiation against the tips of recently produces *.bundle file(s).\n>>>>>> +Servers might even pre-generate the results of such negotiations for\n>>>>>> +the `uploadpack.packObjectsHook` as new pushes come in.\n>>>>>> +\n>>>>>> +I.e. the server would anticipate that fresh clones will download a\n>>>>>> +known bundle, followed by catching up to the current state of the\n>>>>>> +repository using ref tips found in that bundle (or bundles).\n>>>>>> +\n>>>>>> +PROTOCOL for bundle-uri\n>>>>>> +^^^^^^^^^^^^^^^^^^^^^^^\n>>>>>> +\n>>>>>> +A `bundle-uri` request takes no arguments, and as noted above does not\n>>>>>> +currently advertise a capability value. Both may be added in the\n>>>>>> +future.\n>>>>>\n>>>>> One thing I realized was missing from this proposal is any interaction\n>>>>> with partial clone. It would be disappointing if we could not advertise\n>>>>> bundles of commit-and-tree packfiles for blobless partial clones.\n>>>>>\n>>>>> There is currently no way for the client to signal the filter type\n>>>>> during this command. Not having any way to extend to include that\n>>>>> seems like an oversight we should remedy before committing to a\n>>>>> protocol that can't be extended.\n>>>>>\n>>>>> (This also seems like a good enough reason to group the URIs into a\n>>>>> struct-like storage, because the filter type could be stored next to\n>>>>> the URI.)\n>>>>\n>>>> I'll update the docs to note that. I'd definitely like to leave out any\n>>>> implementation of filter/shallow for an initial iteration of this for\n>>>> simplicity, but the protocol keyword/behavior is open-ended enough to\n>>>> permit any extension.\n>>>\n>>> It would be good to be explicit about how this would work. Looking at\n>>> it fresh, it seems that the server could send multiple bundle URIs with\n>>> the extra metadata to say which ones have a filter (and what that filter\n>>> is). The client could then check if a bundle matches the given filter.\n>>>\n>>> But this is a bit inverted: the filter mechanism currently has the client\n>>> request a given filter and the server responds with _at least_ that much\n>>> data. This allows the server to ignore things like pathspec-filters or\n>>> certain size-based filters. If the client just ignores a bundle URI\n>>> because it doesn't match the exact filter, this could lead the client to\n>>> ask for the data without a bundle, even if it would be faster to just\n>>> download the advertised bundle.\n>>>\n>>> For this reason, I think it would be valuable for the client to tell\n>>> the server the intended filter, and the server responds with bundle\n>>> URIs that contain a subset of the information that would be provided\n>>> by a later fetch request with that filter.\n>>>\n>>>> I.e. the server can start advertising \"bundle-uri=shallow\", and future\n>>>> clients can request arbitrary key-value pairs in addition to just\n>>>> \"bundle-uri\" now.\n>>>>\n>>>> Having said that I think that *probably* this is something that'll never\n>>>> be implemented, but maybe I'll eat my words there.\n>> \n>> I didn't mean to elide past \"filter\", but was just using \"shallow\" as a\n>> short-hand for one thing in the \"fetch\" dialog that a client can mention\n>> that'll impact PACK generation, just like filter.\n>> \n>> Having thought about this a bit more, I think it should be an invariant\n>> in any bundle-uri design that the server shouldn't communicate any\n>> side-channel information whatsoever about a bundle it advertises, if\n>> that information can't be discovered in the header of that bundle file.\n>> \n>> Mind you, this means throwing out large parts of my current proposed\n>> over-the-wire design, but I think for the better. I.e. the whole\n>> response part where we communicate:\n>> \n>> (bundle-uri (SP bundle-feature-key (=bundle-feature-val)?)* LF)*\n>> flush-pkt\n>> \n>> Would just become something like:\n>> \n>> (bundle-uri delim-pkt bundle-header? delim-pkt)*\n>> flush-pkt\n>> \n>> I.e. we'd optionally transfer the content of the bundle header (content\n>> up to the first \"\\n\\n\") to the client, but *only* ever as a shorthand\n>> for saving the client a roundtrip.\n>\n> It still seems like we're better off letting the client request a\n> filter and have the server present URIs that the client can use,\n> and the server can choose to ignore the filter or provide URIs that\n> are specific to that filter.[...]\n\nI've tested this a bit now and think there's no way to create such a\nbundle currently. I.e. try:\n\n git clone --filter=blob:none --single-branch --no-tags https://github.com/git/git.git\n cd git\n git config --unset remote.origin.partialclonefilter\n git config --unset remote.origin.promisor\n\nYou'll get:\n \n $ GIT_TRACE_PACKET=1 git bundle create --version=3 master.bdl master\n Enumerating objects: 306784, done.\n Counting objects: 100% (306784/306784), done.\n Compressing objects: 100% (69265/69265), done.\n fatal: unable to read c85385dc03228450cb7fb6d306252038a91b47e6\n error: pack-objects died\n\nIf you didn't do that config munging we'd create the pack, but it would\nbe inflated with the blobs (after going back and getting them from the\nserver).\n\nSo aside from any questions of how you'd hypothetically communicate your\ndesire to get such bundle from the server, I don't think it could serve\none up.\n\nSo I think this is moot until the bundle format itself could support\nit. I'll need to \"git bundle [verify|unbundle]\" whatever I get on the\nother end.\n\nI really don't mean this in any way as dodging the desirability of this\nfeature. I'd really like to have it too. I think implementing it should\nbe relatively simple, and I've got an implementation in mind that makes\nthis future-proof for anything else we'd like to add.\n\nI.e. if you look at that v3 format bundle you'll see:\n \n $ head -c 100 master.bdl\n # v3 git bundle\n @object-format=sha1\n e9e5ba39a78c8f5057262d49e261b42a8660d5b9 refs/heads/master\n \n PACK\n\nWouldn't this just be a matter of including extra lines with:\n\n # I'm assuming that the promisor url can be assumed to be \"the url\n # we cloned this from\", but maybe we need @remoteURL=https://....\n @promisor=true\n @filter = blob:none\n\nI.e. exactly corresponding to the .git/config we'd end up with,\nconfig. We'd then (I think) create .git/objects/pack/*.promisor with the\nOIDs of each of the inflated tips (I'm not familiar with .promisor\nfiles).\n\nAnd a thing I need to include in the bundle-uri protocol is that the\nclient should not just include a \"bundle-uri\" attribute, but have a\n\"value\" describing the bundle format it accepts. I.e. now:\n\n bundle-uri=v3,object-format\n\nAnd for supporting the above:\n\n bundle-uri=v3,object-format,promisor,filter\n\nI.e. currently we die on any bundle capability except \"object-format\",\nif we're going to discover what to send we'd like a less crappy way than\nparsing the version from the \"agent\" field.\n\n> Sending the full list and making the client decide what it wants seems\n> like it will be more complicated than necessary. However, I'll\n> withhold complete judgement until I see a full proposal in a v2.\n\nI'm very thankful for the thorough review, and it's exciting that you'd\nlike to use this feature in some form, and I'll definitely do my best to\nsupport (and if not, future-proof) any use-cases you have in mind.\n\nBut I really don't get how this wouldn't effectively be functionally\nindistinguishable from packfile-uri, sans a nit here and there.\n\nI can see the convenience of having say 100 bundles, advertising 5, and\nthen after a full negotiation dialog pointing the equivalent of a\npackfile-uri at a *.bundle file, just because that's what you happen to\nhave around already. If bundle-uri is your main static file distribution\nyou don't want a duplicate *.pack (without the bundle header) just for\nthat.\n\nI think a logical extension of the packfile-uri feature for those that\nneed extended negotiation before deciding on the static URL would be to\nteach the packfile-uri downloader to ignore an optional bundle header of\nany PACK it finds at a URL (which would not be the same as this\nproposal), just to support that use-case.\n\nBut, isn't that essentially what you'd want in those cases?\n\nSpewing a \"here's my bundles\" at a client gets it started quickly, and\nalso has the side-benefit of making those assets more cachable, as well\nas creating a known base for the caching of any subsequent \"...and a\nPACK negotiation to make it fully up-to-date\" request.\n\nThe bundles are also our de-facto sneakernet and format, and can be used\nfor incremental replication. All of which is also a sweet spot for\nbundle-uri, i.e. the combination of being able to re-use already\n\"replicated\" files for CDN-ing, and providing wider access to CDN\nfeatures for \"dumb\" servers.\n\nBut once we're in dialog with a client to discuss its arbitrary filter\npreferences before giving it a URL we're going to be most likely\nimplementing that as a mode of upload-pack.c anyway, and when it spews\nout an optional URL at the end of that dialog....\n\n>> I think the sweet spot for \"bundle-uri\" is to advertise a small number\n>> of bundles that encompass common clone/fetch patterns. I.e. something\n>> like a bundle for a full clone with the repo data up to today, and maybe\n>> a couple of other bundles covering a time period that clients would be\n>> likely to incrementally update within, e.g. 1 week ago..today &&\n>> yesterday..now.\n>> \n>> I agree that adding say \"full clone, --depth=1\" and \"full clone, no\n>> blobs\" etc. to that might be *very* useful for some deployments, but per\n>> the above I think we should really add that to the bundle format first,\n>> not protocol v2.\n> \n> I'm focusing my interest in this topic not on \"how can we make what we\n> already do faster?\" but \"how can we unlock scale not previously\n> possible?\" Allowing blobless clones is an important part of this, in\n> my opinion, so it is my _default_ mode of operating.\n\n*nod*, I'm keen to support it using something like what's described\nabove. I.e. stick it as new headers in the bundle format, then be able\nto advertise those for common cases. I'd think most of these would be a\nfew combinations (usually something copy/pasted from a relevant dev\nguide), e.g. \"all history, no blobs\", \"main branch, no tags, no blobs\"\netc.\n\nAside from this bundle-uri protocol proposal being able to sneakernet a\nrepo you cloned like that around as-is seems highly desirable, and just\na feature gap we added when those features were added to \"git fetch\" and\nfriends, but not \"git bundle\".\n"},{"id":"440036","messageId":"4039d500-1924-9610-495f-69e196f17322@gmail.com","threadId":"56782","inReplyTo":"211027.86a6iuxk3x.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-29T18:30:54Z","receivedAt":"2021-10-29T18:31:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/27/2021 4:22 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Oct 27 2021, Derrick Stolee wrote:\n>> It still seems like we're better off letting the client request a\n>> filter and have the server present URIs that the client can use,\n>> and the server can choose to ignore the filter or provide URIs that\n>> are specific to that filter.[...]\n> \n> I've tested this a bit now and think there's no way to create such a\n> bundle currently. I.e. try:\n> \n> git clone --filter=blob:none --single-branch --no-tags https://github.com/git/git.git\n> cd git\n> git config --unset remote.origin.partialclonefilter\n> git config --unset remote.origin.promisor\n> \n> You'll get:\n> \n> $ GIT_TRACE_PACKET=1 git bundle create --version=3 master.bdl master\n> Enumerating objects: 306784, done.\n> Counting objects: 100% (306784/306784), done.\n> Compressing objects: 100% (69265/69265), done.\n> fatal: unable to read c85385dc03228450cb7fb6d306252038a91b47e6\n> error: pack-objects died\n> \n> If you didn't do that config munging we'd create the pack, but it would\n> be inflated with the blobs (after going back and getting them from the\n> server).\n> \n> So aside from any questions of how you'd hypothetically communicate your\n> desire to get such bundle from the server, I don't think it could serve\n> one up.\n> \n> So I think this is moot until the bundle format itself could support\n> it. I'll need to \"git bundle [verify|unbundle]\" whatever I get on the\n> other end.\n\nThank you for demonstrating that bundles don't currently work with\nfilters. This would need to be remedied, but can be done independently.\n\nFor now, let's just make sure that there is a paved path for extending\nwhatever is decided here to work with filters in the future.\n\nAs for the overall design, I have some thoughts that I'm going to\nbreak out into a new message responding to your cover letter.\n\nThanks,\n-Stolee\n"},{"id":"440047","messageId":"c9c4e1e7-aaa2-bbad-355b-8439fad93fa7@gmail.com","threadId":"56782","inReplyTo":"cover-0.3-00000000000-20211025T211159Z-avarab@gmail.com","subject":"Re: [PATCH 0/3] bundle-uri: \"dumb\" static CDN offloading, spec & server implementation","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-29T18:46:19Z","receivedAt":"2021-10-29T18:46:46Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n> This implements a new \"bundle-uri\" protocol v2 extension, which allows\n> servers to advertise *.bundle files which clients can pre-seed their\n> full \"clone\"'s or incremental \"fetch\"'s from.\n> \n> This is both an alternative to, and complimentary to the existing\n> \"packfile-uri\" mechanism, i.e. servers and/or clients can pick one or\n> both, but would generally pick one over the other.\n> \n> This \"bundle-uri\" mechanism has the advantage of being dumber, and\n> offloads more complexity from the server side to the client\n> side.\n\nGenerally, I like that using bundles presents an easier way to serve\nstatic content from an alternative source and then let Git's fetch\nnegotiation catch up with the remainder.\n\nHowever, after inspecting your design and talking to some GitHub\nengineers who know more about CDNs and general internet things than I\ndo, I want to propose an alternative design. I think this new design\nis simultaneously more flexible as well as promotes further decoupling\nof the origin Git server and the bundle contents.\n\nYour proposed design extends protocol v2 to let the client request a\nlist of bundle URIs from the origin server. However, this still requires\nthe origin server to know about this list. Further, your implementation\nfocuses on the server side without integrating with the client.\n\nI propose that we flip this around. The \"bundle server\" should know\nwhich bundles are available at which URIs, and the client should contact\nthe bundle server directly for a \"table of contents\" that lists these\nURIs, along with metadata related to each URI. The origin Git server\nthen would only need to store the list of bundle servers and the URIs\nto their table of contents. The client could then pick from among those\nbundle servers (probably by ping time, or randomly) to start the bundle\ndownloads.\n\nTo summarize, there are two pieces here, that can be implemented at\ndifferent times:\n\n1. Create a specification for a \"bundle server\" that doesn't need to\n speak the Git protocol at all. This could be a REST API specification\n using well-established standards such as JSON for the table of\n contents.\n\n2. Create a way for the origin Git server to advertise known bundle\n servers to clients so they can automatically benefit from faster\n downloads without needing to know about bundle servers.\n\nThere are a few key benefits to this approach:\n\n * Further decoupling. The origin Git server doesn't need to know how\n the bundle server organizes its bundles. This allows maximum flexibility\n depending on whether the bundles are stored in something like a CDN\n (where bundles can't be too big) or some kind of blob storage (where\n they can have arbitrarily large size).\n\n * The bundle servers could be run completely independently from the\n origin Git server. Organizations could run their own bundle servers to\n host data in the same building as their build farms. As long as they\n can configure the bundle location at clone/fetch time, the origin Git\n server doesn't need to be involved.\n\nWhile I didn't go so far as to create a clear standard or implement a\nprototype in the Git codebase, I created a very simple prototype [1] using\na python script that parses a JSON table of contents and downloads\nbundles into the Git repository. Then, I made a 'clone.sh' script that\ninitializes a repository using the bundle fetcher and fetching the\nremainder from the origin Git server. I even computed static bundles for\nthe git.git repository based on where 'master' has been over several days\nin the past month, to give an example of incremental bundles. You can\ntest the approach all the way to including the fetch to github.com (note\nhow the GitHub servers were not modified in any way for this).\n\n[1] https://github.com/derrickstolee/bundles\n\nThere are a lot of limitations to the prototype, but it hopefully\ndemonstrates the possibility of using something other than the Git protocol\nto solve these problems.\n\nLet me know if you are interested in switching your approach to something\nmore like what I propose here. There are many more questions about what\ninformation could/should be located in the table of contents and how it can\nbe extended in the future. I'm interested to explore that space with you.\n\nThanks,\n-Stolee\n"},{"id":"440119","messageId":"211030.86mtmr3po0.gmgdl@evledraar.gmail.com","threadId":"56782","inReplyTo":"c9c4e1e7-aaa2-bbad-355b-8439fad93fa7@gmail.com","subject":"Re: [PATCH 0/3] bundle-uri: \"dumb\" static CDN offloading, spec & server implementation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-30T07:21:38Z","receivedAt":"2021-10-30T08:03:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Oct 29 2021, Derrick Stolee wrote:\n\n> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>> This implements a new \"bundle-uri\" protocol v2 extension, which allows\n>> servers to advertise *.bundle files which clients can pre-seed their\n>> full \"clone\"'s or incremental \"fetch\"'s from.\n>> \n>> This is both an alternative to, and complimentary to the existing\n>> \"packfile-uri\" mechanism, i.e. servers and/or clients can pick one or\n>> both, but would generally pick one over the other.\n>> \n>> This \"bundle-uri\" mechanism has the advantage of being dumber, and\n>> offloads more complexity from the server side to the client\n>> side.\n>\n> Generally, I like that using bundles presents an easier way to serve\n> static content from an alternative source and then let Git's fetch\n> negotiation catch up with the remainder.\n>\n> However, after inspecting your design and talking to some GitHub\n> engineers who know more about CDNs and general internet things than I\n> do, I want to propose an alternative design. I think this new design\n> is simultaneously more flexible as well as promotes further decoupling\n> of the origin Git server and the bundle contents.\n>\n> Your proposed design extends protocol v2 to let the client request a\n> list of bundle URIs from the origin server. However, this still requires\n> the origin server to know about this list. [...]\n\nInteresting, more below...\n\n> Further, your implementation focuses on the server side without\n> integrating with the client.\n\nDo you mean these 3 patches we're discussing now? Yes, that's the\nserver-side and protocol specification only, because I figured talking\nabout just the spec might be helpful.\n\nBut as noted in the CL and previously on-list I have a larger set of\npatches to implement the client behavior, an old RFC version of that\nhere (I've since changed some things...):\nhttps://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n\nI mean, you commented on those too, so I'm not sure if that's what you\nmeant, but just for context...\n\n> I propose that we flip this around. The \"bundle server\" should know\n> which bundles are available at which URIs, and the client should contact\n> the bundle server directly for a \"table of contents\" that lists these\n> URIs, along with metadata related to each URI. The origin Git server\n> then would only need to store the list of bundle servers and the URIs\n> to their table of contents. The client could then pick from among those\n> bundle servers (probably by ping time, or randomly) to start the bundle\n> downloads.\n\nI hadn't considered the server not advertising the list, but pointing to\nanother URI that has the list. I was thinking that the server would be\nclose enough to whatever's generating the list that updating the list\nthere wouldn't be a meaningful limitation for anyone.\n\nBut you seem to have a use-case for it, I'd be curious to hear why\nspecifically, but in any case that's easy to support in the client\npatches I have.\n\nThere's a point at which we get the list of URIs from the server, to\nsupport your case the client would just advertise the one TOC URI.\n\nThen similarly to the \"packfile-uri\" special-case of handling a *.bundle\ninstead of a PACK that I noted in [1], the downloader would just spot\n\"oh this isn't a bundle, but list of URIs, and then fetch those (even\nrecursively), and eventually get to *.bundle files.\n\n> To summarize, there are two pieces here, that can be implemented at\n> different times:\n>\n> 1. Create a specification for a \"bundle server\" that doesn't need to\n> speak the Git protocol at all. This could be a REST API specification\n> using well-established standards such as JSON for the table of\n> contents.\n>\n> 2. Create a way for the origin Git server to advertise known bundle\n> servers to clients so they can automatically benefit from faster\n> downloads without needing to know about bundle servers.\n>\n> There are a few key benefits to this approach:\n>\n> * Further decoupling. The origin Git server doesn't need to know how\n> the bundle server organizes its bundles. This allows maximum flexibility\n> depending on whether the bundles are stored in something like a CDN\n> (where bundles can't be too big) or some kind of blob storage (where\n> they can have arbitrarily large size).\n>\n> * The bundle servers could be run completely independently from the\n> origin Git server. Organizations could run their own bundle servers to\n> host data in the same building as their build farms. As long as they\n> can configure the bundle location at clone/fetch time, the origin Git\n> server doesn't need to be involved.\n>\n> While I didn't go so far as to create a clear standard or implement a\n> prototype in the Git codebase, I created a very simple prototype [1] using\n> a python script that parses a JSON table of contents and downloads\n> bundles into the Git repository. Then, I made a 'clone.sh' script that\n> initializes a repository using the bundle fetcher and fetching the\n> remainder from the origin Git server. I even computed static bundles for\n> the git.git repository based on where 'master' has been over several days\n> in the past month, to give an example of incremental bundles. You can\n> test the approach all the way to including the fetch to github.com (note\n> how the GitHub servers were not modified in any way for this).\n>\n> [1] https://github.com/derrickstolee/bundles\n>\n> There are a lot of limitations to the prototype, but it hopefully\n> demonstrates the possibility of using something other than the Git protocol\n> to solve these problems.\n\nIn your proposal the TOC bundle itself doesn't need to speak the git\nprotocol.\n\nBut as as soon as we specify such a thing all of that becomes a part of\nthe git protocol at large in any meaningful way, i.e. git.git's client\nand any other client that wants to implement the full protocol at large\nwould now need to understand not only pkt-line but also ship a JSON\ndecoder etc.\n\nI don't see an inherent problem with us wanting to support some nested\nencoding format as part of the protocol, but JSON seems like a\nparticularly bad choice. It's specified as UTF-8 only (or rather, \"a\nUnicode enoding\"), so you can't stick both valid UTF-8 and binary data\ninto it.\n\nOur refs on the other hand don't conform to that, so having a JSON\nformat means you can never have something that refers to refnames, which\ngiven that we're talking about bundles, whose own header already has\nthat information.\n\n> Let me know if you are interested in switching your approach to something\n> more like what I propose here. There are many more questions about what\n> information could/should be located in the table of contents and how it can\n> be extended in the future. I'm interested to explore that space with you.\n\nAs noted above, the TOC part of this seems interesting, and I don't see\na reason not to implement that.\n\nBut as noted in [1] I really don't see why it would be a good idea to\nimplement a side-format that's encoding a limited subset of what you'd\nfind in bundle headers.\n\nSpecifically on the meta-information you're proposing:\n\n== requires\n\nIn your example you've added a monolithic \"requires\" relationship\nbetween bundles, saying \"This assumes that the bundles can be ordered\".\n\nBut that's not something you can assume for actual bundle files,\ni.e. the prerequisite relationship is per-reftip, it's not the case that\na given bundle requires another bundle, it's the case that tips found in\nthem may or may not depend on other prerequisites.\n\nIf you're creating bundles that contain only one tip there's a 1=1\nmapping to what you're proposing with \"requires\", but otherwise there\nisn't.\n\n== timestamp\n\n\"This allows us to reexamine the table of contents and only download the\nbundles that are newer than that timestamp.\"\n\nWe're usually going to be fetching these over http(s), why duplicate\nwhat you can already get if the server just takes care to create unique\nfilenames (e.g. as a function of the SHA of their contents), and then\nprovides appropriate caching headers to a client so that they'll be\ncached forever?\n\nI think that gives you everything you'd like out of the \"timestamp\" and\nmore, the \"more\" being that since it's part of a protocol that's already\nstandard you'd have e.g. intermediate caching proxies understanding this\nimplicitly, in addition to the git client itself.\n\nSo on a network that's say locally unpacking https connections to a\npublic CDN you could have a local caching proxy for your N local\nclients, as opposed to a custom \"timestamp\" value, which only each local\ngit client will understand.\n\n== Generally\n\nSorry, I've got to run, so I haven't addressed all the things you\nbrought up, but generally while I think that the TOC idea is a good one.\n\nI don't see a reason for why most/all of the other bits shouldn't be\nleaning into either the bundle header (and for any TOC shortcut, dump it\nas-is, as noted in [1]), or in the case of \"timestamp\" lean into the\nproperties of the transport protocol.\n\nAnd just generally on overall protocol complexity, wouldn't it be OK if\nany such TOC is just in pkt-line format?\n\nWe could just provide a git plumbing tool to spew that out, and having\nsome static server job call that once and ever more serve up a a\nplain-file doesn't seem like a big restriction, and would mean that any\ngit client code wouldn't need to deal with another encoding format.\n\n1. https://lore.kernel.org/git/211027.86a6iuxk3x.gmgdl@evledraar.gmail.com/\n"},{"id":"440124","messageId":"c46db4a8-c705-6c6c-bcb3-b2d294120dce@iee.email","threadId":"56782","inReplyTo":"211027.86ilxixoxz.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-10-30T14:51:19Z","receivedAt":"2021-10-30T14:51:25Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"An aside:\n\nOn 27/10/2021 19:01, Ævar Arnfjörð Bjarmason wrote:\n> I don't think you can say \"this bundle has no blobs\". The\n> \"prerequisites\" hard map to the same thing you could put on a\n> \"want/have\" line during PACK negotiation.\n>\n> I think we could/should fix that, i.e. we can \n\n> bump the bundle format\n> version and have it encode some extended prerequisites/filter/shallow\n> etc information.\nIf the format is bumped, could we also include the\nHEAD= info within that format.\nThe `guess the HEAD` algorithm isn't ideal and shows up in user\nquestions every now and again.\n\n> You'd then have a 1=1 match between the features of\n> git-upload-pack and what you can transfer via the bundle side-channel.\n--\nPhilip\n"},{"id":"440271","messageId":"05262630-2e53-3edf-8fa1-fd1a41cc2d28@gmail.com","threadId":"56782","inReplyTo":"211030.86mtmr3po0.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/3] bundle-uri: \"dumb\" static CDN offloading, spec & server implementation","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-01T21:00:55Z","receivedAt":"2021-11-01T21:01:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/30/2021 3:21 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Oct 29 2021, Derrick Stolee wrote:\n> \n>> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>>> This implements a new \"bundle-uri\" protocol v2 extension, which allows\n>>> servers to advertise *.bundle files which clients can pre-seed their\n>>> full \"clone\"'s or incremental \"fetch\"'s from.\n>>>\n>>> This is both an alternative to, and complimentary to the existing\n>>> \"packfile-uri\" mechanism, i.e. servers and/or clients can pick one or\n>>> both, but would generally pick one over the other.\n>>>\n>>> This \"bundle-uri\" mechanism has the advantage of being dumber, and\n>>> offloads more complexity from the server side to the client\n>>> side.\n>>\n>> Generally, I like that using bundles presents an easier way to serve\n>> static content from an alternative source and then let Git's fetch\n>> negotiation catch up with the remainder.\n>>\n>> However, after inspecting your design and talking to some GitHub\n>> engineers who know more about CDNs and general internet things than I\n>> do, I want to propose an alternative design. I think this new design\n>> is simultaneously more flexible as well as promotes further decoupling\n>> of the origin Git server and the bundle contents.\n>>\n>> Your proposed design extends protocol v2 to let the client request a\n>> list of bundle URIs from the origin server. However, this still requires\n>> the origin server to know about this list. [...]\n> \n> Interesting, more below...\n> \n>> Further, your implementation focuses on the server side without\n>> integrating with the client.\n> \n> Do you mean these 3 patches we're discussing now? Yes, that's the\n> server-side and protocol specification only, because I figured talking\n> about just the spec might be helpful.\n> \n> But as noted in the CL and previously on-list I have a larger set of\n> patches to implement the client behavior, an old RFC version of that\n> here (I've since changed some things...):\n> https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n> \n> I mean, you commented on those too, so I'm not sure if that's what you\n> meant, but just for context...\n\nYeah, I'm not able to keep all of that in my head, and I focused on\nwhat you presented in this thread.\n\n>> I propose that we flip this around. The \"bundle server\" should know\n>> which bundles are available at which URIs, and the client should contact\n>> the bundle server directly for a \"table of contents\" that lists these\n>> URIs, along with metadata related to each URI. The origin Git server\n>> then would only need to store the list of bundle servers and the URIs\n>> to their table of contents. The client could then pick from among those\n>> bundle servers (probably by ping time, or randomly) to start the bundle\n>> downloads.\n> \n> I hadn't considered the server not advertising the list, but pointing to\n> another URI that has the list. I was thinking that the server would be\n> close enough to whatever's generating the list that updating the list\n> there wouldn't be a meaningful limitation for anyone.\n> \n> But you seem to have a use-case for it, I'd be curious to hear why\n> specifically, but in any case that's easy to support in the client\n> patches I have.\n\nShow me the client patches and then I can determine if I think that\nis sufficiently flexible.\n\nIn general, I want to expand the scope of this feature beyond \"bundles\non a CDN\" and towards \"alternative sources of Git object data\" which\n_could_ be a CDN, but could also be geodistributed HTTP servers that\nmanage their own copy of the Git data (periodically fetching from the\norigin). These could be self-hosted by organizations with needs for\nlow-latency, high-throughput downloads of object data.\n\nFor a concrete example, a group with a build farm could create their\nown bundle server on the same LAN as the build machines, and they could\nmirror whatever Git service they want. This requires the users setting\nup the server and telling the machines about the URL for the table of\ncontents at clone/fetch time.\n\nBy having the origin Git server advertise the table of contents, hosts\nsuch as GitHub, GitLab, and others could have their own CDN solutions\nthat clients discover automatically. This is clearly the environment\nyou are targeting. Allowing a redirection to another table of contents\nfurther decouples what is responsible for the bundle organization away\nfrom the origin Git server.\n\nOne thing neither of us have touched is authentication, so we'll want\nto find out how to access private information securely in this model.\nAuthentication doesn't matter for CDNs, but would matter for other\nhosting models.\n\n> There's a point at which we get the list of URIs from the server, to\n> support your case the client would just advertise the one TOC URI.\n> \n> Then similarly to the \"packfile-uri\" special-case of handling a *.bundle\n> instead of a PACK that I noted in [1], the downloader would just spot\n> \"oh this isn't a bundle, but list of URIs, and then fetch those (even\n> recursively), and eventually get to *.bundle files.\n\nThis recursive \"follow the contents\" approach seems to be a nice\ngeneral approach. I would still want to have some kind of specification\nabout what could be seen at these URIs before modifying the protocol v2\nspecification.\n\n>> To summarize, there are two pieces here, that can be implemented at\n>> different times:\n>>\n>> 1. Create a specification for a \"bundle server\" that doesn't need to\n>> speak the Git protocol at all. This could be a REST API specification\n>> using well-established standards such as JSON for the table of\n>> contents.\n>>\n>> 2. Create a way for the origin Git server to advertise known bundle\n>> servers to clients so they can automatically benefit from faster\n>> downloads without needing to know about bundle servers.\n>>\n>> There are a few key benefits to this approach:\n>>\n>> * Further decoupling. The origin Git server doesn't need to know how\n>> the bundle server organizes its bundles. This allows maximum flexibility\n>> depending on whether the bundles are stored in something like a CDN\n>> (where bundles can't be too big) or some kind of blob storage (where\n>> they can have arbitrarily large size).\n>>\n>> * The bundle servers could be run completely independently from the\n>> origin Git server. Organizations could run their own bundle servers to\n>> host data in the same building as their build farms. As long as they\n>> can configure the bundle location at clone/fetch time, the origin Git\n>> server doesn't need to be involved.\n>>\n>> While I didn't go so far as to create a clear standard or implement a\n>> prototype in the Git codebase, I created a very simple prototype [1] using\n>> a python script that parses a JSON table of contents and downloads\n>> bundles into the Git repository. Then, I made a 'clone.sh' script that\n>> initializes a repository using the bundle fetcher and fetching the\n>> remainder from the origin Git server. I even computed static bundles for\n>> the git.git repository based on where 'master' has been over several days\n>> in the past month, to give an example of incremental bundles. You can\n>> test the approach all the way to including the fetch to github.com (note\n>> how the GitHub servers were not modified in any way for this).\n>>\n>> [1] https://github.com/derrickstolee/bundles\n>>\n>> There are a lot of limitations to the prototype, but it hopefully\n>> demonstrates the possibility of using something other than the Git protocol\n>> to solve these problems.\n> \n> In your proposal the TOC bundle itself doesn't need to speak the git\n> protocol.\n\nYes. I see this as a HUGE opportunity for flexibility.\n\n> But as as soon as we specify such a thing all of that becomes a part of\n> the git protocol at large in any meaningful way, i.e. git.git's client\n> and any other client that wants to implement the full protocol at large\n> would now need to understand not only pkt-line but also ship a JSON\n> decoder etc.\n\nI use JSON as my example because it is easy to implement in other\nlanguages. It's easy to use in practically any other language than C.\n\nThe reason to use something like JSON is that it already encodes a way\nto include optional, structured information in the list of results. I\nfocused on using that instead of specifics of a pkt-line protocol.\n\nYou have some flexibility in your protocol, but it's unclear how the\noptional data would work without careful testing and actually building\na way to store and communicate the optional data.\n\n> I don't see an inherent problem with us wanting to support some nested\n> encoding format as part of the protocol, but JSON seems like a\n> particularly bad choice. It's specified as UTF-8 only (or rather, \"a\n> Unicode enoding\"), so you can't stick both valid UTF-8 and binary data\n> into it.\n> \n> Our refs on the other hand don't conform to that, so having a JSON\n> format means you can never have something that refers to refnames, which\n> given that we're talking about bundles, whose own header already has\n> that information.\n\nAs I imagine it, we won't want the bundles to store real refnames,\nanyway, since we just need pointers into the commit history to start\nthe incremental fetch of the real refs after the bundles are downloaded.\nThe prototype I made doesn't even store the bundled refs in \"refs/heads\"\nor \"refs/remotes\".\n\n>> Let me know if you are interested in switching your approach to something\n>> more like what I propose here. There are many more questions about what\n>> information could/should be located in the table of contents and how it can\n>> be extended in the future. I'm interested to explore that space with you.\n> \n> As noted above, the TOC part of this seems interesting, and I don't see\n> a reason not to implement that.\n> \n> But as noted in [1] I really don't see why it would be a good idea to\n> implement a side-format that's encoding a limited subset of what you'd\n> find in bundle headers.\n\nYou say \"bundle headers\" a lot as if we are supposed to download and\nexamine the start of the bundle file before completing the download.\nIs that what you mean? Or, do you mean that somehow you are communicating\nthe bundle header in your list of URIs and I just missed it?\n\nIf you do mean that the bundle header _must_ be included in the table\nof contents, then I think that is a blocker for scaling this approach,\nsince the bundle header includes the 'prerequisite' and 'reference'\nrecords, which could be substantial. At scale, info/refs already takes\ntoo much data to want to do often, let alone multiple times in a single\nrequest.\n\nI think the table of contents should provide enough information for the\nclient to decide if they should initiate the download at all, but be\nflexible to multiple strategies for organizing the data.\n\n> Specifically on the meta-information you're proposing:\n> \n> == requires\n> == timestamp\n\nPart of the point of my proposal was to show how things can work in a\ndifferent way, especially with a flexible format like JSON. One possible\nway to organize bundles is in a linear list of files that form snapshots\nof the history at a given timestamp, storing the new objects since the\nprevious bundle. A client could store a local timestamp as a heuristic\nfor whether they need to download a bundle, but if they are missing\nreachable objects, then the 'requires' tag gives them a bundle that\ncould fill in those gaps (they might need to follow a list until closing\nall the gaps, depending on many factors).\n\nSo, that's the high-level \"one could organize bundles like this\" plan.\nIt is _not_ intended as the only way to do it, but I also believe it is\nthe most efficient. It's also why things like \"requires\" and \"timestamp\"\nare intended to be optional metadata.\n\n> == requires\n> In your example you've added a monolithic \"requires\" relationship\n> between bundles, saying \"This assumes that the bundles can be ordered\".\n> \n> But that's not something you can assume for actual bundle files,\n> i.e. the prerequisite relationship is per-reftip, it's not the case that\n> a given bundle requires another bundle, it's the case that tips found in\n> them may or may not depend on other prerequisites.\n\nNo, not all lists of bundles can be ordered. For instance, if you\nuse the \"here is where the 'main' branch was for each month, minus\nthe previous month\" as one list of bundles and then another \"here is\nwhere the 'dev' branch was for each month, minus the 'dev' for the\nprevious month and the 'main' branch for the current month\" then you\nget a partial order instead.\n\nI agree that it is important to allow for full generality of cases like\nthis, but for large repositories, users will probably just want a\nsnapshot of all the ref tips.\n\n> If you're creating bundles that contain only one tip there's a 1=1\n> mapping to what you're proposing with \"requires\", but otherwise there\n> isn't.\n> \n> \n> \"This allows us to reexamine the table of contents and only download the\n> bundles that are newer than that timestamp.\"\n> \n> We're usually going to be fetching these over http(s), why duplicate\n> what you can already get if the server just takes care to create unique\n> filenames (e.g. as a function of the SHA of their contents), and then\n> provides appropriate caching headers to a client so that they'll be\n> cached forever?\n\nThis assumes that a bundle URI will always be available forever, and that\nthe table of contents will never shift with any future reorganization.\nIf the snapshot layout that I specified was always additive, then the URI\nwould be sufficient (although we would need to keep a full list of every\nURI ever downloaded) but also a single timestamp would be sufficient.\n\nThe issue arises if the bundle server reorganizes the data somehow, or\nworse, the back-end of the bundle server is completely replaced with a\ndifferent server that had a different view of the refs at these timestamps.\n\nNow, the 'requires' links provide a way to reconcile missing objects\nafter downloading only the new bundles, without downloading the full list.\n\n> I think that gives you everything you'd like out of the \"timestamp\" and\n> more, the \"more\" being that since it's part of a protocol that's already\n> standard you'd have e.g. intermediate caching proxies understanding this\n> implicitly, in addition to the git client itself.\n> \n> So on a network that's say locally unpacking https connections to a\n> public CDN you could have a local caching proxy for your N local\n> clients, as opposed to a custom \"timestamp\" value, which only each local\n> git client will understand.\n\nI don't understand most of what you are saying here, and perhaps that\nis my lack of understanding of possible network services that you are\nreferencing.\n\nWhat I'm trying to communicate is that a URI is not enough information\nto make a decision about whether or not the Git client should start\ndownloading that data. Providing clues about the bundle content can\nbe helpful.\n\nIn talking with some others about this, they were thinking about\nadvertising the ref tips at the bundle boundaries. This would be a\nlist of \"have\"s and \"have not\"s as OIDs that were used to generate\nthe bundle. However, in my opinion, this only works if you are focused\non snapshots of a single ref instead of a large set of moving refs\n(think thousands of refs). In that environment, timestamps are rather\neffective so it's nice to have the option.\n\nI'm also not saying that you need to implement an organization such\nas the one I'm proposing. I am only strongly recommending that you\nbuild it with enough generality that it is possible.\n\n> == Generally\n> \n> Sorry, I've got to run, so I haven't addressed all the things you\n> brought up, but generally while I think that the TOC idea is a good one.\n> \n> I don't see a reason for why most/all of the other bits shouldn't be\n> leaning into either the bundle header (and for any TOC shortcut, dump it\n> as-is, as noted in [1]), or in the case of \"timestamp\" lean into the\n> properties of the transport protocol.\n> \n> And just generally on overall protocol complexity, wouldn't it be OK if\n> any such TOC is just in pkt-line format?\n\nThe complexity either lives in the code that parses a well-known format\nor the design of a format that is sufficiently general to handle extra\nmetadata that we could use for the future. Pick your poison.\n\n> We could just provide a git plumbing tool to spew that out, and having\n> some static server job call that once and ever more serve up a a\n> plain-file doesn't seem like a big restriction, and would mean that any\n> git client code wouldn't need to deal with another encoding format.\n\nI agree that it would be good to have a Git command that prepares bundles\nfor publishing on a static webserver, complete with table of contents.\nIt would be especially good if this incrementally updated the table with\nnew bundles as time goes on.\n\nThanks,\n-Stolee\n"},{"id":"440280","messageId":"211102.8635of2zet.gmgdl@evledraar.gmail.com","threadId":"56782","inReplyTo":"05262630-2e53-3edf-8fa1-fd1a41cc2d28@gmail.com","subject":"Re: [PATCH 0/3] bundle-uri: \"dumb\" static CDN offloading, spec & server implementation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-01T23:18:21Z","receivedAt":"2021-11-02T00:07:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 01 2021, Derrick Stolee wrote:\n\n> On 10/30/2021 3:21 AM, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Fri, Oct 29 2021, Derrick Stolee wrote:\n>> \n>>> On 10/25/2021 5:25 PM, Ævar Arnfjörð Bjarmason wrote:\n>>>> This implements a new \"bundle-uri\" protocol v2 extension, which allows\n>>>> servers to advertise *.bundle files which clients can pre-seed their\n>>>> full \"clone\"'s or incremental \"fetch\"'s from.\n>>>>\n>>>> This is both an alternative to, and complimentary to the existing\n>>>> \"packfile-uri\" mechanism, i.e. servers and/or clients can pick one or\n>>>> both, but would generally pick one over the other.\n>>>>\n>>>> This \"bundle-uri\" mechanism has the advantage of being dumber, and\n>>>> offloads more complexity from the server side to the client\n>>>> side.\n>>>\n>>> Generally, I like that using bundles presents an easier way to serve\n>>> static content from an alternative source and then let Git's fetch\n>>> negotiation catch up with the remainder.\n>>>\n>>> However, after inspecting your design and talking to some GitHub\n>>> engineers who know more about CDNs and general internet things than I\n>>> do, I want to propose an alternative design. I think this new design\n>>> is simultaneously more flexible as well as promotes further decoupling\n>>> of the origin Git server and the bundle contents.\n>>>\n>>> Your proposed design extends protocol v2 to let the client request a\n>>> list of bundle URIs from the origin server. However, this still requires\n>>> the origin server to know about this list. [...]\n>> \n>> Interesting, more below...\n>> \n>>> Further, your implementation focuses on the server side without\n>>> integrating with the client.\n>> \n>> Do you mean these 3 patches we're discussing now? Yes, that's the\n>> server-side and protocol specification only, because I figured talking\n>> about just the spec might be helpful.\n>> \n>> But as noted in the CL and previously on-list I have a larger set of\n>> patches to implement the client behavior, an old RFC version of that\n>> here (I've since changed some things...):\n>> https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n>> \n>> I mean, you commented on those too, so I'm not sure if that's what you\n>> meant, but just for context...\n>\n> Yeah, I'm not able to keep all of that in my head, and I focused on\n> what you presented in this thread.\n\n[...]\n\n>>> I propose that we flip this around. The \"bundle server\" should know\n>>> which bundles are available at which URIs, and the client should contact\n>>> the bundle server directly for a \"table of contents\" that lists these\n>>> URIs, along with metadata related to each URI. The origin Git server\n>>> then would only need to store the list of bundle servers and the URIs\n>>> to their table of contents. The client could then pick from among those\n>>> bundle servers (probably by ping time, or randomly) to start the bundle\n>>> downloads.\n>> \n>> I hadn't considered the server not advertising the list, but pointing to\n>> another URI that has the list. I was thinking that the server would be\n>> close enough to whatever's generating the list that updating the list\n>> there wouldn't be a meaningful limitation for anyone.\n>> \n>> But you seem to have a use-case for it, I'd be curious to hear why\n>> specifically, but in any case that's easy to support in the client\n>> patches I have.\n>\n> Show me the client patches and then I can determine if I think that\n> is sufficiently flexible.\n\nI'm working on that larger re-roll, hoping to have something to submit\nby the end of the week. I've snipped out much of the below, to the\nextent that it's probably better on my part to reply to it with working\ncode than just continued discussion..\n\n> In general, I want to expand the scope of this feature beyond \"bundles\n> on a CDN\" and towards \"alternative sources of Git object data\" which\n> _could_ be a CDN, but could also be geodistributed HTTP servers that\n> manage their own copy of the Git data (periodically fetching from the\n> origin). These could be self-hosted by organizations with needs for\n> low-latency, high-throughput downloads of object data.\n\n*Nod*. FWIW I share those goal[...]\n\n> One thing neither of us have touched is authentication, so we'll want\n> to find out how to access private information securely in this model.\n> Authentication doesn't matter for CDNs, but would matter for other\n> hosting models.\n\nWe've got authentication already for packfile-uri in the form of bearer\ntokens, and I was planning to do the same (there's recent related\npatches on-list to strip out those URLs during logging for that reason).\n\nI was planning to handle it the same way, i.e. a server implementation\nwould be responsible for spewing out a bundle-uri that's\nauthenticated/non-public.\n\nIn practice I think that probably something that just allows generating\nthe bundle-uri list via a hook would be flexible enough for both this &\nsome of the other use-cases you've had in mind, as long as we pass the\nprotocol-specific auth info down to it in some way.\n\n>> There's a point at which we get the list of URIs from the server, to\n>> support your case the client would just advertise the one TOC URI.\n>> \n>> Then similarly to the \"packfile-uri\" special-case of handling a *.bundle\n>> instead of a PACK that I noted in [1], the downloader would just spot\n>> \"oh this isn't a bundle, but list of URIs, and then fetch those (even\n>> recursively), and eventually get to *.bundle files.\n>\n> This recursive \"follow the contents\" approach seems to be a nice\n> general approach. I would still want to have some kind of specification\n> about what could be seen at these URIs before modifying the protocol v2\n> specification.\n\n*nod*, of course.\n\n> [...]\n> Yes. I see this as a HUGE opportunity for flexibility.\n>\n>> But as as soon as we specify such a thing all of that becomes a part of\n>> the git protocol at large in any meaningful way, i.e. git.git's client\n>> and any other client that wants to implement the full protocol at large\n>> would now need to understand not only pkt-line but also ship a JSON\n>> decoder etc.\n>\n> I use JSON as my example because it is easy to implement in other\n> languages. It's easy to use in practically any other language than C.\n>\n> The reason to use something like JSON is that it already encodes a way\n> to include optional, structured information in the list of results. I\n> focused on using that instead of specifics of a pkt-line protocol.\n>\n> You have some flexibility in your protocol, but it's unclear how the\n> optional data would work without careful testing and actually building\n> a way to store and communicate the optional data.\n\nYes, it's definitely not as out-of-the-box extensible as a nested\nstructure like JSON, but if we leave in space for arbitrary key-values\n(which I'm planning) then we should be able to extend it in the future,\na key/value could even be a serialized data of some sort...\n\n>> I don't see an inherent problem with us wanting to support some nested\n>> encoding format as part of the protocol, but JSON seems like a\n>> particularly bad choice. It's specified as UTF-8 only (or rather, \"a\n>> Unicode enoding\"), so you can't stick both valid UTF-8 and binary data\n>> into it.\n>> \n>> Our refs on the other hand don't conform to that, so having a JSON\n>> format means you can never have something that refers to refnames, which\n>> given that we're talking about bundles, whose own header already has\n>> that information.\n>\n> As I imagine it, we won't want the bundles to store real refnames,\n> anyway, since we just need pointers into the commit history to start\n> the incremental fetch of the real refs after the bundles are downloaded.\n> The prototype I made doesn't even store the bundled refs in \"refs/heads\"\n> or \"refs/remotes\".\n\n*nod*, FWIW I've got off-list patches to make that use-case much easier\nwith \"git bundle\", i.e. teaching it to take \\t\\n... on\n--stdin, not just \\n..., so you can pick arbitrary names.\n\n>>> Let me know if you are interested in switching your approach to something\n>>> more like what I propose here. There are many more questions about what\n>>> information could/should be located in the table of contents and how it can\n>>> be extended in the future. I'm interested to explore that space with you.\n>> \n>> As noted above, the TOC part of this seems interesting, and I don't see\n>> a reason not to implement that.\n>> \n>> But as noted in [1] I really don't see why it would be a good idea to\n>> implement a side-format that's encoding a limited subset of what you'd\n>> find in bundle headers.\n>\n> You say \"bundle headers\" a lot as if we are supposed to download and\n> examine the start of the bundle file before completing the download.\n> Is that what you mean? Or, do you mean that somehow you are communicating\n> the bundle header in your list of URIs and I just missed it?\n\nIt's something I've changed my mind on mid-way through this RFC, but\nyes, as described in\nhttps://lore.kernel.org/git/211027.86a6iuxk3x.gmgdl@evledraar.gmail.com/\nI was originally thinking of a design like\n\n client: bundle-uri\n server: https://example.com/bundle2.bdl\n server: https://example.com/bundle1.bdl optional key=values\n\nBut am now thinking of/working on something like:\n\n client: command=bundle-uri\n client: v3\n client: object-format\n client: want=headers \n server: https://example.com/bundle2.bdl \n server: # v3 git bundle\n server: @object-format=sha1\n server: e9e5ba39a78c8f5057262d49e261b42a8660d5b9 refs/heads/master \n server: https://example.com/bundle1.bdl [...]\n\nI.e. a client requests bundle-uris with N arguments, those communicate\nwhat sort of bundles it's able to understand, and that it would like the\nserver to give it headers as a shortcut, if it can.\n\nThe server replies with a list of URIs to bundles (or TOC's etc.), and\noptionally as a shortcut to the client includes the headers of those\nbundles.\n\nIt could also simply skip those, but then the client will need to go and\nfetch the headers from the pointed-to resources, or decide it doesn't\nneed to.\n\n> If you do mean that the bundle header _must_ be included in the table\n> of contents, then I think that is a blocker for scaling this approach,\n> since the bundle header includes the 'prerequisite' and 'reference'\n> records, which could be substantial. At scale, info/refs already takes\n> too much data to want to do often, let alone multiple times in a single\n> request.\n\nWe'll see when I submit the updated working patches for this, and in\ncoming up with testcases, but I think that you can attain a sweet spot\nin most repositories by advertising some of your common/recent tips, as\nopposed to a big dump of all the refs.\n\n> [...]\n> Part of the point of my proposal was to show how things can work in a\n> different way, especially with a flexible format like JSON. One possible\n> way to organize bundles is in a linear list of files that form snapshots\n> of the history at a given timestamp, storing the new objects since the\n> previous bundle. A client could store a local timestamp as a heuristic\n> for whether they need to download a bundle, but if they are missing\n> reachable objects, then the 'requires' tag gives them a bundle that\n> could fill in those gaps (they might need to follow a list until closing\n> all the gaps, depending on many factors).\n>\n> So, that's the high-level \"one could organize bundles like this\" plan.\n> It is _not_ intended as the only way to do it, but I also believe it is\n> the most efficient. It's also why things like \"requires\" and \"timestamp\"\n> are intended to be optional metadata.\n\n[more below]\n\n> [...]\n>> If you're creating bundles that contain only one tip there's a 1=1\n>> mapping to what you're proposing with \"requires\", but otherwise there\n>> isn't.\n>> \n>> \n>> \"This allows us to reexamine the table of contents and only download the\n>> bundles that are newer than that timestamp.\"\n>> \n>> We're usually going to be fetching these over http(s), why duplicate\n>> what you can already get if the server just takes care to create unique\n>> filenames (e.g. as a function of the SHA of their contents), and then\n>> provides appropriate caching headers to a client so that they'll be\n>> cached forever?\n>\n> This assumes that a bundle URI will always be available forever, and that\n> the table of contents will never shift with any future reorganization.\n> If the snapshot layout that I specified was always additive, then the URI\n> would be sufficient (although we would need to keep a full list of every\n> URI ever downloaded) but also a single timestamp would be sufficient.\n>\n> The issue arises if the bundle server reorganizes the data somehow, or\n> worse, the back-end of the bundle server is completely replaced with a\n> different server that had a different view of the refs at these timestamps.\n>\n> Now, the 'requires' links provide a way to reconcile missing objects\n> after downloading only the new bundles, without downloading the full list.\n\n[also on this]\n\n>> I think that gives you everything you'd like out of the \"timestamp\" and\n>> more, the \"more\" being that since it's part of a protocol that's already\n>> standard you'd have e.g. intermediate caching proxies understanding this\n>> implicitly, in addition to the git client itself.\n>> \n>> So on a network that's say locally unpacking https connections to a\n>> public CDN you could have a local caching proxy for your N local\n>> clients, as opposed to a custom \"timestamp\" value, which only each local\n>> git client will understand.\n>\n> I don't understand most of what you are saying here, and perhaps that\n> is my lack of understanding of possible network services that you are\n> referencing.\n>\n> What I'm trying to communicate is that a URI is not enough information\n> to make a decision about whether or not the Git client should start\n> downloading that data. Providing clues about the bundle content can\n> be helpful.\n>\n> In talking with some others about this, they were thinking about\n> advertising the ref tips at the bundle boundaries. This would be a\n> list of \"have\"s and \"have not\"s as OIDs that were used to generate\n> the bundle. However, in my opinion, this only works if you are focused\n> on snapshots of a single ref instead of a large set of moving refs\n> (think thousands of refs). In that environment, timestamps are rather\n> effective so it's nice to have the option.\n>\n> I'm also not saying that you need to implement an organization such\n> as the one I'm proposing. I am only strongly recommending that you\n> build it with enough generality that it is possible.\n\nOn the \"This assumes that a bundle URI will always be available forever\"\n& generally on piggy-backing on the HTTP protocol. I mean that you can\njust advertise a:\n\n https://example.com/big-bundle.bdl\n\nI.e. a URL that never changes, and if you serve it up with appropriate\ncaching headers a client may or may not request it, e.g. a \"clone\"\nprobably will every time, but we MAY also just ignore it based on some\nother client heuristic.\n\nBut then let's say you serve up (opaque SHA1s of the content) with\nappropriate Cache-Control headers[1][2]:\n\n https://example.com/18719eddecbdf01d6c4166402d62e178482d83d4.bdl\n https://example.com/9cfaf0ef69c3c3024ff5fe92ba84bf7f6caefa2a.bdl\n\nNow a client only needs to grab those once, and if the server operator\nhas set Cache-Control appropriately we'll only need to request the\nheader for each resource once, e.g. for a use-case of having a \"big\nbundle\" we update monthly, and some weekly updates etc.\n\nOne reason I highly prefer this sort of approach is that it works well\nout of the box with other software.\n\nWant to make your local git clients faster? Just pipe those URLs into\nwget, and if you've got a squid/varnish or other http cache sitting in\nfront of your clients doing that will pre-seed your local cache.\n\nI may also turn out to be wrong, but early tests with\npipelining/streaming the headers are very promising (see e.g. [3] for\nthe API). I.e. for the common case of N bundles on the same CDN you can\npipeline them all & stream them in parallel, and if you don't like what\nyou see in some of them you don't need to continue to download the PACK\npayload itself.\n\n1. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control\n2. https://bitsup.blogspot.com/2016/05/cache-control-immutable.html\n3. https://curl.se/libcurl/c/http2-download.html\n"},{"id":"451138","messageId":"RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com","threadId":"56782","inReplyTo":"cover-0.3-00000000000-20211025T211159Z-avarab@gmail.com","subject":"[RFC PATCH v2 00/13] bundle-uri: a \"dumb CDN\" for git","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T16:24:12Z","receivedAt":"2022-03-11T16:24:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Per recent discussion[1] this is my not-quite-feature-complete version\nof the bundle-uri capability. This was sent to the list in some form\nbeforfe in [2] and [3].\n\nRecently Derrick Stolee has sent an alternate implementation of some\nof the same ideas in [4]. Per [1] we're planning to work together on\ngetting a version of this into git that makes everyone happy, sending\nwhat I've got here is the first step in that.\n\nA high-level summary of the important differences in my approach &\nDerrick's (which I hope I'm summarizing fairly here) is that his\napproach optionally adds a bundle TOC format, that format allows you\nto define topology relationships between bundles to guide a\n(returning) client in what it needs to fetch.\n\nWhereas the idea in this series is to lean entirely on the client\ndownloading bundles & inferring what needs to be done via the\ntip/prereqs listed in the header format of the (existing, not changed\nhere) bundle format.\n\nBoth have pros & cons, I started trying to summarize those, but let's\nleave that for later.\n\nThere's also some high-level \"journey\" differences in the\ntwo. E.g. Derrick implemented the ability to have \"git bundle\" itself\nfetch bundles remotely, I don't have that and instead lean entirely on\nthe protocol and \"fetch\". Those differences really aren't important,\nand we can have our cake & eat it too on that front. I.e. end up with\nsome sensible intersection (or union) of the tooling.\n\nI ran out of time to finish up some of what I had on this topic this\nweek, but figured (especially since I'd promised to get it done this\nweek) to send what I have now for discussion.\n\nThings missing & reader's notes:\n\n * I had some amendmends to the protocol I meant to distill further\n into the protocol docs at [5]. Basically omitting the ability to\n transmit key-values and to have it just be a list of URIs with an\n optional
for each one, which is purely a server-to-client\n aid (i.e. those headers will be what you'll find in the pointed-to\n bundles).\n\n* This series goes up to \"clone\", but I also have incremental fetch\n patches. I ran into an (easily solvable bug) in that & thought it\n was best to omit it for now. It'll be here soon.\n\n Basically for incremental fetch we'll in parallel get the headers\n for remote bundles, and then do an early abort of those downloads\n that we deem that we don't need.\n\n Clever (but all standard & boring) use of HTTP caching headers\n between client & servers then allows the client to not request the\n same thing again and again. I.e. want less server load on your CDN?\n Just have the bundles be unique URLs and set appropriate caching\n headers.\n\n* 13/13 demonstrates some of those ideas at a high level.\n\n* A problem with this implementation (and Derrick's, I believe) is\n that it keeps a server waiting while we twiddle our thumbs and wget\n (or equivalent) the bundle(s) locally. If you e.g. clone\n \"chromium.git\" the server will get tired of waiting, drop the\n connection, and unless the bundle is 100% up-to-date the \"clone\"\n will fail.\n\n The solution to this is to get the bundle headers in parallel, and\n as soon as we've got them present the OIDs in the headers as \"HAVE\"\n to the server, which'll then send us an incremental PACK (and even\n possibly a packfile-uri) for the difference between those bundle(s)\n and what its tips are.\n\n We can then simply disconnect, download/index-pack everything, and\n do a connectivity checkat the end.\n\n This requires some careful error handling, e.g. if the resulting\n repo isn't valid we'll need to retry, and the current negotiation\n API isn't very happy to negotiate on the basis of OIDs we don't have\n in the object store (but the protocol handles it just fine).\n\n So while I have local patches for (some of) that I opted to leave it\n out for now. Once we have incremental fetch it's *mostly* an\n optimization anyway (except in cases of e.g. chromium.git), so for\n the initial protocol etc. discussion it's probably best to leave it\n out.\n\n * This currently fails CI on Windows & 32 bit Linux for the most\n trivial of reasons, need to adjust printf formats to use PRIuMAX\n etc., but I ran out of time, sorry.\n\n1. https://lore.kernel.org/git/ddebc223-1e13-e758-f9b1-d3f23961e459@github.com/\n2. https://lore.kernel.org/git/patch-3.3-64224ec2cba-20211025T211159Z-avarab@gmail.com/\n3. https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/\n4. https://lore.kernel.org/git/pull.1160.git.1645641063.gitgitgadget@gmail.com/\n5. https://lore.kernel.org/git/211027.86ilxixoxz.gmgdl@evledraar.gmail.com/\n\nÆvar Arnfjörð Bjarmason (13):\n protocol v2: add server-side \"bundle-uri\" skeleton\n bundle-uri docs: add design notes\n bundle-uri client: add \"bundle-uri\" parsing + tests\n connect.c: refactor sending of agent & object-format\n bundle-uri client: add minimal NOOP client\n bundle-uri client: add \"git ls-remote-bundle-uri\"\n bundle-uri client: add transfer.injectBundleURI support\n bundle-uri client: add boolean transfer.bundleURI setting\n fetch-pack: add a deref_without_lazy_fetch_extended()\n fetch-pack: move --keep=* option filling to a function\n bundle.h: make \"fd\" version of read_bundle_header() public\n bundle-uri client: support for bundle-uri with \"clone\"\n bundle-uri: make the download program configurable\n\n Documentation/config/transfer.txt | 33 ++\n Documentation/git-ls-remote-bundle-uri.txt | 62 +++\n Documentation/git-ls-remote.txt | 1 +\n Documentation/technical/bundle-uri.txt | 119 ++++++\n Documentation/technical/protocol-v2.txt | 214 +++++++++++\n Makefile | 3 +\n builtin.h | 1 +\n builtin/clone.c | 7 +\n builtin/ls-remote-bundle-uri.c | 90 +++++\n bundle-uri.c | 183 +++++++++\n bundle-uri.h | 29 ++\n bundle.c | 8 +-\n bundle.h | 2 +\n command-list.txt | 1 +\n connect.c | 80 +++-\n fetch-pack.c | 306 ++++++++++++++-\n fetch-pack.h | 6 +\n git.c | 1 +\n remote.h | 4 +\n serve.c | 6 +\n t/helper/test-bundle-uri.c | 83 ++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/lib-t5730-protocol-v2-bundle-uri.sh | 424 +++++++++++++++++++++\n t/t5701-git-serve.sh | 124 +++++-\n t/t5730-protocol-v2-bundle-uri-file.sh | 36 ++\n t/t5731-protocol-v2-bundle-uri-git.sh | 17 +\n t/t5732-protocol-v2-bundle-uri-http.sh | 17 +\n t/t5750-bundle-uri-parse.sh | 153 ++++++++\n transport-helper.c | 13 +\n transport-internal.h | 7 +\n transport.c | 120 ++++++\n transport.h | 22 ++\n 33 files changed, 2141 insertions(+), 33 deletions(-)\n create mode 100644 Documentation/git-ls-remote-bundle-uri.txt\n create mode 100644 Documentation/technical/bundle-uri.txt\n create mode 100644 builtin/ls-remote-bundle-uri.c\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n create mode 100644 t/helper/test-bundle-uri.c\n create mode 100644 t/lib-t5730-protocol-v2-bundle-uri.sh\n create mode 100755 t/t5730-protocol-v2-bundle-uri-file.sh\n create mode 100755 t/t5731-protocol-v2-bundle-uri-git.sh\n create mode 100755 t/t5732-protocol-v2-bundle-uri-http.sh\n create mode 100755 t/t5750-bundle-uri-parse.sh\n\nRange-diff against v1:\n 2: 3ac0539c053 ! 1: 2fc87ce092b protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - protocol v2: specify static seeding of clone/fetch via \"bundle-uri\"\n + protocol v2: add server-side \"bundle-uri\" skeleton\n \n - Add a server-side implementation of a new \"bundle-uri\" command to\n - protocol v2. As discussed in the updated \"protocol-v2.txt\" this will\n - allow conforming clients to optionally seed their initial clones or\n - incremental fetches from URLs containing \"*.bundle\" files created with\n - \"git bundle create\".\n + Add a skeleton server-side implementation of a new \"bundle-uri\"\n + command to protocol v2. This will allow conforming clients to\n + optionally seed their initial clones or incremental fetches from URLs\n + containing \"*.bundle\" files created with \"git bundle create\".\n \n The use-cases are similar to those of the existing \"Packfile URIs\",\n and the two feature can be combined within a single request, but\n \"bundle-uri\" has a few advantages over packfile-uris in some some\n common scenarios, discussed below.\n \n - This change does not give us a working \"bundle-uri\" client. I have\n - those patches as a follow-up, but let's first establish what the\n - protocol for this should be like first. The client implementation will\n - then implement this specification.\n + This change does not give us a working \"bundle-uri\" client, subsequent\n + commits will do that. Let's first establish what the protocol for this\n + should be like first. The client implementation will then implement\n + this specification.\n \n With this change when the uploadpack.bundleURI config is set to a\n URI (or URIs, if set >1 times), advertise a \"bundle-uri\" command. Then\n @@ Commit message\n .gitmodules check in that context. See [6] for the \"ls-refs unborn\"\n feature which modified code in similar areas of the request flow.\n \n + Finally, there's currently a concurrent (submitted after the v1 of\n + this commit, but before the subsequent client parts of this\n + implementation) RFC of a somewhat similar \"bundle-uri\" facility at\n + [7].\n +\n 1. https://lore.kernel.org/git/20111110074330.GA27925@sigill.intra.peff.net/\n 2. https://lore.kernel.org/git/20190514092900.GA11679@sigill.intra.peff.net/\n 3. https://lore.kernel.org/git/YFJWz5yIGng+a16k@coredump.intra.peff.net/\n @@ Commit message\n Merged as 6ee353d42f3 (Merge branch 'jt/transfer-fsck-across-packs',\n 2021-03-01)\n 6. 69571dfe219 (Merge branch 'jt/clone-unborn-head', 2021-02-17)\n + 7. https://lore.kernel.org/git/pull.1160.git.1645641063.gitgitgadget@gmail.com/\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n @@ bundle-uri.h (new)\n @@\n +#ifndef BUNDLE_URI_H\n +#define BUNDLE_URI_H\n -+\n -+struct repository;\n -+struct packet_reader;\n -+struct packet_writer;\n ++#include \"repository.h\"\n ++#include \"pkt-line.h\"\n ++#include \"strbuf.h\"\n +\n +/**\n + * API used by serve.[ch].\n -: ----------- > 2: 84c4036a510 bundle-uri docs: add design notes\n 3: 64224ec2cba ! 3: 3abfb2290fd bundle-uri client: add \"bundle-uri\" parsing + tests\n @@ bundle-uri.c: int bundle_uri_command(struct repository *r,\n \n ## bundle-uri.h ##\n @@\n - struct repository;\n - struct packet_reader;\n - struct packet_writer;\n -+struct string_list;\n + #include \"repository.h\"\n + #include \"pkt-line.h\"\n + #include \"strbuf.h\"\n ++#include \"string-list.h\"\n \n /**\n * API used by serve.[ch].\n -@@ bundle-uri.h: struct packet_writer;\n +@@\n int bundle_uri_advertise(struct repository *r, struct strbuf *value);\n int bundle_uri_command(struct repository *r, struct packet_reader *request);\n \n 1: 7639b9bbac5 ! 4: f64aefa9ece leak tests: mark t5701-git-serve.sh as passing SANITIZE=leak\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - leak tests: mark t5701-git-serve.sh as passing SANITIZE=leak\n + connect.c: refactor sending of agent & object-format\n \n - The \"t5701-git-serve.sh\" test passes when run under a git compiled\n - with SANITIZE=leak, let's mark it as such to add it to the\n - \"linux-leaks\" CI job.\n + Refactor the sending of the \"agent\" and \"object-format\" capabilities\n + into a function.\n +\n + This was added in its current form in ab67235bc4 (connect: parse v2\n + refs with correct hash algorithm, 2020-05-25). When we connect to a v2\n + server we need to know about its object-format, and it needs to know\n + about ours. Since most things in connect.c and transport.c piggy-back\n + on the eager getting of remote refs via the handshake() those commands\n + can make use of the just-sent-over object-format by ls-refs.\n +\n + But I'm about to add a command that may come after ls-refs, and may\n + not, but we need the server to know about our user-agent and\n + object-format. So let's split this into a function.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n - ## t/t5701-git-serve.sh ##\n -@@ t/t5701-git-serve.sh: test_description='test protocol v2 server commands'\n - GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n - export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n + ## connect.c ##\n +@@ connect.c: 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 +@@ connect.c: 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 + \tconst char **unborn_head_target = transport_options ?\n +@@ connect.c: 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 -+TEST_PASSES_SANITIZE_LEAK=true\n - . ./test-lib.sh\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 - test_expect_success 'test capability advertisement' '\n + \tif (server_options && server_options->nr &&\n + \t server_supports_v2(\"server-option\", 1))\n -: ----------- > 5: 105ced66409 bundle-uri client: add minimal NOOP client\n -: ----------- > 6: 617a6b16df8 bundle-uri client: add \"git ls-remote-bundle-uri\"\n -: ----------- > 7: b0ce379528e bundle-uri client: add transfer.injectBundleURI support\n -: ----------- > 8: 44d96a0f5f8 bundle-uri client: add boolean transfer.bundleURI setting\n -: ----------- > 9: d9f5b486511 fetch-pack: add a deref_without_lazy_fetch_extended()\n -: ----------- > 10: 31a22eb3bd4 fetch-pack: move --keep=* option filling to a function\n -: ----------- > 11: 5ade9419454 bundle.h: make \"fd\" version of read_bundle_header() public\n -: ----------- > 12: bb0d681f5f0 bundle-uri client: support for bundle-uri with \"clone\"\n -: ----------- > 13: 40f37c8b9d5 bundle-uri: make the download program configurable\n-- \n2.35.1.1337.g7e32d794afe\n\n"},{"id":"451139","messageId":"RFC-patch-v2-02.13-84c4036a510-20220311T155841Z-avarab@gmail.com","threadId":"56782","inReplyTo":"RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com","subject":"[RFC PATCH v2 02/13] bundle-uri docs: add design notes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T16:24:14Z","receivedAt":"2022-03-11T16:24:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a design doc for the bundle-uri protocol extension to go along\nwith the packfile-uri extension added in cd8402e0fd8 (Documentation:\nadd Packfile URIs design doc, 2020-06-10).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/technical/bundle-uri.txt | 119 ++++++++++++++++++++++++\n Documentation/technical/protocol-v2.txt | 5 +\n 2 files changed, 124 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..5ae9a15eafe\n--- /dev/null\n+++ b/Documentation/technical/bundle-uri.txt\n@@ -0,0 +1,119 @@\n+Bundle URI Design Notes\n+=======================\n+\n+Protocol\n+--------\n+\n+See `bundle-uri` in the link:protocol-v2.html[protocol-v2]\n+documentation for a discussion of the bundle-uri command, and the\n+expectations of clients and servers.\n+\n+This document is a a more general discussion of how the `bundle-uri`\n+command fits in with the rest of the git ecosystem, its design goals\n+and non-goals, comparison to alternatives etc.\n+\n+Comparison with Packfile URIs\n+-----------------------------\n+\n+There is a similar \"Packfile URIs\" facility, see the\n+link:packfile-uri.html[packfile-uri] documentation for details.\n+\n+The Packfile URIs facility requires a much closer cooperation between\n+CDN and server than the bundle URI facility.\n+\n+I.e. the server MUST know what objects exist in the packfile URI it's\n+pointing to, as well as its pack checksum. Failure to do so will not\n+only result in a client error (the packfile hash won't match), but\n+even if it got past that would likely result in a corrupt repository\n+with tips pointing to unreachable objects.\n+\n+By comparison the bundle URIs are meant to be a \"dumb\" solution\n+friendly to e.g. having a weekly cronjob take a snapshot of a git\n+repository, that snapshot being uploaded to a network of FTP mirrors\n+(which may be inconsistent or out of date).\n+\n+The server does not need to know what state the side-channel download\n+is at, because the client will first validate it, and then optionally\n+negotiate with the server using what it discovers there.\n+\n+Using the local `transfer.injectBundleURI` configuration variable (see\n+linkgit:git-config[1]) the `bundle-uri` mechanism doesn't even need\n+the server to support it.\n+\n+Security\n+--------\n+\n+The omission of something equivalent to the packfile in the\n+Packfile URIs protocol is intentional, as having it would require\n+closer server and CDN cooperation than some server operators are\n+comfortable with.\n+\n+Furthermore, it is not needed for security. The server doesn't need to\n+trust its CDN. If the server were to attempt to send harmful content\n+to the client, the result would not validate against the server's\n+provided ref tips gotten from ls-refs.\n+\n+The lack of a such a hash does leave room open to a malicious CDN\n+operation to be annoying however. E.g. they could inject irrelevant\n+objects into the bundles, which would enlarge the downloaded\n+repository until a \"gc\" would eventually throw them away.\n+\n+In practice the lack of a hash is considered to be a non-issue. Anyone\n+concerned about such security problems between their server and their\n+CDN is going to be pointing to a \"https\" URL under their control. For\n+a client the \"threat\" is the same as without bundle-uri, i.e. a server\n+is free to be annoying today and send you garbage in the PACK that you\n+won't need.\n+\n+Security issues peculiar to bundle-uri\n+--------------------------------------\n+\n+Both packfile-uri and bundle-uri use the `fetch.uriProtocols`\n+configuration variable (see linkgit:git-config[1]) to configure which\n+protocols they support.\n+\n+By default this is set to \"http,https\" for both, but bundle-uri\n+supports adding \"file\" to that list. The server can thus point to\n+\"file://\" URIs it expects the client to have access to.\n+\n+This is primarily intended for use with the `transfer.injectBundleURI`\n+mechanism, but can also be useful e.g. in a centralized environment\n+where a server might point to a \"file:///mnt/bundles/big-repo.bdl\" it\n+knows to be mounted on the local machine (e.g. a racked server),\n+points to it in its \"bundle-uri\" response.\n+\n+The client can then add \"file\" to the `fetch.uriProtocols` list to\n+obey such responses. That does mean that a malicious server can point\n+to any arbitrary file on the local machine. The threat of this is\n+considered minimal, since anyone adding `file` to `fetch.uriProtocols`\n+likely knows what they're doing and controls both ands, and the worst\n+they can do is make a curl(1) pipe garbage into \"index-pack\" (which\n+will likely promptly die on the non-PACK-file).\n+\n+Security comparison with packfile-uri\n+-------------------------------------\n+\n+The initial implementation of packfile-uri needed special adjusting to\n+run \"git fsck\" on incoming .gitmodules files, this was to deal with a\n+general security issue in git, See CVE-2018-17456.\n+\n+The current packfile-uri mechanism requires special handling around\n+\"fsck\" to do such cross-PACK fsck's, this is because it first indexes\n+the \"incremental\" PACK, and then any PACK(s) provided via\n+packfile-uri, before finally doing a full connectivity check.\n+\n+This is effect doing the fsck one might do via \"clone\" and \"fetch\" in\n+reverse, or the equivalent of starting with the incremental \"fetch\",\n+followed by the \"clone\".\n+\n+Since the packfile-uri mechanism can result in the .gitmodules blob\n+referenced by such a \"fetch\" to be in the pack for the \"clone\" the\n+fetch-pack process needs to keep state between the indexing of\n+multiple packs, to remember to fsck the blob (via the \"clone\") later\n+after seeing it in a tree (from the \"fetch).\n+\n+There are no known security issues with the way packfile-uri does\n+this, but since bundle-uri effectively emulates what a which doesn't\n+support either \"bundle-uri\" or \"packfile-uri\" would do on clone/fetch,\n+any future security issues peculiar to the packfile-uri approach are\n+unlikely to be shared by it.\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 3ea96add398..3a51492049f 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -775,3 +775,8 @@ A client receiving such a a response MAY assume that they can skip\n retrieving the header from a bundle at the indicated URI, and thus\n save themselves and the server(s) the request(s) needed to inspect the\n headers of that bundle or bundles.\n+\n+bundle-uri SEE ALSO\n+^^^^^^^^^^^^^^^^^^^\n+\n+See the link:bundle-uri.html[Bundle URI Design Notes] for more.\n-- \n2.35.1.1337.g7e32d794afe\n\n"},{"id":"451140","messageId":"RFC-patch-v2-01.13-2fc87ce092b-20220311T155841Z-avarab@gmail.com","threadId":"56782","inReplyTo":"RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com","subject":"[RFC PATCH v2 01/13] protocol v2: add server-side \"bundle-uri\" skeleton","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T16:24:13Z","receivedAt":"2022-03-11T16:24:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a skeleton server-side implementation of a new \"bundle-uri\"\ncommand to protocol v2. This will allow conforming clients to\noptionally seed their initial clones or incremental fetches from URLs\ncontaining \"*.bundle\" files created with \"git bundle create\".\n\nThe use-cases are similar to those of the existing \"Packfile URIs\",\nand the two feature can be combined within a single request, but\n\"bundle-uri\" has a few advantages over packfile-uris in some some\ncommon scenarios, discussed below.\n\nThis change does not give us a working \"bundle-uri\" client, subsequent\ncommits will do that. Let's first establish what the protocol for this\nshould be like first. The client implementation will then implement\nthis specification.\n\nWith this change when the uploadpack.bundleURI config is set to a\nURI (or URIs, if set >1 times), advertise a \"bundle-uri\" command. Then\nwhen the client requests \"bundle-uri\" emit those URIs back at them.\n\nDifferences between this and the existing packfile-uri facility:\n\n A. There is no \"real\" support for packfile-uri in git.git. The\n uploadpack.blobPackfileUri setting allows carving out a list of\n blobs (actually any OIDs), but as alluded to in bfc2a36ff2a (Doc:\n clarify contents of packfile sent as URI, 2021-01-20) the only\n \"real\" implementation is JGit based.\n\n B. The uploadpack.blobPackfileUri is a MUST where this is a\n \"CAN\". I.e. once a client says they support packfile-uri of given\n list of protocols the server will send them a PACK response\n assuming they've downloaded the URI they client was sent, if the\n client doesn't do that they don't have a valid repository.\n\n Pointing at a bundle and having the client send us \"have\"\n lines (or not, maybe they couldn't fetch it, or decided they\n didn't want to) is more flexible, and can gracefully recover\n e.g. if the CDN isn't reachable (maybe you do support \"https\", but\n the CDN provider is down, or blocked your whole country).\n\n C. The client, after executing \"ls-refs\" will disconnect if it has\n also grabbed the \"bundle-uris\" and knows the server won't send it\n anything it doesn't already have (or expect to have, if it's\n downloading the bundles concurrent to an early disconnect).\n\n This is in (small) contrast to packfile-uri where a client would\n enter a negotiation dialog, which may or may not result in a\n packfile-uri and/or an inline PACK.\n\n D. Because of \"C\" clients can, if the bundles are up-to-date, get an\n up-to-date repository with just \"bundle-uri\" and \"ls-refs\" commands,\n with no need to enter a dialog with \"git upload-pack\".\n\n That small dialog is unlikely to matter for performance purposes,\n this section is just noting differences between \"bundle-uri\" and\n \"packfile-uri\".\n\nAs noted above the features are compatible, a client that supports\n\"bundle-uri\" and \"packfile-uri\" might download a bundle, and then\nproceed with a \"fetch\" dialog, that dialog might then result in\n\"packfile-uri\" response.\n\nIn practice server operators are unlikely to want to mix the two,\nsince the main benefit of either approach is the ability to offload\nlarge \"clone\" responses to CDNs. A server operator would have little\nreason not to go with one approach or the other.\n\nThere was a suggestion of implementing a similar feature long ago[1]\nby Jeff King. The main difference between it and this approach is that\nwe've since gained protocol v2, so we can add this as an optional path\nin the dialog between client and server. The 2011 implementation\nhooked into the transport mechanism to try to clone from a bundle\ndirectly. See also [2] and [3] for some later mentions of that\napproach.\n\nSee also [4] for the series that implemented\nuploadpack.blobPackfileUri, and [5] for a series on top that did the\n.gitmodules check in that context. See [6] for the \"ls-refs unborn\"\nfeature which modified code in similar areas of the request flow.\n\nFinally, there's currently a concurrent (submitted after the v1 of\nthis commit, but before the subsequent client parts of this\nimplementation) RFC of a somewhat similar \"bundle-uri\" facility at\n[7].\n\n1. https://lore.kernel.org/git/20111110074330.GA27925@sigill.intra.peff.net/\n2. https://lore.kernel.org/git/20190514092900.GA11679@sigill.intra.peff.net/\n3. https://lore.kernel.org/git/YFJWz5yIGng+a16k@coredump.intra.peff.net/\n4. https://lore.kernel.org/git/cover.1591821067.git.jonathantanmy@google.com/\n Merged as 34e849b05a4 (Merge branch 'jt/cdn-offload', 2020-06-25)\n5. https://lore.kernel.org/git/cover.1614021092.git.jonathantanmy@google.com/\n Merged as 6ee353d42f3 (Merge branch 'jt/transfer-fsck-across-packs',\n 2021-03-01)\n6. 69571dfe219 (Merge branch 'jt/clone-unborn-head', 2021-02-17)\n7. https://lore.kernel.org/git/pull.1160.git.1645641063.gitgitgadget@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/technical/protocol-v2.txt | 209 ++++++++++++++++++++++++\n Makefile | 1 +\n bundle-uri.c | 55 +++++++\n bundle-uri.h | 13 ++\n serve.c | 6 +\n t/t5701-git-serve.sh | 124 +++++++++++++-\n 6 files changed, 407 insertions(+), 1 deletion(-)\n create mode 100644 bundle-uri.c\n create mode 100644 bundle-uri.h\n\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 8a877d27e23..3ea96add398 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -566,3 +566,212 @@ and associated requested information, each separated by a single space.\n \tattr = \"size\"\n \n \tobj-info = obj-id SP obj-size\n+\n+bundle-uri\n+~~~~~~~~~~\n+\n+If the 'bundle-uri' capability is advertised, the server supports the\n+`bundle-uri' command.\n+\n+The capability is currently advertised with no value (i.e. not\n+\"bundle-uri=somevalue\"), a value may be added in the future for\n+supporting command-wide extensions. Clients MUST ignore any unknown\n+capability values and proceed with the 'bundle-uri` dialog they\n+support.\n+\n+The 'bundle-uri' command is intended to be issued before `fetch` to\n+get URIs to bundle files (see linkgit:git-bundle[1]) to \"seed\" and\n+inform the subsequent `fetch` command.\n+\n+The client CAN issue `bundle-uri` before or after any other valid\n+command. To be useful to clients it's expected that it'll be issued\n+after an `ls-refs` and before `fetch`, but CAN be issued at any time\n+in the dialog.\n+\n+DISCUSSION of bundle-uri\n+^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+The intent of the feature is optimize for server resource consumption\n+in the common case by changing the common case of fetching a very\n+large PACK during linkgit:git-clone[1] into a smaller incremental\n+fetch.\n+\n+It also allows servers to achieve better caching in combination with\n+an `uploadpack.packObjectsHook` (see linkgit:git-config[1]).\n+\n+By having new clones or fetches be a more predictable and common\n+negotiation against the tips of recently produces *.bundle file(s).\n+Servers might even pre-generate the results of such negotiations for\n+the `uploadpack.packObjectsHook` as new pushes come in.\n+\n+I.e. the server would anticipate that fresh clones will download a\n+known bundle, followed by catching up to the current state of the\n+repository using ref tips found in that bundle (or bundles).\n+\n+PROTOCOL for bundle-uri\n+^^^^^^^^^^^^^^^^^^^^^^^\n+\n+A `bundle-uri` request takes no arguments, and as noted above does not\n+currently advertise a capability value. Both may be added in the\n+future.\n+\n+When the client issues a `command=bundle-uri` the response is a list\n+of URIs the server would like the client to fetch out-of-bounds before\n+proceeding with the `fetch` request in this format:\n+\n+\toutput = bundle-uri-line\n+\t\t bundle-uri-line* flush-pkt\n+\n+\tbundle-uri-line = PKT-LINE(bundle-uri)\n+\t\t\t *(SP bundle-feature-key *(=bundle-feature-val))\n+\t\t\t LF\n+\n+\tbundle-uri = A URI such as a https://, ssh:// etc. URI\n+\n+\tbundle-feature-key = Any printable ASCII characters except SP or \"=\"\n+\tbundle-feature-val = Any printable ASCII characters except SP or \"=\"\n+\n+No `bundle-feature-key`=`bundle-feature-value` fields are currently\n+defined. See the discussion of features below.\n+\n+Clients are still expected to fully parse the line according to the\n+above format, lines that do not conform to the format SHOULD be\n+discarded. The user MAY be warned in such a case.\n+\n+bundle-uri CLIENT AND SERVER EXPECTATIONS\n+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+\".bundle\" FORMAT\n+++++++++++++++++\n+\n+The advertised bundle(s) MUST be in a format that \"git bundle verify\"\n+would accept. I.e. they MUST contain one or more reference tips for\n+use by the client, MUST indicate prerequisites (in any) with standard\n+\"-\" prefixes, and MUST indicate their \"object-format\", if\n+applicable. Create \"*.bundle\" files with \"git bundle create\".\n+\n+bundle-uri CLIENT ERROR RECOVERY\n+++++++++++++++++++++++++++++++++\n+\n+A client MUST above all gracefully degrade on errors, whether that\n+error is because of bad missing/data in the bundle URI(s), because\n+that client is too dumb to e.g. understand and fully parse out bundle\n+headers and their prerequisite relationships, or something else.\n+\n+Server operators should feel confident in turning on \"bundle-uri\" and\n+not worry if e.g. their CDN goes down that clones or fetches will run\n+into hard failures. Even if the server bundle bundle(s) are\n+incomplete, or bad in some way the client should still end up with a\n+functioning repository, just as if it had chosen not to use this\n+protocol extension.\n+\n+All subsequent discussion on client and server interaction MUST keep\n+this in mind.\n+\n+bundle-uri SERVER TO CLIENT\n++++++++++++++++++++++++++++\n+\n+The ordering of the returned bundle uris is not significant. Clients\n+MUST parse their headers to discover their contained OIDS and\n+prerequisites. A client MUST consider the content of the bundle(s)\n+themselves and their header as the ultimate source of truth.\n+\n+A server MAY even return bundle(s) that don't have any direct\n+relationship to the repository being cloned (either through accident,\n+or intentional \"clever\" configuration), and expect a client to sort\n+out what data they'd like from the bundle(s), if any.\n+\n+bundle-uri CLIENT TO SERVER\n++++++++++++++++++++++++++++\n+\n+The client SHOULD provide reference tips found in the bundle header(s)\n+as 'have' lines in any subsequent `fetch` request. A client MAY also\n+ignore the bundle(s) entirely if doing so is deemed worse for some\n+reason, e.g. if the bundles can't be downloaded, it doesn't like the\n+tips it finds etc.\n+\n+WHEN ADVERTISED BUNDLE(S) REQUIRE NO FURTHER NEGOTIATION\n+++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n+\n+If after issuing `bundle-uri` and `ls-refs`, and getting the header(s)\n+of the bundle(s) the client finds that the ref tips it wants can be\n+retrieved entirety from advertised bundle(s), it MAY disconnect. The\n+results of such a 'clone' or 'fetch' should be indistinguishable from\n+the state attained without using bundle-uri.\n+\n+EARLY CLIENT DISCONNECTIONS AND ERROR RECOVERY\n+++++++++++++++++++++++++++++++++++++++++++++++\n+\n+A client MAY perform an early disconnect while still downloading the\n+bundle(s) (having streamed and parsed their headers). In such a case\n+the client MUST gracefully recover from any errors related to\n+finishing the download and validation of the bundle(s).\n+\n+I.e. a client might need to re-connect and issue a 'fetch' command,\n+and possibly fall back to not making use of 'bundle-uri' at all.\n+\n+This \"MAY\" behavior is specified as such (and not a \"SHOULD\") on the\n+assumption that a server advertising bundle uris is more likely than\n+not to be serving up a relatively large repository, and to be pointing\n+to URIs that have a good chance of being in working order. A client\n+MAY e.g. look at the payload size of the bundles as a heuristic to see\n+if an early disconnect is worth it, should falling back on a full\n+\"fetch\" dialog be necessary.\n+\n+WHEN ADVERTISED BUNDLE(S) REQUIRE FURTHER NEGOTIATION\n++++++++++++++++++++++++++++++++++++++++++++++++++++++\n+\n+A client SHOULD commence a negotiation of a PACK from the server via\n+the \"fetch\" command using the OID tips found in advertised bundles,\n+even if's still in the process of downloading those bundle(s).\n+\n+This allows for aggressive early disconnects from any interactive\n+server dialog. The client blindly trusts that the advertised OID tips\n+are relevant, and issues them as 'have' lines, it then requests any\n+tips it would like (usually from the \"ls-refs\" advertisement) via\n+'want' lines. The server will then compute a (hopefully small) PACK\n+with the expected difference between the tips from the bundle(s) and\n+the data requested.\n+\n+The only connection the client then needs to keep active is to the\n+concurrently downloading static bundle(s), when those and the\n+incremental PACK are retrieved they should be inflated and\n+validated. Any errors at this point should be gracefully recovered\n+from, see above.\n+\n+bundle-uri PROTOCOL FEATURES\n+^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+As noted above no `bundle-feature-key`=`bundle-feature-value` fields\n+are currently defined.\n+\n+They are intended for future per-URI metadata which older clients MUST\n+ignore and gracefully degrade on. Any fields they do recognize they\n+CAN also ignore.\n+\n+Any backwards-incompatible addition of pre-URI key-value will be\n+guarded by a new value or values in 'bundle-uri' capability\n+advertisement itself, and/or by new future `bundle-uri` request\n+arguments.\n+\n+While no per-URI key-value are currently supported currently they're\n+intended to support future features such as:\n+\n+ * Add a \"hash=\" or \"size=\" advertise the expected hash or\n+ size of the bundle file.\n+\n+ * Advertise that one or more bundle files are the same (to e.g. have\n+ clients round-robin or otherwise choose one of N possible files).\n+\n+ * A \"oid=\" shortcut and \"prerequisite=\" shortcut. For\n+ expressing the common case of a bundle with one tip and no\n+ prerequisites, or one tip and one prerequisite.\n++\n+This would allow for optimizing the common case of servers who'd like\n+to provide one \"big bundle\" containing only their \"main\" branch,\n+and/or incremental updates thereof.\n++\n+A client receiving such a a response MAY assume that they can skip\n+retrieving the header from a bundle at the indicated URI, and thus\n+save themselves and the server(s) the request(s) needed to inspect the\n+headers of that bundle or bundles.\ndiff --git a/Makefile b/Makefile\nindex 6f0b4b775fe..5a3d35109a1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -855,6 +855,7 @@ LIB_OBJS += blob.o\n LIB_OBJS += bloom.o\n LIB_OBJS += branch.o\n LIB_OBJS += bulk-checkin.o\n+LIB_OBJS += bundle-uri.o\n LIB_OBJS += bundle.o\n LIB_OBJS += cache-tree.o\n LIB_OBJS += cbtree.o\ndiff --git a/bundle-uri.c b/bundle-uri.c\nnew file mode 100644\nindex 00000000000..ff054ddc690\n--- /dev/null\n+++ b/bundle-uri.c\n@@ -0,0 +1,55 @@\n+#include \"cache.h\"\n+#include \"bundle-uri.h\"\n+#include \"pkt-line.h\"\n+#include \"config.h\"\n+\n+static void send_bundle_uris(struct packet_writer *writer,\n+\t\t\t struct string_list *uris)\n+{\n+\tstruct string_list_item *item;\n+\n+\tfor_each_string_list_item(item, uris)\n+\t\tpacket_writer_write(writer, \"%s\", item->string);\n+}\n+\n+static int advertise_bundle_uri = -1;\n+static struct string_list bundle_uris = STRING_LIST_INIT_DUP;\n+static int bundle_uri_config(const char *var, const char *value, void *data)\n+{\n+\tif (!strcmp(var, \"uploadpack.bundleuri\")) {\n+\t\tadvertise_bundle_uri = 1;\n+\t\tstring_list_append(&bundle_uris, value);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+int bundle_uri_advertise(struct repository *r, struct strbuf *value)\n+{\n+\tif (advertise_bundle_uri != -1)\n+\t\tgoto cached;\n+\n+\tgit_config(bundle_uri_config, NULL);\n+\tadvertise_bundle_uri = !!bundle_uris.nr;\n+\n+cached:\n+\treturn advertise_bundle_uri;\n+}\n+\n+int bundle_uri_command(struct repository *r,\n+\t\t struct packet_reader *request)\n+{\n+\tstruct packet_writer writer;\n+\tpacket_writer_init(&writer, 1);\n+\n+\twhile (packet_reader_read(request) == PACKET_READ_NORMAL)\n+\t\tdie(_(\"bundle-uri: unexpected argument: '%s'\"), request->line);\n+\tif (request->status != PACKET_READ_FLUSH)\n+\t\tdie(_(\"bundle-uri: expected flush after arguments\"));\n+\n+\tsend_bundle_uris(&writer, &bundle_uris);\n+\n+\tpacket_writer_flush(&writer);\n+\n+\treturn 0;\n+}\ndiff --git a/bundle-uri.h b/bundle-uri.h\nnew file mode 100644\nindex 00000000000..5a7e556a0ba\n--- /dev/null\n+++ b/bundle-uri.h\n@@ -0,0 +1,13 @@\n+#ifndef BUNDLE_URI_H\n+#define BUNDLE_URI_H\n+#include \"repository.h\"\n+#include \"pkt-line.h\"\n+#include \"strbuf.h\"\n+\n+/**\n+ * API used by serve.[ch].\n+ */\n+int bundle_uri_advertise(struct repository *r, struct strbuf *value);\n+int bundle_uri_command(struct repository *r, struct packet_reader *request);\n+\n+#endif /* BUNDLE_URI_H */\ndiff --git a/serve.c b/serve.c\nindex b3fe9b5126a..f3e0203d2c6 100644\n--- a/serve.c\n+++ b/serve.c\n@@ -8,6 +8,7 @@\n #include \"protocol-caps.h\"\n #include \"serve.h\"\n #include \"upload-pack.h\"\n+#include \"bundle-uri.h\"\n \n static int advertise_sid = -1;\n static int client_hash_algo = GIT_HASH_SHA1;\n@@ -136,6 +137,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 = \"bundle-uri\",\n+\t\t.advertise = bundle_uri_advertise,\n+\t\t.command = bundle_uri_command,\n+\t},\n };\n \n void protocol_v2_advertise_capabilities(void)\ndiff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\nindex 1896f671cb3..9d053f77a93 100755\n--- a/t/t5701-git-serve.sh\n+++ b/t/t5701-git-serve.sh\n@@ -13,7 +13,7 @@ test_expect_success 'test capability advertisement' '\n \twrong_algo sha1:sha256\n \twrong_algo sha256:sha1\n \tEOF\n-\tcat >expect <<-EOF &&\n+\tcat >expect.base <<-EOF &&\n \tversion 2\n \tagent=git/$(git version | cut -d\" \" -f3)\n \tls-refs=unborn\n@@ -21,8 +21,11 @@ test_expect_success 'test capability advertisement' '\n \tserver-option\n \tobject-format=$(test_oid algo)\n \tobject-info\n+\tEOF\n+\tcat >expect.trailer <<-EOF &&\n \t0000\n \tEOF\n+\tcat expect.base expect.trailer >expect &&\n \n \tGIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \\\n \t\t--advertise-capabilities >out &&\n@@ -342,4 +345,123 @@ test_expect_success 'basics of object-info' '\n \ttest_cmp expect actual\n '\n \n+# Test the basics of bundle-uri\n+#\n+test_expect_success 'test capability advertisement with uploadpack.bundleURI' '\n+\ttest_config uploadpack.bundleURI FAKE &&\n+\n+\tcat >expect.extra <<-EOF &&\n+\tbundle-uri\n+\tEOF\n+\tcat expect.base \\\n+\t expect.extra \\\n+\t expect.trailer >expect &&\n+\n+\tGIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \\\n+\t\t--advertise-capabilities >out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: dies if not enabled' '\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\t0000\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\tfatal: invalid command '\"'\"'bundle-uri'\"'\"'\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\tERR serve: invalid command '\"'\"'bundle-uri'\"'\"'\n+\tEOF\n+\n+\ttest_must_fail test-tool serve-v2 --stateless-rpc out 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_must_be_empty out\n+'\n+\n+\n+test_expect_success 'basics of bundle-uri: enabled with single URI' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/repo.bdl &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0000\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\thttps://cdn.example.com/repo.bdl\n+\t0000\n+\tEOF\n+\n+\ttest-tool serve-v2 --stateless-rpc out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: enabled with single URI' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/repo.bdl &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0000\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\thttps://cdn.example.com/repo.bdl\n+\t0000\n+\tEOF\n+\n+\ttest-tool serve-v2 --stateless-rpc out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: enabled with two URIs' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/repo.bdl &&\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/recent.bdl --add &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0000\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\thttps://cdn.example.com/repo.bdl\n+\thttps://cdn.example.com/recent.bdl\n+\t0000\n+\tEOF\n+\n+\ttest-tool serve-v2 --stateless-rpc out &&\n+\ttest-tool pkt-line unpack actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'basics of bundle-uri: unknown future feature(s)' '\n+\ttest_config uploadpack.bundleURI https://cdn.example.com/fake.bdl &&\n+\n+\ttest-tool pkt-line pack >in <<-EOF &&\n+\tcommand=bundle-uri\n+\tobject-format=$(test_oid algo)\n+\t0001\n+\tsome-feature\n+\twe-do-not\n+\tknow=about\n+\t0000\n+\tEOF\n+\n+\tcat >err.expect <<-\\EOF &&\n+\tfatal: bundle-uri: unexpected argument: '\"'\"'some-feature'\"'\"'\n+\tEOF\n+\n+\ttest_must_fail test-tool serve-v2 --stateless-rpc out 2>err.actual &&\n+\ttest_cmp err.expect err.actual &&\n+\ttest_must_be_empty out\n+'\n+\n test_done\n-- \n2.35.1.1337.g7e32d794afe\n\n"},{"id":"451141","messageId":"RFC-patch-v2-04.13-f64aefa9ece-20220311T155841Z-avarab@gmail.com","threadId":"56782","inReplyTo":"RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com","subject":"[RFC PATCH v2 04/13] connect.c: refactor sending of agent & object-format","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T16:24:16Z","receivedAt":"2022-03-11T16:24:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor 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 \n---\n connect.c | 33 ++++++++++++++++++++-------------\n 1 file changed, 20 insertions(+), 13 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex afc79a6236e..e6d0b1d34bd 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 \tconst char **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-- \n2.35.1.1337.g7e32d794afe\n\n"},{"id":"451142","messageId":"RFC-patch-v2-06.13-617a6b16df8-20220311T155841Z-avarab@gmail.com","threadId":"56782","inReplyTo":"RFC-cover-v2-00.13-00000000000-20220311T155841Z-avarab@gmail.com","subject":"[RFC PATCH v2 06/13] bundle-uri client: add \"git ls-remote-bundle-uri\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-11T16:24:18Z","receivedAt":"2022-03-11T16:24:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a git-ls-remote-bundle-uri command, this is a thin wrapper for\nissuing protocol v2 \"bundle-uri\" commands to a server, and to the\nparsing routines in bundle-uri.c.\n\nSince in the \"git clone\" case we'll have already done the handshake(),\nbut not here, introduce a \"got_advertisement\" state along with\n\"got_remote_heads\". It seems to me that the \"got_remote_heads\" is\nbadly named in the first place, and the whole logic of eagerly getting\nls-refs on handshake() or not could be refactored somewhat, but let's\nnot do that now, and instead just add another self-documenting state\nvariable.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-ls-remote-bundle-uri.txt | 62 ++++++++++\n Documentation/git-ls-remote.txt | 1 +\n Makefile | 1 +\n builtin.h | 1 +\n builtin/clone.c | 2 +-\n builtin/ls-remote-bundle-uri.c | 90 ++++++++++++++\n command-list.txt | 1 +\n git.c | 1 +\n t/lib-t5730-protocol-v2-bundle-uri.sh | 132 +++++++++++++++++++++\n transport.c | 43 +++++--\n transport.h | 6 +-\n 11 files changed, 329 insertions(+), 11 deletions(-)\n create mode 100644 Documentation/git-ls-remote-bundle-uri.txt\n create mode 100644 builtin/ls-remote-bundle-uri.c\n\ndiff --git a/Documentation/git-ls-remote-bundle-uri.txt b/Documentation/git-ls-remote-bundle-uri.txt\nnew file mode 100644\nindex 00000000000..793d7677f2f\n--- /dev/null\n+++ b/Documentation/git-ls-remote-bundle-uri.txt\n@@ -0,0 +1,62 @@\n+git-ls-remote-bundle-uri(1)\n+===========================\n+\n+NAME\n+----\n+git-ls-remote-bundle-uri - List 'bundle-uri' in a remote repository\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git ls-remote-bundle-uri' [-q |--quiet] [--uri] [--upload-pack=]\n+\t\t\t [[-o | --server-option=]