On Tue, Aug 20, 2024 at 1:32 PM Christian Couder
<christian.couder@xxxxxxxxx> wrote:
>
> On Wed, Jul 31, 2024 at 6:16 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
> >
> > On Wed, Jul 31, 2024 at 03:40:13PM +0200, Christian Couder wrote:
> > > 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
[...]
> > 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.
