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

From: Shawn O. Pearce <spearce@spearce.org>
Date: Jun 7, 2009, 20:43 UTC
Message-ID: <20090607204343.GC16497@spearce.org>
In-Reply-To: <d411cc4a0906061458g494d80dbwe3a5358edfd1d49e@mail.gmail.com>

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:

Show 5 quoted lines

>  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.

Show 8 quoted lines

>  [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.