Hi,
Jonathan Tan wrote:
> Support for push options in the receive-pack protocol (and all Git
> components that speak it) have been added over a few commits, but not
> fully documented (especially its interaction with signed pushes). Update
> the protocol documentation to include the relevant details.
Thanks. This could be combined with the previous patch, since that
patch is enforcing what this patch documents.
[...]
> -This list is followed by a flush-pkt. Then the push options are transmitted
> -one per packet followed by another flush-pkt. After that the packfile that
> -should contain all the objects that the server will need to complete the new
> -references will be sent.
> +This list is followed by a flush-pkt.
I think this removed more than intended.
Before c714e45f (receive-pack: implement advertising and receiving
push options, 2016-07-14), this said
This list is followed by a flush-pkt and then the packfile that should
contain all the objects that the server will need to complete the new
references.
which I think would work fine.
[...]
> - update-request = *shallow ( command-list | push-cert ) [packfile]
> + update-request = *shallow ( command-list | push-cert )
This seems confusing to me both before and after. How does
update-request get used? Is it supposed to include the packfile or not?
Where do push-options fit into an unsigned request?
[...]
> @@ -500,12 +497,35 @@ references will be sent.
> PKT-LINE("pusher" SP ident LF)
> PKT-LINE("pushee" SP url LF)
> PKT-LINE("nonce" SP nonce LF)
> + *PKT-LINE("push-option" SP push-option LF)
> PKT-LINE(LF)
> *PKT-LINE(command LF)
> *PKT-LINE(gpg-signature-lines LF)
> PKT-LINE("push-cert-end" LF)
>
> - packfile = "PACK" 28*(OCTET)
> + push-option = 1*CHAR
> +----
> +
> +If the server has advertised the 'push-options' capability and the client has
> +specified 'push-options' as part of the capability list above, the client then
> +sends its push options followed by a flush-pkt.
> +
> +----
> + push-options = *PKT-LINE(push-option) flush-pkt
> +----
> +
> +For backwards compatibility with older Git servers, if the client sends a push
> +cert and push options, it SHOULD send its push options both embedded within the
This needs to be a MUST, not SHOULD.
> +push cert and after the push cert. (Note that the push options within the cert
> +are prefixed, but the push options after the cert are not.) Both these lists
> +SHOULD be consistent.
Likewise this one.
What does it mean to be consistent?
> +
> +After that the packfile that
> +should contain all the objects that the server will need to complete the new
> +references will be sent.
> +
> +----
> + packfile = "PACK" 28*(OCTET)
> ----
Thanks,
Jonathan