* [PATCH] http-protocol.txt: add missing flush to example
@ 2021-11-18 11:16 TimTIM via GitGitGadget
2021-11-18 16:38 ` Jeff King
0 siblings, 1 reply; 2+ messages in thread
From: TimTIM via GitGitGadget @ 2021-11-18 11:16 UTC (permalink / raw)
To: git; +Cc: TimTIM, TimTim Wong
From: TimTim Wong <t2@live.hk>
Signed-off-by: Timothy Wong <i@timtim.hk>
---
Fix example in documentation
wants must be flushed with 0000 before haves
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1141%2Fwegylexy%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1141/wegylexy/patch-1-v1
Pull-Request: https://github.com/git/git/pull/1141
Documentation/technical/http-protocol.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt
index cc5126cfeda..facb315a993 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -314,8 +314,9 @@ Clients MUST first perform ref discovery with
C: Content-Type: application/x-git-upload-pack-request
C:
C: 0032want 0a53e9ddeaddad63ad106860237bbf53411d11a7\n
- C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\n
C: 0000
+ C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\n
+ C: done
S: 200 OK
S: Content-Type: application/x-git-upload-pack-result
@@ -337,9 +338,9 @@ server advertises capability `allow-tip-sha1-in-want` or
`allow-reachable-sha1-in-want`.
compute_request = want_list
+ "0000"
have_list
- request_end
- request_end = "0000" / "done"
+ "done"
want_list = PKT-LINE(want SP cap_list LF)
*(want_pkt)
base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc
--
gitgitgadget
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] http-protocol.txt: add missing flush to example
2021-11-18 11:16 [PATCH] http-protocol.txt: add missing flush to example TimTIM via GitGitGadget
@ 2021-11-18 16:38 ` Jeff King
0 siblings, 0 replies; 2+ messages in thread
From: Jeff King @ 2021-11-18 16:38 UTC (permalink / raw)
To: TimTIM via GitGitGadget; +Cc: git, TimTIM
On Thu, Nov 18, 2021 at 11:16:43AM +0000, TimTIM via GitGitGadget wrote:
> From: TimTim Wong <t2@live.hk>
>
> Signed-off-by: Timothy Wong <i@timtim.hk>
> ---
> Fix example in documentation
>
> wants must be flushed with 0000 before haves
This explanation would probably make more sense in the commit message.
I think this is only true for the v0 protocol. In the v2 protocol, we
take all of the wants/haves as a single sequence. That may be OK for our
purposes here, though. While the v2 docs do refer to http-protocol.txt,
it is only for the "Initial Client Request" section. The actual v2 fetch
primitive is defined separately in protocol-v2.txt.
> diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt
> index cc5126cfeda..facb315a993 100644
> --- a/Documentation/technical/http-protocol.txt
> +++ b/Documentation/technical/http-protocol.txt
> @@ -314,8 +314,9 @@ Clients MUST first perform ref discovery with
> C: Content-Type: application/x-git-upload-pack-request
> C:
> C: 0032want 0a53e9ddeaddad63ad106860237bbf53411d11a7\n
> - C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\n
> C: 0000
> + C: 0032have 441b40d833fdfa93eb2908e52742248faf0ee993\n
> + C: done
OK, so the "have" lines moves after the flush, which makes sense.
There's no longer a trailing flush, though. Instead, we put in a "done"
command. This is OK, and what we'd often send in practice, but it is
optional (due to "no-done"). I wonder if it is worth keeping things
simpler. I dunno.
(BTW, if you run with GIT_TRACE_PACKET here, it's a bit misleading. We
_do_ send a flush after the "done", but it's the flush to tell
remote-curl that it should send the stateless-rpc packet; it doesn't
actually go over the wire with the HTTP request).
> @@ -337,9 +338,9 @@ server advertises capability `allow-tip-sha1-in-want` or
> `allow-reachable-sha1-in-want`.
>
> compute_request = want_list
> + "0000"
> have_list
> - request_end
> - request_end = "0000" / "done"
> + "done"
>
OK, so here we put the flush after want_list, which is good. But this
loses the optionality of "done". I think it's valid to just send the
flush, but this BNF no longer reflects that. Or if I'm wrong, I think
this needs to be discussed in the commit message.
I wondered if the non-http protocol had a similar problem, but it treats
the "want" and "have" phases more independently. E.g., we get:
upload-request = want-list
*shallow-line
*1depth-request
[filter-request]
flush-pkt
and then later:
upload-haves = have-list
compute-end
compute-end = flush-pkt / PKT-LINE("done")
which is OK.
-Peff
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-18 16:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 11:16 [PATCH] http-protocol.txt: add missing flush to example TimTIM via GitGitGadget
2021-11-18 16:38 ` Jeff King
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.