Actually implement object negotiation

Only signal that we need a pack if we do need it and don't send a want
just because it's the first. If we don't need to download the pack,
then we can skip all of the negotiation and just return success.

Signed-off-by: Carlos Martín Nieto <carlos@cmartin.tk>

4 files changed, 79 insertions, 30 deletions

diff --git a/src/fetch.c b/src/fetch.c
index 03febe279..0dce875f8 100644
--- a/src/fetch.c
+++ b/src/fetch.c
@@ -75,6 +75,8 @@ static int filter_wants(git_remote *remote)
 		/* If we have the object, mark it so we don't ask for it */
 		if (git_odb_exists(repo->db, &head->oid))
 			head->local = 1;
+		else
+			remote->need_pack = 1;
 
 		error = git_vector_insert(&list, head);
 		if (error < GIT_SUCCESS)
@@ -108,11 +110,13 @@ int git_fetch_negotiate(git_remote *remote)
 	/* Don't try to negotiate when we don't want anything */
 	if (list->len == 0)
 		return GIT_SUCCESS;
+	if (!remote->need_pack)
+		return GIT_SUCCESS;
+
 	/*
 	 * Now we have everything set up so we can start tell the server
 	 * what we want and what we have.
 	 */
-	remote->need_pack = 1;
 	error = git_transport_send_wants(remote->transport, list);
 	if (error < GIT_SUCCESS)
 		return git__rethrow(error, "Failed to send want list");
diff --git a/src/netops.c b/src/netops.c
index 55cb7e45e..8126bcee3 100644
--- a/src/netops.c
+++ b/src/netops.c
@@ -55,7 +55,7 @@ int gitno_recv(gitno_buffer *buf)
 
 	ret = recv(buf->fd, buf->data + buf->offset, buf->len - buf->offset, 0);
 	if (ret < 0)
-		return git__throw(GIT_EOSERR, "Failed to receive data");
+		return git__throw(GIT_EOSERR, "Failed to receive data: %s", strerror(errno));
 	if (ret == 0) /* Orderly shutdown, so exit */
 		return GIT_SUCCESS;
 
