{"thread":{"id":"56934","subject":"[PATCH] http-protocol.txt: add missing flush to example","startedAt":"2021-11-18T11:22:59Z","lastAt":"2021-11-18T16:38:57Z","messageCount":2,"participants":["TimTIM via GitGitGadget","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"441598","messageId":"pull.1141.git.git.1637234203401.gitgitgadget@gmail.com","threadId":"56934","inReplyTo":null,"subject":"[PATCH] http-protocol.txt: add missing flush to example","fromName":"TimTIM via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-18T11:16:43Z","receivedAt":"2021-11-18T11:22:59Z","isPatch":true,"sender":{"key":"name:TimTIM","avatar":null},"body":"From: TimTim Wong <t2@live.hk>\n\nSigned-off-by: Timothy Wong <i@timtim.hk>\n---\n    Fix example in documentation\n    \n    wants must be flushed with 0000 before haves\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1141%2Fwegylexy%2Fpatch-1-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1141/wegylexy/patch-1-v1\nPull-Request: https://github.com/git/git/pull/1141\n\n Documentation/technical/http-protocol.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt\nindex cc5126cfeda..facb315a993 100644\n--- a/Documentation/technical/http-protocol.txt\n+++ b/Documentation/technical/http-protocol.txt\n@@ -314,8 +314,9 @@ Clients MUST first perform ref discovery with\n    C: Content-Type: application/x-git-upload-pack-request\n    C:\n    C: 0032want 0a53e9ddeaddad63ad106860237bbf53411d11a7\\n\n-   C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\\n\n    C: 0000\n+   C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\\n\n+   C: done\n \n    S: 200 OK\n    S: Content-Type: application/x-git-upload-pack-result\n@@ -337,9 +338,9 @@ server advertises capability `allow-tip-sha1-in-want` or\n `allow-reachable-sha1-in-want`.\n \n   compute_request   =  want_list\n+\t\t       \"0000\"\n \t\t       have_list\n-\t\t       request_end\n-  request_end       =  \"0000\" / \"done\"\n+\t\t       \"done\"\n \n   want_list         =  PKT-LINE(want SP cap_list LF)\n \t\t       *(want_pkt)\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\n-- \ngitgitgadget\n"},{"id":"441615","messageId":"YZaBnlUWDiid3Pc3@coredump.intra.peff.net","threadId":"56934","inReplyTo":"pull.1141.git.git.1637234203401.gitgitgadget@gmail.com","subject":"Re: [PATCH] http-protocol.txt: add missing flush to example","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-18T16:38:54Z","receivedAt":"2021-11-18T16:38:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 18, 2021 at 11:16:43AM +0000, TimTIM via GitGitGadget wrote:\n\n> From: TimTim Wong <t2@live.hk>\n> \n> Signed-off-by: Timothy Wong <i@timtim.hk>\n> ---\n>     Fix example in documentation\n>     \n>     wants must be flushed with 0000 before haves\n\nThis explanation would probably make more sense in the commit message.\n\nI think this is only true for the v0 protocol. In the v2 protocol, we\ntake all of the wants/haves as a single sequence. That may be OK for our\npurposes here, though. While the v2 docs do refer to http-protocol.txt,\nit is only for the \"Initial Client Request\" section. The actual v2 fetch\nprimitive is defined separately in protocol-v2.txt.\n\n> diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt\n> index cc5126cfeda..facb315a993 100644\n> --- a/Documentation/technical/http-protocol.txt\n> +++ b/Documentation/technical/http-protocol.txt\n> @@ -314,8 +314,9 @@ Clients MUST first perform ref discovery with\n>     C: Content-Type: application/x-git-upload-pack-request\n>     C:\n>     C: 0032want 0a53e9ddeaddad63ad106860237bbf53411d11a7\\n\n> -   C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\\n\n>     C: 0000\n> +   C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\\n\n> +   C: done\n\nOK, so the \"have\" lines moves after the flush, which makes sense.\nThere's no longer a trailing flush, though. Instead, we put in a \"done\"\ncommand. This is OK, and what we'd often send in practice, but it is\noptional (due to \"no-done\"). I wonder if it is worth keeping things\nsimpler. I dunno.\n\n(BTW, if you run with GIT_TRACE_PACKET here, it's a bit misleading. We\n_do_ send a flush after the \"done\", but it's the flush to tell\nremote-curl that it should send the stateless-rpc packet; it doesn't\nactually go over the wire with the HTTP request).\n\n> @@ -337,9 +338,9 @@ server advertises capability `allow-tip-sha1-in-want` or\n>  `allow-reachable-sha1-in-want`.\n>  \n>    compute_request   =  want_list\n> +\t\t       \"0000\"\n>  \t\t       have_list\n> -\t\t       request_end\n> -  request_end       =  \"0000\" / \"done\"\n> +\t\t       \"done\"\n>  \n\nOK, so here we put the flush after want_list, which is good. But this\nloses the optionality of \"done\". I think it's valid to just send the\nflush, but this BNF no longer reflects that. Or if I'm wrong, I think\nthis needs to be discussed in the commit message.\n\nI wondered if the non-http protocol had a similar problem, but it treats\nthe \"want\" and \"have\" phases more independently. E.g., we get:\n\n    upload-request    =  want-list\n                       *shallow-line\n                       *1depth-request\n                       [filter-request]\n                       flush-pkt\n\nand then later:\n\n    upload-haves      =  have-list\n                         compute-end\n    compute-end       =  flush-pkt / PKT-LINE(\"done\")\n\nwhich is OK.\n\n-Peff\n"}]}