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

[PATCH v2 0/4] Sideband the whole fetch v2 response

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Jan 15, 2019, 19:40 UTC
Message-ID
<cover.1547581039.git.jonathantanmy@google.com>
In-Reply-To
<cover.1547244620.git.jonathantanmy@google.com>
Like v1, this is on origin/ms/packet-err-check.
Thanks, Junio, for your reviews.
Jonathan Tan (4):
  pkt-line: introduce struct packet_writer
  sideband: reverse its dependency on pkt-line
  {fetch,upload}-pack: sideband v2 fetch response
  tests: define GIT_TEST_SIDEBAND_ALL
 Documentation/technical/protocol-v2.txt |  10 ++
 fetch-pack.c                            |  13 +-
 pkt-line.c                              | 121 +++++++++++++++---
 pkt-line.h                              |  34 +++++
 sideband.c                              | 161 ++++++++++++------------
 sideband.h                              |  14 ++-
 t/README                                |   5 +
 t/lib-httpd/apache.conf                 |   1 +
 t/t5537-fetch-shallow.sh                |   3 +-
 t/t5701-git-serve.sh                    |   2 +-
 t/t5702-protocol-v2.sh                  |   4 +-
 upload-pack.c                           | 131 +++++++++++--------
 12 files changed, 343 insertions(+), 156 deletions(-)
Interdiff against v1:
diff --git a/pkt-line.c b/pkt-line.c
index 71b17e6b93..69162f3990 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -447,16 +447,16 @@ int recv_sideband(const char *me, int in_stream, int out)
 
 	while (1) {
 		len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 0);
-		retval = diagnose_sideband(me, buf, len, 0);
+		retval = demultiplex_sideband(me, buf, len, 0);
 		switch (retval) {
-			case SIDEBAND_PRIMARY:
-				write_or_die(out, buf + 1, len - 1);
-				break;
-			case SIDEBAND_PROGRESS:
-				/* already written by diagnose_sideband() */
-				break;
-			default: /* flush or error */
-				return retval;
+		case SIDEBAND_PRIMARY:
+			write_or_die(out, buf + 1, len - 1);
+			break;
+		case SIDEBAND_PROGRESS:
+			/* already written by demultiplex_sideband() */
+			break;
+		default: /* errors: message already written */
+			return retval;
 		}
 	}
 }
@@ -474,6 +474,7 @@ void packet_reader_init(struct packet_reader *reader, int fd,
 	reader->buffer = packet_buffer;
 	reader->buffer_size = sizeof(packet_buffer);
 	reader->options = options;
+	reader->me = "git";
 }
 
 enum packet_read_status packet_reader_read(struct packet_reader *reader)
@@ -483,6 +484,10 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
 		return reader->status;
 	}
 
+	/*
+	 * Consume all progress and keepalive packets until a primary payload
+	 * packet is received
+	 */
 	while (1) {
 		int retval;
 		reader->status = packet_read_with_status(reader->fd,
@@ -494,29 +499,31 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
 							 reader->options);
 		if (!reader->use_sideband)
 			break;
-		retval = diagnose_sideband(reader->me, reader->buffer,
-					   reader->pktlen, 1);
+		retval = demultiplex_sideband(reader->me, reader->buffer,
+					      reader->pktlen, 1);
 		switch (retval) {
-			case SIDEBAND_PROTOCOL_ERROR:
-			case SIDEBAND_REMOTE_ERROR:
-				BUG("should have died in diagnose_sideband");
-			case SIDEBAND_FLUSH:
-				goto nonprogress;
-			case SIDEBAND_PRIMARY:
-				if (reader->pktlen != 1)
-					goto nonprogress;
-				/*
-				 * Since pktlen is 1, this is a keepalive
-				 * packet. Wait for the next one.
-				 */
-				break;
-			default: /* SIDEBAND_PROGRESS */
-				;
+		case SIDEBAND_PROTOCOL_ERROR:
+		case SIDEBAND_REMOTE_ERROR:
+			BUG("should have died in diagnose_sideband");
+		case SIDEBAND_FLUSH:
+			goto nonprogress_received;
+		case SIDEBAND_PRIMARY:
+			if (reader->pktlen != 1)
+				goto nonprogress_received;
+			/*
+			 * Since the packet contains nothing but the sideband
+			 * designator, this is a keepalive packet. Wait for the
+			 * next one.
+			 */
+			break;
+		default: /* SIDEBAND_PROGRESS */
+			;
 		}
 	}
 
-nonprogress:
+nonprogress_received:
 	if (reader->status == PACKET_READ_NORMAL)
+		/* Skip the sideband designator if sideband is used */
 		reader->line = reader->use_sideband ?
 			reader->buffer + 1 : reader->buffer;
 	else
@@ -549,7 +556,7 @@ void packet_writer_write(struct packet_writer *writer, const char *fmt, ...)
 
 	va_start(args, fmt);
 	packet_write_fmt_1(writer->dest_fd, 0,
-			   writer->use_sideband ? "\1" : "", fmt, args);
+			   writer->use_sideband ? "\001" : "", fmt, args);
 	va_end(args);
 }
 
@@ -559,7 +566,7 @@ void packet_writer_error(struct packet_writer *writer, const char *fmt, ...)
 
 	va_start(args, fmt);
 	packet_write_fmt_1(writer->dest_fd, 0,
-			   writer->use_sideband ? "\3" : "ERR ", fmt, args);
+			   writer->use_sideband ? "\003" : "ERR ", fmt, args);
 	va_end(args);
 }
 
