fetch-pack: Use a strbuf to compose the want list

This change is being offered as a refactoring to make later
commits in the smart HTTP series easier.

By changing the enabled capabilities to be formatted in a strbuf
it is easier to add a new capability to the set of supported
capabilities.

By formatting the want portion of the request into a strbuf and
writing it as a whole block we can later decide to hold onto
the req_buf (instead of releasing it) to recycle in stateless
communications.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin-fetch-pack.c

@@ -165,6 +165,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	const unsigned char *sha1;
 	unsigned in_vain = 0;
 	int got_continue = 0;
+	struct strbuf req_buf = STRBUF_INIT;
 
 	if (marked)
 		for_each_ref(clear_marks, NULL);
@@ -175,6 +176,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	fetching = 0;
 	for ( ; refs ; refs = refs->next) {
 		unsigned char *remote = refs->old_sha1;
+		const char *remote_hex;
 		struct object *o;
 
 		/*
@@ -192,27 +194,36 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 			continue;
 		}
 
-		if (!fetching)
+		remote_hex = sha1_to_hex(remote);
-			packet_write(fd[1], "want %s%s%s%s%s%s%s%s\n",
+		if (!fetching) {
-				     sha1_to_hex(remote),
+			struct strbuf c = STRBUF_INIT;
-				     (multi_ack ? " multi_ack" : ""),
+			if (multi_ack)          strbuf_addstr(&c, " multi_ack");
-				     (use_sideband == 2 ? " side-band-64k" : ""),
+			if (use_sideband == 2)  strbuf_addstr(&c, " side-band-64k");
-				     (use_sideband == 1 ? " side-band" : ""),
+			if (use_sideband == 1)  strbuf_addstr(&c, " side-band");
-				     (args.use_thin_pack ? " thin-pack" : ""),
+			if (args.use_thin_pack) strbuf_addstr(&c, " thin-pack");
-				     (args.no_progress ? " no-progress" : ""),
+			if (args.no_progress)   strbuf_addstr(&c, " no-progress");
-				     (args.include_tag ? " include-tag" : ""),
+			if (args.include_tag)   strbuf_addstr(&c, " include-tag");
-				     (prefer_ofs_delta ? " ofs-delta" : ""));
+			if (prefer_ofs_delta)   strbuf_addstr(&c, " ofs-delta");
-		else
+			packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
-			packet_write(fd[1], "want %s\n", sha1_to_hex(remote));
+			strbuf_release(&c);
+		} else
+			packet_buf_write(&req_buf, "want %s\n", remote_hex);
 		fetching++;
 	}
-	if (is_repository_shallow())
+
-		write_shallow_commits(fd[1], 1);
+	if (!fetching) {
-	if (args.depth > 0)
+		strbuf_release(&req_buf);
-		packet_write(fd[1], "deepen %d", args.depth);
+		packet_flush(fd[1]);
-	packet_flush(fd[1]);
-	if (!fetching)
 		return 1;
+	}
+
+	if (is_repository_shallow())
+		write_shallow_commits(&req_buf, 1);
+	if (args.depth > 0)
+		packet_buf_write(&req_buf, "deepen %d", args.depth);
+	packet_buf_flush(&req_buf);
+
+	safe_write(fd[1], req_buf.buf, req_buf.len);
 
 	if (args.depth > 0) {
 		char line[1024];
@@ -296,6 +307,8 @@ done:
 		multi_ack = 0;
 		flushes++;
 	}
+	strbuf_release(&req_buf);
+
 	while (flushes || multi_ack) {
 		int ack = get_ack(fd[0], result_sha1);
 		if (ack) {
@@ -809,6 +822,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 
 	if (args.depth > 0) {
 		struct cache_time mtime;
+		struct strbuf sb = STRBUF_INIT;
 		char *shallow = git_path("shallow");
 		int fd;
 
@@ -826,12 +840,14 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 
 		fd = hold_lock_file_for_update(&lock, shallow,
 					       LOCK_DIE_ON_ERROR);
-		if (!write_shallow_commits(fd, 0)) {
+		if (!write_shallow_commits(&sb, 0)
+		 || write_in_full(fd, sb.buf, sb.len) != sb.len) {
 			unlink_or_warn(shallow);
 			rollback_lock_file(&lock);
 		} else {
 			commit_lock_file(&lock);
 		}
+		strbuf_release(&sb);
 	}
 
 	reprepare_packed_git();

commit.c

@@ -199,7 +199,7 @@ struct commit_graft *lookup_commit_graft(const unsigned char *sha1)
 	return commit_graft[pos];
 }
 
-int write_shallow_commits(int fd, int use_pack_protocol)
+int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
 {
 	int i, count = 0;
 	for (i = 0; i < commit_graft_nr; i++)
@@ -208,12 +208,10 @@ int write_shallow_commits(int fd, int use_pack_protocol)
 				sha1_to_hex(commit_graft[i]->sha1);
 			count++;
 			if (use_pack_protocol)
-				packet_write(fd, "shallow %s", hex);
+				packet_buf_write(out, "shallow %s", hex);
 			else {
-				if (write_in_full(fd, hex,  40) != 40)
+				strbuf_addstr(out, hex);
-					break;
+				strbuf_addch(out, '\n');
-				if (write_str_in_full(fd, "\n") != 1)
-					break;
 			}
 		}
 	return count;

commit.h

@@ -131,7 +131,7 @@ extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);
 
 extern int register_shallow(const unsigned char *sha1);
 extern int unregister_shallow(const unsigned char *sha1);
-extern int write_shallow_commits(int fd, int use_pack_protocol);
+extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol);
 extern int is_repository_shallow(void);
 extern struct commit_list *get_shallow_commits(struct object_array *heads,
 		int depth, int shallow_flag, int not_shallow_flag);