On Tue, Aug 20, 2024 at 1:32 PM Christian Couder
[off-list ref] wrote:

On Wed, Jul 31, 2024 at 6:16 PM Taylor Blau [off-list ref] wrote:

quoted

On Wed, Jul 31, 2024 at 03:40:13PM +0200, Christian Couder wrote:

quoted

diff --git a/Documentation/gitprotocol-v2.txt b/Documentation/gitprotocol-v2.txt
index 414bc625d5..4d8d3839c4 100644
--- a/Documentation/gitprotocol-v2.txt
+++ b/Documentation/gitprotocol-v2.txt

@@ -781,6 +781,43 @@ retrieving the header from a bundle at the indicated URI, and thus
 save themselves and the server(s) the request(s) needed to inspect the
 headers of that bundle or bundles.

+promisor-remote=<pr-infos>
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The server may advertise some promisor remotes it is using, if it's OK
+for the server that a client uses them too. In this case <pr-infos>
+should be of the form:
+
+     pr-infos = pr-info | pr-infos ";" pr-info

[...]

quoted

I wonder if it would instead be useful to have <pr-infos> first write
out how many <pr-info>s it contains, and then write out each <pr-info>
separated by a NUL byte, so that none of the files in the <pr-info>
itself are restricted in what characters they can use.

I am not sure how NUL bytes would interfere with the pkt-line.[c,h] code though.

As Junio said pkt-line.[ch] is about <length> and <bytes> and it is
used to transfer the pack data stream that can have arbitrary bytes,
so there is no problem with NUL bytes. Sorry for not checking.

However I still think that capabilities have been using a simple text
format for now which works well, and that it's better to respect that
format and not introduce complexity in it if it's not necessary.

For example t5555-http-smart-common.sh has:

cat >expect <<-EOF &&
    version 2
    agent=FAKE
    ls-refs=unborn
    fetch=shallow wait-for-done
    server-option
    object-format=$(test_oid algo)
    0000
    EOF

to check the capabilities sent by `git upload-pack --advertise-refs`.

t5701 also uses similar instructions to check protocol v2 server commands.

So I think it's nice for tests and debugging if we keep using a simple
text format.

Also writing the number of <pr-info>s and then each <pr-info>
separated by a NUL byte might not save a lot of bytes compared to
urlencoded content if necessary, as I don't think many special
characters will need to be urlencoded most of the time.

So in the version 2 of this patch series, I haven't changed this.

Thanks for the review.