remote: start moving the protocol to a common area

For the transition, http is going to keep its own logic until the
git/common code catches up with the implied multi_ack that http
has. This also has the side-effect of making the code cleaner and more
correct regardingt he protocol.

examples/network/fetch.c

@@ -99,6 +99,9 @@ int fetch(git_repository *repo, int argc, char **argv)
 		printf("\rReceived %d/%d objects in %d bytes", stats.processed, stats.total, bytes);
 	} while (!data.finished);
 
+	if (data.ret < 0)
+		goto on_error;
+
 	printf("\rReceived %d/%d objects in %d bytes\n", stats.processed, stats.total, bytes);
 
 	// Disconnect the underlying connection to prevent from idling.

src/common.h

@@ -59,4 +59,7 @@ void giterr_set_regex(const regex_t *regex, int error_code);
 
 #include "util.h"
 
+typedef struct git_transport git_transport;
+typedef struct gitno_buffer gitno_buffer;
+
 #endif /* INCLUDE_common_h__ */

src/fetch.c

@@ -18,6 +18,7 @@
 #include "pack.h"
 #include "fetch.h"
 #include "netops.h"
+#include "pkt.h"
 
 struct filter_payload {
 	git_remote *remote;
@@ -73,6 +74,44 @@ static int filter_wants(git_remote *remote)
 	return remote->transport->ls(remote->transport, &filter_ref__cb, &p);
 }
 
+/* Wait until we get an ack from the */
+static int recv_pkt(gitno_buffer *buf)
+{
+	const char *ptr = buf->data, *line_end;
+	git_pkt *pkt;
+	int pkt_type, error;
+
+	do {
+		/* Wait for max. 1 second */
+		if ((error = gitno_select_in(buf, 1, 0)) < 0) {
+			return -1;
+		} else if (error == 0) {
+			/*
+			 * Some servers don't respond immediately, so if this
+			 * happens, we keep sending information until it
+			 * answers. Pretend we received a NAK to convince higher
+			 * layers to do so.
+			 */
+			return GIT_PKT_NAK;
+		}
+
+		if ((error = gitno_recv(buf)) < 0)
+			return -1;
+
+		error = git_pkt_parse_line(&pkt, ptr, &line_end, buf->offset);
+		if (error == GIT_EBUFS)
+			continue;
+		if (error < 0)
+			return -1;
+	} while (error);
+
+	gitno_consume(buf, line_end);
+	pkt_type = pkt->type;
+	git__free(pkt);
+
+	return pkt_type;
+}
+
 /*
  * In this first version, we push all our refs in and start sending
  * them out. When we get an ACK we hide that commit and continue
@@ -81,6 +120,12 @@ static int filter_wants(git_remote *remote)
 int git_fetch_negotiate(git_remote *remote)
 {
 	git_transport *t = remote->transport;
+	gitno_buffer *buf = &t->buffer;
+	git_buf data = GIT_BUF_INIT;
+	git_revwalk *walk = NULL;
+	int error, pkt_type;
+	unsigned int i;
+	git_oid oid;
 
 	if (filter_wants(remote) < 0) {
 		giterr_set(GITERR_NET, "Failed to filter the reference list for wants");
@@ -92,18 +137,90 @@ int git_fetch_negotiate(git_remote *remote)
 		return 0;
 
 	/*
-	 * Now we have everything set up so we can start tell the server
-	 * what we want and what we have.
+	 * Now we have everything set up so we can start tell the
+	 * server what we want and what we have. Call the function if
+	 * the transport has its own logic. This is transitional and
+	 * will be removed once this function can support git and http.
 	 */
