Re: Comments pack protocol description in "Git Community Book" (second round)

["Shawn O. Pearce" <spearce@spearce.org>]

Scott Chacon <schacon@gmail.com> wrote:
> In addition to that, I started taking a shot at putting together an
> RFC formatted documentation of this protocol as was requested.
...
> http://git-scm.com/gitserver.txt

SSH is described by RFC 4251 and RFC 4254.  Reference it when you
mention it.

Section 2.2.3 Commit is missing spaces after the parent, author,
committer, encoding headers:

>  parent    = "parent" + sha + \n
>  userinfo  = NAME <EMAIL> TIME
>  author    = "author" + userinfo + \n
>  committer = "committer" + userinfo + \n
>  encoding  = "encoding" + encoding + \n

2.2.4. Tag, same problem.

>   At the end of the
> packfile is a 20-byte SHA1 sum of all the shas in that packfile.

No.  The SHA-1 checksum on the footer of the pack is over all of
the preceeding bytes of the pack.

> (B << 4) & A bytes when expanded

No.  (B << 4) | A bytes when expanded.

>  [1 byte]   | 1 | type (3) | size A (4)     |  |- object #3 header
>             +-------------------------------+  |
>  [1 byte]   | 0 | size data B (7)           |  |
>             +-------------------------------+  |
>  [1 byte]   | 0 | size data C (7)           |  |
>             +-------------------------------+ -+
>             | compressed object data        | (C << 11) & (B << 4) & A
>             |                               | bytes when expanded

The B byte has the high bit set (1).  And the length is
(C << 11) | (B << 4) | A.

Also, I found reading that difficult, and it doesn't mention the
OBJ_REF_DELTA or OBJ_OFS_DELTA cases.

You also need to note that the version number in the file header
is currently '2', as described by this RFC.

>    Finally, the trailer records 20-byte SHA1 checksum of the rest of the
>   file.

Like I said above, its the preceeding bytes of the pack.

Section 4.2 Git Protocol, explain the git:// URI first, and then
how a client splits that into the request, and then how it formats
the request.  Don't forget to include an example with a non-standard
port number.

Also document what the standard port number is.

Elsewhere in the document you say 'upload-pack' or 'receive-pack'.
I think you should be saying 'git-upload-pack' or 'git-receive-pack'
everywhere, as these are the formal names in the protocol.

Section 5.2, Capabilities:

>  Client sends space separated list of capabilities it wants.  It
>  SHOULD send a subset of server capabilities, i.e do not send
>  capabilities served does not advertise.  The client SHOULD NOT ask
>  for capabilities the server did not say it supports.

I thought we had said it was client MUST send a subset of server
capabilities; client MUST NOT ask for capabilities server did
not advertise support of.

>  Server MUST ignore capabilities it does not understand.  Server MUST
>  NOT ignore capabilities that client requested and server advertised.

I think that's just lazy coding on the server part.  If the server
gets a capability request it can't honor, it MUST abort, it might
corrupt the stream to the client.

> 5.2.1.  multi-ack
>
>  The 'multi-ack' capability allows the server to return "ACK $SHA1

multi_ack

>  Without multi_ack, a client sends have lines in --date-order until
>  the server has found a common base.  That means the client will send

Explain --date-order, don't assume the reader knows it.

I'm giving up for now.  :-)

-- 
Shawn.