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

[WIP RFC 4/5] upload-pack: refactor writing of "packfile" line

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Dec 3, 2018, 23:37 UTC
Message-ID
<1d678a78a63b7e58988b52c8c0efab38c34a6848.1543879256.git.jonathantanmy@google.com>
In-Reply-To
<cover.1543879256.git.jonathantanmy@google.com>

A subsequent patch allows pack-objects to output additional information (in addition to the packfile that it currently outputs). This means that we must hold off on writing the "packfile" section header to the client before we process the output of pack-objects, so move the writing of the "packfile" section header to read_pack_objects_stdout().

Unfortunately, this also means that we cannot send keepalive packets until pack-objects starts sending out the packfile, since the sideband is only established when the "packfile" section header is sent.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 upload-pack.c | 47 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index cec43e0a46..aa2589b858 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -104,9 +104,12 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
 struct output_state {
 	char buffer[8193];
 	int used;
+	unsigned packfile_started : 1;
+	struct strbuf progress_buf;
 };
 
-static int read_pack_objects_stdout(int outfd, struct output_state *os)
+static int read_pack_objects_stdout(int outfd, struct output_state *os,
+				    int use_protocol_v2)
 {
 	/* Data ready; we keep the last byte to ourselves
 	 * in case we detect broken rev-list, so that we
@@ -126,6 +129,12 @@ static int read_pack_objects_stdout(int outfd, struct output_state *os)
 	}
 	os->used += readsz;
 
+	if (!os->packfile_started) {
+		os->packfile_started = 1;
+		if (use_protocol_v2)
+			packet_write_fmt(1, "packfile\n");
+	}
+
 	if (os->used > 1) {
 		send_client_data(1, os->buffer, os->used - 1);
 		os->buffer[0] = os->buffer[os->used - 1];
@@ -138,8 +147,17 @@ static int read_pack_objects_stdout(int outfd, struct output_state *os)
 	return readsz;
 }
 
+static void flush_progress_buf(struct strbuf *progress_buf)
+{
+	if (!progress_buf->len)
+		return;
+	send_client_data(2, progress_buf->buf, progress_buf->len);
+	strbuf_reset(progress_buf);
+}
+
 static void create_pack_file(const struct object_array *have_obj,
-			     const struct object_array *want_obj)
+			     const struct object_array *want_obj,
+			     int use_protocol_v2)
 {
 	struct child_process pack_objects = CHILD_PROCESS_INIT;
 	struct output_state output_state = {0};
@@ -260,9 +278,13 @@ static void create_pack_file(const struct object_array *have_obj,
 			 */
 			sz = xread(pack_objects.err, progress,
 				  sizeof(progress));
-			if (0 < sz)
-				send_client_data(2, progress, sz);
-			else if (sz == 0) {
+			if (0 < sz) {
+				if (output_state.packfile_started)
+					send_client_data(2, progress, sz);
+				else
+					strbuf_add(&output_state.progress_buf,
+						   progress, sz);
+			} else if (sz == 0) {
 				close(pack_objects.err);
 				pack_objects.err = -1;
 			}
@@ -273,8 +295,10 @@ static void create_pack_file(const struct object_array *have_obj,
 		}
 		if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {
 			int result = read_pack_objects_stdout(pack_objects.out,
-							      &output_state);
-
+							      &output_state,
+							      use_protocol_v2);
+			if (output_state.packfile_started)
+				flush_progress_buf(&output_state.progress_buf);
 			if (result == 0) {
 				close(pack_objects.out);
 				pack_objects.out = -1;
@@ -293,12 +317,14 @@ static void create_pack_file(const struct object_array *have_obj,
 		 * protocol to say anything, so those clients are just out of
 		 * luck.
 		 */
-		if (!ret && use_sideband) {
+		if (!ret && use_sideband && output_state.packfile_started) {
 			static const char buf[] = "0005\1";
 			write_or_die(1, buf, 5);
 		}
 	}
 
+	flush_progress_buf(&output_state.progress_buf);
+
 	if (finish_command(&pack_objects)) {
 		error("git upload-pack: git-pack-objects died with error.");
 		goto fail;
@@ -1094,7 +1120,7 @@ void upload_pack(struct upload_pack_options *options)
 	if (want_obj.nr) {
 		struct object_array have_obj = OBJECT_ARRAY_INIT;
 		get_common_commits(&have_obj, &want_obj);
-		create_pack_file(&have_obj, &want_obj);
+		create_pack_file(&have_obj, &want_obj, 0);
 	}
 }
 
@@ -1475,8 +1501,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
 			send_wanted_ref_info(&data);
 			send_shallow_info(&data, &want_obj);
 
-			packet_write_fmt(1, "packfile\n");
-			create_pack_file(&have_obj, &want_obj);
+			create_pack_file(&have_obj, &want_obj, 1);
 			state = FETCH_DONE;
 			break;
 		case FETCH_DONE:
-- 
2.19.0.271.gfe8321ec05.dirty
Previous: Junio C HamanoNext: Junio C Hamano
Message 24 of 30 in “[WIP RFC 0/5] Design for offloading part of packfile response to CDN”
  1. Jonathan TanDec 3, 2018
  2. 1/5 Documentation: order protocol v2 sectionsJonathan Tan, Dec 3, 2018
  3. Junio C HamanoDec 5, 2018
  4. Jonathan TanDec 6, 2018
  5. Junio C HamanoDec 9, 2018
  6. 2/5 Documentation: add Packfile URIs design docJonathan Tan, Dec 3, 2018
  7. Stefan BellerDec 4, 2018
  8. brian m. carlsonDec 4, 2018
  9. Jonathan TanDec 4, 2018
  10. Christian CouderFeb 19, 2019
  11. Jonathan TanFeb 19, 2019
  12. Christian CouderFeb 22, 2019
  13. Ævar Arnfjörð BjarmasonFeb 19, 2019
  14. brian m. carlsonFeb 21, 2019
  15. Ævar Arnfjörð BjarmasonFeb 22, 2019
  16. Junio C HamanoDec 5, 2018
  17. Junio C HamanoDec 5, 2018
  18. Jonathan TanDec 6, 2018
  19. Ævar Arnfjörð BjarmasonFeb 19, 2019
  20. Jonathan TanFeb 19, 2019
  21. 3/5 upload-pack: refactor reading of pack-objects outJonathan Tan, Dec 3, 2018
  22. Stefan BellerDec 4, 2018
  23. Junio C HamanoDec 5, 2018
  24. 4/5 upload-pack: refactor writing of "packfile" lineJonathan Tan, Dec 3, 2018
  25. Junio C HamanoDec 6, 2018
  26. Jonathan TanDec 6, 2018
  27. Junio C HamanoDec 7, 2018
  28. 5/5 upload-pack: send part of packfile response as uriJonathan Tan, Dec 3, 2018
  29. Stefan BellerDec 4, 2018
  30. Stefan BellerDec 4, 2018

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.