git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] pack-protocol: document newline behavior in push commands

From
Jeff King <peff@peff.net>
Date
Sep 3, 2015, 08:02 UTC
Message-ID
<20150903080205.GA20182@sigill.intra.peff.net>

Our pack-protocol spec indicates that a pushing client should send ref update commands like:

  $old_sha1 $new_sha1 $ref\n

with each ref update in its own pktline, with a trailing newline. However, git itself does not follow this behavior; it omits the trailing newline.

For the most part the distinction is harmless. The `receive-pack` on the other end will call `packet_read_line`, which strips off the newline if it is there, and we are fine either way.

Things are more complicated for the initial ref, which also has capabilities. The spec says to send:

  $old_sha1 $new_sha1 $ref\0 $caps\n

which is also OK by the current `receive-pack code (the newline is at the end of the packet, so we still strip it).

As an aside, it would _not_ be OK to send:
  $old_sha1 $new_sha1 $ref\n\0 $caps\n

The spec does not allow that and receive-pack would reject it, but it is perhaps a mistake that a naive client implementation might make.

So what is in the current spec is quite reasonable, and simply follows the normal rules for pkt-line framing (we say LF, but it really is optional). But it does not document how git actually behaves.

Signed-off-by: Jeff King <peff@peff.net>
---
+cc Junio, Dave, and Shawn, as this is somewhat related to the
discussion in
http://thread.gmane.org/gmane.comp.version-control.git/273175/focus=273444

I happened to be looking into some protocol stuff recently and noticed that we do not follow the spec here (but interestingly, libgit2 does!).

Frankly, this feels a bit like a step backwards to me. I am tempted to suggest instead that git start sending the newlines, but I'm not sure it's worth any potential fallout.

I'm also tempted to scrap this and say it just falls under the rule that every PKT-LINE is "sender SHOULD include LF" and "receiver MUST NOT complain about missing LF" (which does appear earlier in the document, though in a different context). Or maybe we should write out that rule explicitly and drop the "LF" from all parts of the grammar (which is what the thread above advocates).

 Documentation/technical/pack-protocol.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index 4064fc7..9ce53b4 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -469,8 +469,8 @@ references.
 
   shallow           =  PKT-LINE("shallow" SP obj-id LF)
 
-  command-list      =  PKT-LINE(command NUL capability-list LF)
-		       *PKT-LINE(command LF)
+  command-list      =  PKT-LINE(command NUL capability-list)
+		       *PKT-LINE(command)
 		       flush-pkt
 
   command           =  create / delete / update
@@ -586,8 +586,8 @@ An example client/server communication might look like this:
    S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/team\n
    S: 0000
 
-   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug\n
-   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master\n
+   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug
+   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master
    C: 0000
    C: [PACKDATA]
 
-- 
2.5.1.812.ge796bff
Next: Jeff King
Message 1 of 7 in “pack-protocol: document newline behavior in push commands”
  1. pack-protocol: document newline behavior in push commandsJeff King, Sep 3, 2015
  2. pack-protocol: clarify LF-handling in PKT-LINE()Jeff King, Sep 3, 2015
  3. Jeff KingSep 3, 2015
  4. Junio C HamanoSep 3, 2015
  5. Junio C HamanoSep 3, 2015
  6. Jeff KingSep 4, 2015
  7. Junio C HamanoSep 4, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.