diff --git a/sideband.c b/sideband.c
index cbeab380ae..9d3051e3fe 100644
--- a/sideband.c
+++ b/sideband.c
@@ -113,7 +113,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
 #define ANSI_SUFFIX "\033[K"
 #define DUMB_SUFFIX "        "
 
-int diagnose_sideband(const char *me, char *buf, int len, int die_on_error)
+int demultiplex_sideband(const char *me, char *buf, int len, int die_on_error)
 {
 	static const char *suffix;
 	struct strbuf outbuf = STRBUF_INIT;
diff --git a/sideband.h b/sideband.h
index 3786670694..f75c4fde2a 100644
--- a/sideband.h
+++ b/sideband.h
@@ -8,15 +8,14 @@
 #define SIDEBAND_PROGRESS 2
 
 /*
- * buf and len should be the result of reading a line from a remote sending
- * multiplexed data.
+ * Inspects a multiplexed packet read from the remote and returns which
+ * sideband it is for.
  *
- * Determines the nature of the result and returns it. If
- * SIDEBAND_PROTOCOL_ERROR, SIDEBAND_REMOTE_ERROR, or SIDEBAND_PROGRESS, also
- * prints a message (or the formatted contents of the notice in the case of
- * SIDEBAND_PROGRESS) to stderr.
+ * If SIDEBAND_PROTOCOL_ERROR, SIDEBAND_REMOTE_ERROR, or SIDEBAND_PROGRESS,
+ * also prints a message (or the formatted contents of the notice in the case
+ * of SIDEBAND_PROGRESS) to stderr.
  */
-int diagnose_sideband(const char *me, char *buf, int len, int die_on_error);
+int demultiplex_sideband(const char *me, char *buf, int len, int die_on_error);
 
 void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
 
-- 
2.19.0.271.gfe8321ec05.dirty
Previous: Jonathan TanNext: Jonathan Tan
Message 18 of 41 in “Sideband the whole fetch v2 response”
  1. 0/4 Sideband the whole fetch v2 responseJonathan Tan, Jan 11, 2019
  2. 1/4 pkt-line: introduce struct packet_writerJonathan Tan, Jan 11, 2019
  3. Junio C HamanoJan 14, 2019
  4. Jonathan TanJan 15, 2019
  5. 2/4 sideband: reverse its dependency on pkt-lineJonathan Tan, Jan 11, 2019
  6. Junio C HamanoJan 14, 2019
  7. Junio C HamanoJan 14, 2019
  8. Jonathan TanJan 15, 2019
  9. Jonathan TanJan 15, 2019
  10. 3/4 {fetch,upload}-pack: sideband v2 fetch responseJonathan Tan, Jan 11, 2019
  11. Junio C HamanoJan 14, 2019
  12. Jonathan TanJan 15, 2019
  13. 4/4 tests: define GIT_TEST_SIDEBAND_ALLJonathan Tan, Jan 11, 2019
  14. 5/4 Documentation: order protocol v2 sectionsJonathan Tan, Jan 11, 2019
  15. 6/4 Documentation: add Packfile URIs design docJonathan Tan, Jan 11, 2019
  16. 7/4 upload-pack: refactor reading of pack-objects outJonathan Tan, Jan 11, 2019
  17. 8/4 upload-pack: send part of packfile response as uriJonathan Tan, Jan 11, 2019
  18. 0/4 Sideband the whole fetch v2 responseJonathan Tan, Jan 15, 2019
  19. 1/4 pkt-line: introduce struct packet_writerJonathan Tan, Jan 15, 2019
  20. 2/4 sideband: reverse its dependency on pkt-lineJonathan Tan, Jan 15, 2019
  21. SZEDER GáborJan 16, 2019
  22. Jonathan TanJan 16, 2019
  23. SZEDER GáborJan 16, 2019
  24. 3/4 {fetch,upload}-pack: sideband v2 fetch responseJonathan Tan, Jan 15, 2019
  25. 4/4 tests: define GIT_TEST_SIDEBAND_ALLJonathan Tan, Jan 15, 2019
  26. Junio C HamanoJan 15, 2019
  27. Junio C HamanoJan 15, 2019
  28. Jonathan TanJan 15, 2019
  29. Junio C HamanoJan 15, 2019
  30. Jonathan TanJan 15, 2019
  31. Junio C HamanoJan 15, 2019
  32. Jonathan TanJan 16, 2019
  33. Junio C HamanoJan 16, 2019
  34. 0/4 Sideband the whole fetch v2 responseJonathan Tan, Jan 16, 2019
  35. 1/4 pkt-line: introduce struct packet_writerJonathan Tan, Jan 16, 2019
  36. 2/4 sideband: reverse its dependency on pkt-lineJonathan Tan, Jan 16, 2019
  37. 3/4 {fetch,upload}-pack: sideband v2 fetch responseJonathan Tan, Jan 16, 2019
  38. 4/4 tests: define GIT_TEST_SIDEBAND_ALLJonathan Tan, Jan 16, 2019
  39. Jonathan NiederJan 29, 2019
  40. Jeff KingFeb 13, 2019
  41. Junio C HamanoJan 17, 2019

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.