-	return t->negotiate_fetch(t, remote->repo, &remote->refs);
+	if (t->own_logic)
+		return t->negotiate_fetch(t, remote->repo, &remote->refs);
+
+	/* No own logic, do our thing */
+	if (git_pkt_buffer_wants(&remote->refs, &t->caps, &data) < 0)
+		return -1;
+
+	if (git_fetch_setup_walk(&walk, remote->repo) < 0)
+		goto on_error;
+	/*
+	 * We don't support any kind of ACK extensions, so the negotiation
+	 * boils down to sending what we have and listening for an ACK
+	 * every once in a while.
+	 */
+	i = 0;
+	while ((error = git_revwalk_next(&oid, walk)) == 0) {
+		git_pkt_buffer_have(&oid, &data);
+		i++;
+		if (i % 20 == 0) {
+			git_pkt_buffer_flush(&data);
+			if (git_buf_oom(&data))
+				goto on_error;
+
+			if (t->negotiation_step(t, data.ptr, data.size) < 0)
+				goto on_error;
+
+			git_buf_clear(&data);
+			pkt_type = recv_pkt(buf);
+
+			if (pkt_type == GIT_PKT_ACK) {
+				break;
+			} else if (pkt_type == GIT_PKT_NAK) {
+				continue;
+			} else {
+				giterr_set(GITERR_NET, "Unexpected pkt type");
+				goto on_error;
+			}
+
+		}
+	}
+
+	if (error < 0 && error != GIT_REVWALKOVER)
+		goto on_error;
+
+	/* Tell the other end that we're done negotiating */
+	git_pkt_buffer_done(&data);
+	if (t->negotiation_step(t, data.ptr, data.size) < 0)
+		goto on_error;
+
+	git_buf_free(&data);
+	git_revwalk_free(walk);
+
+	/* Now let's eat up whatever the server gives us */
+	pkt_type = recv_pkt(buf);
+	if (pkt_type != GIT_PKT_ACK && pkt_type != GIT_PKT_NAK) {
+		giterr_set(GITERR_NET, "Unexpected pkt type");
+		return -1;
+	}
+
+	return 0;
+
+on_error:
+	git_revwalk_free(walk);
+	git_buf_free(&data);
+	return -1;
 }
 
 int git_fetch_download_pack(git_remote *remote, git_off_t *bytes, git_indexer_stats *stats)
 {
+	git_transport *t = remote->transport;
+
 	if(!remote->need_pack)
 		return 0;
 
-	return remote->transport->download_pack(remote->transport, remote->repo, bytes, stats);
+	if (t->own_logic)
+		return t->download_pack(t, remote->repo, bytes, stats);
+
+	return git_fetch__download_pack(NULL, 0, t, remote->repo, bytes, stats);
+
 }
 
 /* Receiving data from a socket and storing it is pretty much the same for git and HTTP */