diff --git a/src/pkt.c b/src/pkt.c
index 419fb9cf0..53701112c 100644
--- a/src/pkt.c
+++ b/src/pkt.c
@@ -299,8 +299,8 @@ static int send_want_with_caps(git_remote_head *head, git_transport_caps *caps,
 
 int git_pkt_send_wants(git_headarray *refs, git_transport_caps *caps, int fd)
 {
-	unsigned int i;
-	int ret = GIT_SUCCESS;
+	unsigned int i = 0;
+	int error = GIT_SUCCESS;
 	char buf[STRLEN(WANT_PREFIX) + GIT_OID_HEXSZ + 2];
 	git_remote_head *head;
 
@@ -308,26 +308,35 @@ int git_pkt_send_wants(git_headarray *refs, git_transport_caps *caps, int fd)
 	buf[sizeof(buf) - 2] = '\n';
 	buf[sizeof(buf) - 1] = '\0';
 
-	if (refs->len > 0 && caps->common) {
-		/* Some capabilities are active, so we need to send what we support */
-		send_want_with_caps(refs->heads[0], caps, fd);
-		i = 1;
-	} else {
-		i = 0;
+	/* If there are common caps, find the first one */
+	if (caps->common) {
+		for (; i < refs->len; ++i) {
+			head = refs->heads[i];
+			if (head->local)
+				continue;
+			else
+				break;
+		}
+
+		error = send_want_with_caps(refs->heads[i], caps, fd);
+		if (error < GIT_SUCCESS)
+			return git__rethrow(error, "Failed to send want pkt with caps");
+		/* Increase it here so it's correct whether we run this or not */
+		i++;
 	}
 
+	/* Continue from where we left off */
 	for (; i < refs->len; ++i) {
 		head = refs->heads[i];
 		if (head->local)
 			continue;
 
 		git_oid_fmt(buf + STRLEN(WANT_PREFIX), &head->oid);
-		gitno_send(fd, buf, STRLEN(buf), 0);
+		error = gitno_send(fd, buf, STRLEN(buf), 0);
+		return git__rethrow(error, "Failed to send want pkt");
 	}
 
-	git_pkt_send_flush(fd);
-
-	return ret;
+	return git_pkt_send_flush(fd);
 }
 
 /*
diff --git a/src/transport_git.c b/src/transport_git.c
index bb759a1fe..0e50a087a 100644
--- a/src/transport_git.c
+++ b/src/transport_git.c
@@ -327,7 +327,7 @@ static int git_send_have(git_transport *transport, git_oid *oid)
 	return git_pkt_send_have(oid, t->socket);
 }
 
-static int git_negotiate_fetch(git_transport *transport, git_repository *repo, git_headarray *list)
+static int git_negotiate_fetch(git_transport *transport, git_repository *repo, git_headarray *GIT_UNUSED(list))
 {
 	transport_git *t = (transport_git *) transport;
 	git_revwalk *walk;
@@ -336,6 +336,11 @@ static int git_negotiate_fetch(git_transport *transport, git_repository *repo, g
 	git_oid oid;
 	int error;
 	unsigned int i;
+	char buff[128];
+	gitno_buffer buf;
+	GIT_UNUSED_ARG(list);
+
+	gitno_buffer_setup(&buf, buff, sizeof(buff), t->socket);
 
 	error = git_reference_listall(&refs, repo, GIT_REF_LISTALL);
 	if (error < GIT_ERROR)
@@ -349,12 +354,18 @@ static int git_negotiate_fetch(git_transport *transport, git_repository *repo, g
 	git_revwalk_sorting(walk, GIT_SORT_TIME);
 
 	for (i = 0; i < refs.count; ++i) {
+		/* No tags */
+		if (!git__prefixcmp(refs.strings[i], GIT_REFS_TAGS_DIR))
+			continue;
+
 		error = git_reference_lookup(&ref, repo, refs.strings[i]);
 		if (error < GIT_ERROR) {
 			error = git__rethrow(error, "Failed to lookup %s", refs.strings[i]);
 			goto cleanup;
 		}
 
+		if (git_reference_type(ref) == GIT_REF_SYMBOLIC)
+			continue;
 		error = git_revwalk_push(walk, git_reference_oid(ref));
 		if (error < GIT_ERROR) {
 			error = git__rethrow(error, "Failed to push %s", refs.strings[i]);
@@ -372,17 +383,42 @@ static int git_negotiate_fetch(git_transport *transport, git_repository *repo, g
 	while ((error = git_revwalk_next(&oid, walk)) == GIT_SUCCESS) {
 		error = git_pkt_send_have(&oid, t->socket);
 		i++;
-		/*
-		 * This is a magic number so we don't flood the server. We
-		 * should check every once in a while to see if the server has
-		 * sent an ACK.
-		 */
-		if (i % 160 == 0)
-			break;
+		if (i % 20 == 0) {
+			const char *ptr = buf.data, *line_end;
+			git_pkt *pkt;
+			git_pkt_send_flush(t->socket);
+			while (1) {
+				error = gitno_recv(&buf);
+				if (error < GIT_SUCCESS) {
+				  error = git__rethrow(error, "Error receiving data");
+				  goto cleanup;
+				}
+				error = git_pkt_parse_line(&pkt, ptr, &line_end, buf.offset);
+				if (error == GIT_ESHORTBUFFER)
+					continue;
+				if (error < GIT_SUCCESS) {
+					error = git__rethrow(error, "Failed to get answer");
+					goto cleanup;
+				}
+
+				gitno_consume(&buf, line_end);
+
+				if (pkt->type == GIT_PKT_ACK) {
+					error = GIT_SUCCESS;
+					goto done;
+				} else if (pkt->type == GIT_PKT_NAK) {
+					break;
+				} else {
+					error = git__throw(GIT_ERROR, "Got unexpected pkt type");
+					goto cleanup;
+				}
+			}
+		}
 	}
 	if (error == GIT_EREVWALKOVER)
 		error = GIT_SUCCESS;
 
+done:
 	git_pkt_send_flush(t->socket);
 	git_pkt_send_done(t->socket);
 
@@ -426,17 +462,17 @@ static int store_pack(char **out, gitno_buffer *buf, git_repository *repo)
 		goto cleanup;
 
 	while (1) {
-		error = gitno_recv(buf);
-		if (error < GIT_SUCCESS)
-			goto cleanup;
-		if (error == 0) /* Orderly shutdown */
-			break;
-
+		/* Part of the packfile has been received, don't loose it */
 		error = git_filebuf_write(&file, buf->data, buf->offset);
 		if (error < GIT_SUCCESS)
 			goto cleanup;
 
 		gitno_consume_n(buf, buf->offset);
+		error = gitno_recv(buf);
+		if (error < GIT_SUCCESS)
+			goto cleanup;
+		if (error == 0) /* Orderly shutdown */
+			break;
 	}
 
 	*out = git__strdup(file.path_lock);
@@ -485,9 +521,9 @@ static int git_download_pack(char **out, git_transport *transport, git_repositor
 			if (error < GIT_SUCCESS)
 				return error;
 
-			if (pkt->type == GIT_PKT_PACK) {
+			if (pkt->type == GIT_PKT_PACK)
 				return store_pack(out, &buf, repo);
-			}
+
 			/* For now we don't care about anything */
 			free(pkt);
 			gitno_consume(&buf, line_end);