protocol: basic support for multi_ack_detailed

This tells the server that we speak it, but we don't make use of its
extra information to determine if there's a better place to stop
negotiating.

In a somewhat-related change, reorder the capabilities so we ask for
them in the same order as git does.

Also take this opportunity to factor out a fairly-indented portion of
the negotiation logic.

src/transports/smart.h

@@ -16,6 +16,7 @@
 
 #define GIT_CAP_OFS_DELTA "ofs-delta"
 #define GIT_CAP_MULTI_ACK "multi_ack"
+#define GIT_CAP_MULTI_ACK_DETAILED "multi_ack_detailed"
 #define GIT_CAP_SIDE_BAND "side-band"
 #define GIT_CAP_SIDE_BAND_64K "side-band-64k"
 #define GIT_CAP_INCLUDE_TAG "include-tag"
@@ -40,7 +41,7 @@ enum git_pkt_type {
 	GIT_PKT_UNPACK,
 };
 
-/* Used for multi-ack */
+/* Used for multi_ack and mutli_ack_detailed */
 enum git_ack_status {
 	GIT_ACK_NONE,
 	GIT_ACK_CONTINUE,
@@ -113,6 +114,7 @@ typedef struct transport_smart_caps {
 	int common:1,
 		ofs_delta:1,
 		multi_ack: 1,
+		multi_ack_detailed: 1,
 		side_band:1,
 		side_band_64k:1,
 		include_tag:1,

src/transports/smart_pkt.c

@@ -39,7 +39,7 @@ static int flush_pkt(git_pkt **out)
 	return 0;
 }
 
-/* the rest of the line will be useful for multi_ack */
+/* the rest of the line will be useful for multi_ack and multi_ack_detailed */
 static int ack_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt_ack *pkt;
@@ -62,6 +62,10 @@ static int ack_pkt(git_pkt **out, const char *line, size_t len)
 	if (len >= 7) {
 		if (!git__prefixcmp(line + 1, "continue"))
 			pkt->status = GIT_ACK_CONTINUE;
+		if (!git__prefixcmp(line + 1, "common"))
+			pkt->status = GIT_ACK_COMMON;
+		if (!git__prefixcmp(line + 1, "ready"))
+			pkt->status = GIT_ACK_READY;
 	}
 
 	*out = (git_pkt *) pkt;
@@ -456,25 +460,27 @@ static int buffer_want_with_caps(const git_remote_head *head, transport_smart_ca
 	char oid[GIT_OID_HEXSZ +1] = {0};
 	unsigned int len;
 
-	/* Prefer side-band-64k if the server supports both */
-	if (caps->side_band) {
-		if (caps->side_band_64k)
-			git_buf_printf(&str, "%s ", GIT_CAP_SIDE_BAND_64K);
-		else
-			git_buf_printf(&str, "%s ", GIT_CAP_SIDE_BAND);
-	}
-	if (caps->ofs_delta)
-		git_buf_puts(&str, GIT_CAP_OFS_DELTA " ");
-
-	if (caps->multi_ack)
+	/* Prefer multi_ack_detailed */
+	if (caps->multi_ack_detailed)
+		git_buf_puts(&str, GIT_CAP_MULTI_ACK_DETAILED " ");
+	else if (caps->multi_ack)
 		git_buf_puts(&str, GIT_CAP_MULTI_ACK " ");
 
+	/* Prefer side-band-64k if the server supports both */
+	if (caps->side_band_64k)
+		git_buf_printf(&str, "%s ", GIT_CAP_SIDE_BAND_64K);
+	else if (caps->side_band)
+		git_buf_printf(&str, "%s ", GIT_CAP_SIDE_BAND);
+
 	if (caps->include_tag)
 		git_buf_puts(&str, GIT_CAP_INCLUDE_TAG " ");
 
 	if (caps->thin_pack)
 		git_buf_puts(&str, GIT_CAP_THIN_PACK " ");
 
+	if (caps->ofs_delta)
+		git_buf_puts(&str, GIT_CAP_OFS_DELTA " ");
+
 	if (git_buf_oom(&str))
 		return -1;
 

src/transports/smart_protocol.c

@@ -97,6 +97,13 @@ int git_smart__detect_caps(git_pkt_ref *pkt, transport_smart_caps *caps)
 			continue;
 		}
 
+		/* Keep multi_ack_detailed before multi_ack */
+		if (!git__prefixcmp(ptr, GIT_CAP_MULTI_ACK_DETAILED)) {
+			caps->common = caps->multi_ack_detailed = 1;
+			ptr += strlen(GIT_CAP_MULTI_ACK_DETAILED);
+			continue;
+		}
+
 		if (!git__prefixcmp(ptr, GIT_CAP_MULTI_ACK)) {
 			caps->common = caps->multi_ack = 1;
 			ptr += strlen(GIT_CAP_MULTI_ACK);
@@ -236,6 +243,32 @@ on_error:
 	return -1;
 }
 
+static int wait_while_ack(gitno_buffer *buf)
+{
+	int error;
+	git_pkt_ack *pkt = NULL;
+
+	while (1) {
+		git__free(pkt);
+
+		if ((error = recv_pkt((git_pkt **)&pkt, buf)) < 0)
+			return error;
+
+		if (pkt->type == GIT_PKT_NAK)
+			break;
+
+		if (pkt->type == GIT_PKT_ACK &&
+		    (pkt->status != GIT_ACK_CONTINUE ||
+		     pkt->status != GIT_ACK_COMMON)) {
+			git__free(pkt);
+			break;
+		}
+	}
+
+	git__free(pkt);
+	return 0;
+}
+
 int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, const git_remote_head * const *refs, size_t count)
 {
 	transport_smart *t = (transport_smart *)transport;
@@ -287,7 +320,7 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
 				goto on_error;
 
 			git_buf_clear(&data);
-			if (t->caps.multi_ack) {
+			if (t->caps.multi_ack || t->caps.multi_ack_detailed) {
 				if ((error = store_common(t)) < 0)
 					goto on_error;
 			} else {
@@ -365,7 +398,7 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
 	git_revwalk_free(walk);
 
 	/* Now let's eat up whatever the server gives us */
-	if (!t->caps.multi_ack) {
+	if (!t->caps.multi_ack && !t->caps.multi_ack_detailed) {
 		pkt_type = recv_pkt(NULL, buf);
 
 		if (pkt_type < 0) {
@@ -375,22 +408,10 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
 			return -1;
 		}
 	} else {
-		git_pkt_ack *pkt;
-		do {
-			if ((error = recv_pkt((git_pkt **)&pkt, buf)) < 0)
-				return error;
-
-			if (pkt->type == GIT_PKT_NAK ||
-				(pkt->type == GIT_PKT_ACK && pkt->status != GIT_ACK_CONTINUE)) {
-				git__free(pkt);
-				break;
-			}
-
-			git__free(pkt);
-		} while (1);
+		error = wait_while_ack(buf);
 	}
 
-	return 0;
+	return error;
 
 on_error:
 	git_revwalk_free(walk);