@@ -123,7 +240,7 @@ int git_fetch__download_pack(
 
 	gitno_buffer_setup(t, &buf, buff, sizeof(buff));
 
-	if (memcmp(buffered, "PACK", strlen("PACK"))) {
+	if (buffered && memcmp(buffered, "PACK", strlen("PACK"))) {
 		giterr_set(GITERR_NET, "The pack doesn't start with the signature");
 		return -1;
 	}
@@ -135,7 +252,7 @@ int git_fetch__download_pack(
 		goto on_error;
 
 	memset(stats, 0, sizeof(git_indexer_stats));
-	if (git_indexer_stream_add(idx, buffered, buffered_size, stats) < 0)
+	if (buffered && git_indexer_stream_add(idx, buffered, buffered_size, stats) < 0)
 		goto on_error;
 
 	*bytes = buffered_size;

src/netops.h

@@ -8,10 +8,9 @@
 #define INCLUDE_netops_h__
 
 #include "posix.h"
-#include "transport.h"
 #include "common.h"
 
-typedef struct gitno_buffer {
+struct gitno_buffer {
 	char *data;
 	size_t len;
 	size_t offset;
@@ -19,7 +18,7 @@ typedef struct gitno_buffer {
 #ifdef GIT_SSL
 	struct gitno_ssl *ssl;
 #endif
-} gitno_buffer;
+};
 
 void gitno_buffer_setup(git_transport *t, gitno_buffer *buf, char *data, unsigned int len);
 int gitno_recv(gitno_buffer *buf);

src/transport.h

@@ -12,6 +12,7 @@
 #include "vector.h"
 #include "posix.h"
 #include "common.h"
+#include "netops.h"
 #ifdef GIT_SSL
 # include <openssl/ssl.h>
 # include <openssl/err.h>
@@ -70,16 +71,23 @@ struct git_transport {
 	int direction : 1, /* 0 fetch, 1 push */
 		connected : 1,
 		check_cert: 1,
-		encrypt : 1;
+		encrypt : 1,
+		own_logic: 1; /* transitional */
 #ifdef GIT_SSL
 	struct gitno_ssl ssl;
 #endif
+	gitno_buffer buffer;
 	GIT_SOCKET socket;
+	git_transport_caps caps;
 	/**
 	 * Connect and store the remote heads
 	 */
 	int (*connect)(struct git_transport *transport, int dir);
 	/**
+	 * Send our side of a negotiation
+	 */
+	int (*negotiation_step)(struct git_transport *transport, void *data, size_t len);
+	/**
 	 * Give a list of references, useful for ls-remote
 	 */
 	int (*ls)(struct git_transport *transport, git_headlist_cb list_cb, void *opaque);
@@ -124,7 +132,6 @@ int git_transport_dummy(struct git_transport **transport);
 */
 int git_transport_valid_url(const char *url);
 
-typedef struct git_transport git_transport;
 typedef int (*git_transport_cb)(git_transport **transport);
 
 #endif

src/transports/git.c

@@ -27,9 +27,7 @@ typedef struct {
 	git_protocol proto;
 	git_vector refs;
 	git_remote_head **heads;
-	git_transport_caps caps;
 	char buff[1024];
-	gitno_buffer buf;
 #ifdef GIT_WIN32
 	WSADATA wsd;
 #endif
@@ -131,7 +129,7 @@ on_error:
  */
 static int store_refs(transport_git *t)
 {
-	gitno_buffer *buf = &t->buf;
+	gitno_buffer *buf = &t->parent.buffer;
 	int ret = 0;
 
 	while (1) {
@@ -162,7 +160,7 @@ static int detect_caps(transport_git *t)
 {
 	git_vector *refs = &t->refs;
 	git_pkt_ref *pkt;
-	git_transport_caps *caps = &t->caps;
+	git_transport_caps *caps = &t->parent.caps;
 	const char *ptr;
 
 	pkt = git_vector_get(refs, 0);
@@ -209,7 +207,7 @@ static int git_connect(git_transport *transport, int direction)
 	if (do_connect(t, transport->url) < 0)
 		goto cleanup;
 
-	gitno_buffer_setup(transport, &t->buf, t->buff, sizeof(t->buff));
+	gitno_buffer_setup(transport, &transport->buffer, t->buff, sizeof(t->buff));
 
 	t->parent.connected = 1;
 	if (store_refs(t) < 0)
@@ -248,156 +246,9 @@ static int git_ls(git_transport *transport, git_headlist_cb list_cb, void *opaqu
 	return 0;
 }
 
-/* Wait until we get an ack from the */
-static int recv_pkt(gitno_buffer *buf)
+static int git_negotiation_step(struct git_transport *transport, void *data, size_t len)
 {
-	const char *ptr = buf->data, *line_end;
-	git_pkt *pkt;
-	int pkt_type, error;
-
-	do {
-		/* Wait for max. 1 second */
-		if ((error = gitno_select_in(buf, 1, 0)) < 0) {
-			return -1;
-		} else if (error == 0) {
-			/*
-			 * Some servers don't respond immediately, so if this
-			 * happens, we keep sending information until it
-			 * answers. Pretend we received a NAK to convince higher
-			 * layers to do so.
-			 */
-			return GIT_PKT_NAK;
-		}
-
-		if ((error = gitno_recv(buf)) < 0)
-			return -1;
-
-		error = git_pkt_parse_line(&pkt, ptr, &line_end, buf->offset);
-		if (error == GIT_EBUFS)
-			continue;
-		if (error < 0)
-			return -1;
-	} while (error);
-
-	gitno_consume(buf, line_end);
-	pkt_type = pkt->type;
-	git__free(pkt);
-
-	return pkt_type;
-}
-
-static int git_negotiate_fetch(git_transport *transport, git_repository *repo, const git_vector *wants)
-{
-	transport_git *t = (transport_git *) transport;
-	git_revwalk *walk;
-	git_oid oid;
-	int error;
-	unsigned int i;
-	git_buf data = GIT_BUF_INIT;
-	gitno_buffer *buf = &t->buf;
-
-	if (git_pkt_buffer_wants(wants, &t->caps, &data) < 0)
-		return -1;
-
-	if (git_fetch_setup_walk(&walk, repo) < 0)
-		goto on_error;
-
-	if (gitno_send(transport, data.ptr, data.size, 0) < 0)
-		goto on_error;
-
-	git_buf_clear(&data);
-	/*
-	 * We don't support any kind of ACK extensions, so the negotiation
-	 * boils down to sending what we have and listening for an ACK
-	 * every once in a while.
-	 */
-	i = 0;
-	while ((error = git_revwalk_next(&oid, walk)) == 0) {
-		git_pkt_buffer_have(&oid, &data);
-		i++;
-		if (i % 20 == 0) {
-			int pkt_type;
-
-			git_pkt_buffer_flush(&data);
-			if (git_buf_oom(&data))
-				goto on_error;
-
-			if (gitno_send(transport, data.ptr, data.size, 0) < 0)
-				goto on_error;
-
-			pkt_type = recv_pkt(buf);
-
-			if (pkt_type == GIT_PKT_ACK) {
-				break;
-			} else if (pkt_type == GIT_PKT_NAK) {
-				continue;
-			} else {
-				giterr_set(GITERR_NET, "Unexpected pkt type");
-				goto on_error;
-			}
-
-		}
-	}
-	if (error < 0 && error != GIT_REVWALKOVER)
-		goto on_error;
-
-	/* Tell the other end that we're done negotiating */
-	git_buf_clear(&data);
-	git_pkt_buffer_flush(&data);
-	git_pkt_buffer_done(&data);
-	if (gitno_send(transport, data.ptr, data.size, 0) < 0)
-		goto on_error;
-
-	git_buf_free(&data);
-	git_revwalk_free(walk);
-	return 0;
-
-on_error:
-	git_buf_free(&data);
-	git_revwalk_free(walk);
-	return -1;
-}
-
-static int git_download_pack(git_transport *transport, git_repository *repo, git_off_t *bytes, git_indexer_stats *stats)
-{
-	transport_git *t = (transport_git *) transport;
-	int error = 0, read_bytes;
-	gitno_buffer *buf = &t->buf;
-	git_pkt *pkt;
-	const char *line_end, *ptr;
-
-	/*
-	 * For now, we ignore everything and wait for the pack
-	 */
-	do {
-		ptr = buf->data;
-		/* Whilst we're searching for the pack */
-		while (1) {
-			if (buf->offset == 0) {
-				break;
-			}
-
-			error = git_pkt_parse_line(&pkt, ptr, &line_end, buf->offset);
-			if (error == GIT_EBUFS)
-				break;
-
-			if (error < 0)
-				return error;
-
-			if (pkt->type == GIT_PKT_PACK) {
-				git__free(pkt);
-				return git_fetch__download_pack(buf->data, buf->offset, transport, repo, bytes, stats);
-			}
-
-			/* For now we don't care about anything */
-			git__free(pkt);
-			gitno_consume(buf, line_end);
-		}
-
-		read_bytes = gitno_recv(buf);
-	} while (read_bytes);
-
-	return read_bytes;
+	return gitno_send(transport, data, len, 0);
 }
 
 static int git_close(git_transport *t)
@@ -454,9 +305,8 @@ int git_transport_git(git_transport **out)
 	memset(t, 0x0, sizeof(transport_git));
 
 	t->parent.connect = git_connect;
+	t->parent.negotiation_step = git_negotiation_step;
 	t->parent.ls = git_ls;
-	t->parent.negotiate_fetch = git_negotiate_fetch;
-	t->parent.download_pack = git_download_pack;
 	t->parent.close = git_close;
 	t->parent.free = git_free;
 	t->proto.refs = &t->refs;

src/transports/http.c

@@ -668,6 +668,7 @@ int git_transport_http(git_transport **out)
 
 	memset(t, 0x0, sizeof(transport_http));
 
+	t->parent.own_logic = 1;
 	t->parent.connect = http_connect;
 	t->parent.ls = http_ls;
 	t->parent.negotiate_fetch = http_negotiate_fetch;