Re: [RFC] protocol version 2

> Brandon Williams <bmwill@google.com> writes:
> 
> >   * Capabilities were implemented as a hack and are hidden behind a NUL
> >     byte after the first ref sent from the server during the ref
> >     advertisement:
> > ...
> >
> >   * Various other technical debt (e.g. abusing capabilities to
> >     communicate agent and symref data, service name set using a query
> >     parameter).
> 
> This sounds like a duplicate of the above.

> 
> >  Special Packets
> > -----------------
> >
> > In protocol v2 these special packets will have the following semantics:
> >
> >   * '0000' Flush Packet (flush-pkt) - indicates the end of a message
> >   * '0001' End-of-List delimiter (delim-pkt) - indicates the end of a list
> 
> After reading the remainder of the document twice, I do not think
> the reason why we want to introduce a new type delim-pkt is
> explained and justified well enough.  If a command request takes a
> command packet, zero or more capability packets, a mandatory
> delimiter packet, zero or more parameter packets and a mandatory
> flush packet, then you can use the same "flush" as delimiter in the
> middle.  The delimiter will of course become useful if you can omit
> it when not necessary (e.g. after seeing capabilities, you may see a
> flush and you will know there is no parameters and save the need to
> send one "delim").
> 
> I actually have a reasonable guess why you want to have a separate
> delimiter (which has nothing to do with "optional delim can be
> omitted"), but I want to see it explained in this document clearly
> by its designer(s).

> 
> >     command-request = command
> >                       capability-list
> >                       delim-pkt
> >                       (command specific parameters)
> >                       flush-pkt
> >     command = PKT-LINE("command=" key LF)
> >
> > The server will then acknowledge the command and requested capabilities
> > by echoing them back to the client and then launch into the command.
> >
> >     acknowledge-request = command
> >                           capability-list
> >                           delim-pkt
> >                           execute-command
> >     execute-command = <defined by each command>
> 
> It is not quite clear what the value of "echoing them back" is,
> especially if that is done by always echoing verbatim.  A reader may
> naturally expect, when capabilities are exchanged between two
> parties, these are negotiated so that the only ones that are
> commonly supported by both ends would be used, or something like
> that.

> 
> > A particular command can last for as many rounds as are required to
> > complete the service (multiple for negotiation during fetch and push or
> > no additional trips in the case of ls-refs).
> 
> OK.
> 
> >  Commands in v2
> > ~~~~~~~~~~~~~~~~
> >
> > Services are the core actions that a client wants to perform (fetch,
> > push, etc).  Each service has its own set of capabilities and its own
> > language of commands (think 'want' lines in fetch).  Optionally a
> > service can take in initial parameters or data when a client sends it
> > service request.
> 
> So a service (like "fetch") employ a set of "command"s (like "want",
> "have, etc)?  In the earlier part of the document, we did not see
> any mention of "service" and instead saw only "command" mentioned.
> Is the state machine on both ends and the transition between states
> implicit?  E.g. when one side throws "want" command and the other
> side acknowledges, they understand implicitly that they are now in a
> "fetch" service session, even though there is nothing that said over
> the wire that they are doing so?  One reason I am wondering about
> this is what we should do if a command verb may be applicable in
> multiple services.

> After reading the earlier protocol exchange explanation, I was sort
> of expecting that "fetch" would be the command and "want", "have",
> and "ack" lines would be exchanged as "command specific parameters",
> so a sudden introduction of "services" here was a bit of impedance
> mismatch to me.

> 
> >  Ls-refs
> > ---------
> >
> > Ls-refs can be looked at as the equivalent of the current ls-remote as
> > it is a way to query a remote for the references that it has.  Unlike
> > the current ls-remote, the filtering of the output is done on the server
> > side by passing a number of parameters to the server-side command
> > instead of the filtering occurring on the client.
> >
> > Ls-ref takes in the following parameters:
> >
> >   --head, --tags: Limit to only refs/heads or refs/tags
> 
> I see no need for the above two as long as "refs/heads/*", etc. are
> understood.
> 
> >   --refs: Do not show peeled tags or pseudorefs like HEAD
> 
> So showing peeled tags is the default?  Then I can sort-of see why
> "I am not interested in peeled result".  
> 
> I do not see why "do not show HEAD, MERGE_HEAD, etc." is needed,
> though.  It should be sufficient to just ask for refs/* if you are
> interested only in other things, no?
> 
> >   --symref: In addition to the object pointed by it, show the underlying
> >             ref pointed by it when showing a symbolic ref
> 
> Sort of OK--it probably is easier to always send this, as symbolic
> refs are minority anyway, though.
> 
> >   <refspec>: When specified, only references matching the given patterns
> >              are displayed.
> 
> I do not think you meant <refspec> here.
> 
> The side that is listing what it has has no reason to know what the
> recipient plans to do with the result, so you must be only sending
> the LHS of a refspec.  If your explanation says "given patterns",
> then replace <refspec> with <pattern>.  Do not abuse a term that has
> specific and established meaning for something else.

> 
> > The output of ls-refs is as follows:
> >
> >     output = (no-refs / list-of-refs)
> > 	     *symref
> >              *shallow
> >              flush-pkt
> >
> >     no-refs = PKT-LINE(zero-id SP no-refs LF)
> 
> Can't your list-of-refs have 0 element?  I do not see why you need
> no-refs here.  It's not like you need a dummy line, to the end of
> which you need to append NUL plus hidden capabilities ;-)

> 
> >     list-of-refs = *ref
> >     ref = PKT-LINE((tip / peeled) LF)
> >     tip = obj-id SP refname
> >     peeled = obj-id SP refname "^{}"
> >
> >     symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
> >     shallow = PKT-LINE("shallow" SP obj-id LF)
> 
> Thanks.