Merge pull request #2284 from jacquesg/push-progress-callback

Fire progress and update tips callbacks also for pushes.

11 files changed, 89 insertions, 76 deletions

diff --git a/src/fetch.h b/src/fetch.h
index 9605da1b5..f66e44663 100644
--- a/src/fetch.h
+++ b/src/fetch.h
@@ -17,7 +17,7 @@ int git_fetch__download_pack(
 		git_transport *t,
 		git_repository *repo,
 		git_transfer_progress *stats,
-		git_transfer_progress_callback progress_cb,
+		git_transfer_progress_cb progress_cb,
 		void *progress_payload);
 
 int git_fetch_setup_walk(git_revwalk **out, git_repository *repo);
diff --git a/src/indexer.c b/src/indexer.c
index 346870faa..adf5ceaa7 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -45,7 +45,7 @@ struct git_indexer {
 	unsigned int fanout[256];
 	git_hash_ctx hash_ctx;
 	git_oid hash;
-	git_transfer_progress_callback progress_cb;
+	git_transfer_progress_cb progress_cb;
 	void *progress_payload;
 	char objbuf[8*1024];
 
@@ -120,7 +120,7 @@ int git_indexer_new(
 		const char *prefix,
 		unsigned int mode,
 		git_odb *odb,
-		git_transfer_progress_callback progress_cb,
+		git_transfer_progress_cb progress_cb,
 		void *progress_payload)
 {
 	git_indexer *idx;
diff --git a/src/odb.c b/src/odb.c
index 72d150658..00740d2e2 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -1051,7 +1051,7 @@ int git_odb_open_rstream(git_odb_stream **stream, git_odb *db, const git_oid *oi
 	return error;
 }
 
-int git_odb_write_pack(struct git_odb_writepack **out, git_odb *db, git_transfer_progress_callback progress_cb, void *progress_payload)
+int git_odb_write_pack(struct git_odb_writepack **out, git_odb *db, git_transfer_progress_cb progress_cb, void *progress_payload)
 {
 	size_t i, writes = 0;
 	int error = GIT_ERROR;
diff --git a/src/odb_pack.c b/src/odb_pack.c
index 9ab683882..3750da37f 100644
--- a/src/odb_pack.c
+++ b/src/odb_pack.c
@@ -563,7 +563,7 @@ static void pack_backend__writepack_free(struct git_odb_writepack *_writepack)
 static int pack_backend__writepack(struct git_odb_writepack **out,
 	git_odb_backend *_backend,
         git_odb *odb,
-	git_transfer_progress_callback progress_cb,
+	git_transfer_progress_cb progress_cb,
 	void *progress_payload)
 {
 	struct pack_backend *backend;
diff --git a/src/pack-objects.c b/src/pack-objects.c
index c881e6d99..7e5f667f4 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -1288,7 +1288,7 @@ int git_packbuilder_write(
 	git_packbuilder *pb,
 	const char *path,
 	unsigned int mode,
-	git_transfer_progress_callback progress_cb,
+	git_transfer_progress_cb progress_cb,
 	void *progress_cb_payload)
 {
 	git_indexer *indexer;
diff --git a/src/push.c b/src/push.c
index 5c8de3339..9943f215c 100644
--- a/src/push.c
+++ b/src/push.c
@@ -208,9 +208,7 @@ int git_push_update_tips(
 	int error = 0;
 
 	git_vector_foreach(&push->status, i, status) {
-		/* If this ref update was successful (ok, not ng), it will have an empty message */
-		if (status->msg)
-			continue;
+		int fire_callback = 1;
 
 		/* Find the corresponding remote ref */
 		fetch_spec = git_remote__matching_refspec(push->remote, status->ref);
@@ -230,24 +228,38 @@ int git_push_update_tips(
 		if (j == push->specs.length)
 			continue;
 
-		/* Update the remote ref */
-		if (git_oid_iszero(&push_spec->loid)) {
-			error = git_reference_lookup(&remote_ref, push->remote->repo, git_buf_cstr(&remote_ref_name));
+		/* If this ref update was successful (ok, not ng), it will have an empty message */
+		if (status->msg == NULL) {
+			/* Update the remote ref */
+			if (git_oid_iszero(&push_spec->loid)) {
+				error = git_reference_lookup(&remote_ref, push->remote->repo, git_buf_cstr(&remote_ref_name));
 
-			if (!error) {
-				if ((error = git_reference_delete(remote_ref)) < 0) {
+				if (error >= 0) {
+					error = git_reference_delete(remote_ref);
 					git_reference_free(remote_ref);
-					goto on_error;
 				}
-				git_reference_free(remote_ref);
-			} else if (error == GIT_ENOTFOUND)
-				giterr_clear();
-			else
+			} else {
+				error = git_reference_create(NULL, push->remote->repo,
+							git_buf_cstr(&remote_ref_name), &push_spec->loid, 1, signature,
+							reflog_message ? reflog_message : "update by push");
+			}
+		}
+
+		if (error < 0) {
+			if (error != GIT_ENOTFOUND)
 				goto on_error;
-		} else if ((error = git_reference_create(NULL, push->remote->repo,
-						git_buf_cstr(&remote_ref_name), &push_spec->loid, 1, signature,
-						reflog_message ? reflog_message : "update by push")) < 0)
-			goto on_error;
+
+			giterr_clear();
+			fire_callback = 0;
+		}
+
+		if (fire_callback && push->remote->callbacks.update_tips) {
+			error = push->remote->callbacks.update_tips(git_buf_cstr(&remote_ref_name),
+						&push_spec->roid, &push_spec->loid, push->remote->callbacks.payload);
+
+			if (error < 0)
+				goto on_error;
+		}
 	}
 
 	error = 0;
@@ -677,9 +689,7 @@ void git_push_status_free(push_status *status)
 	if (status == NULL)
 		return;
 
-	if (status->msg)
-		git__free(status->msg);
-
+	git__free(status->msg);
 	git__free(status->ref);
 	git__free(status);
 }
diff --git a/src/remote.c b/src/remote.c
index c23a4643e..ea638e373 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -663,7 +663,7 @@ int git_remote_connect(git_remote *remote, git_direction direction)
 		return error;
 
 	if (t->set_callbacks &&
-		(error = t->set_callbacks(t, remote->callbacks.progress, NULL, remote->callbacks.payload)) < 0)
+		(error = t->set_callbacks(t, remote->callbacks.sideband_progress, NULL, remote->callbacks.payload)) < 0)
 		goto on_error;
 
 	if (!remote->check_cert)
@@ -1246,7 +1246,7 @@ int git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *cal
 
 	if (remote->transport && remote->transport->set_callbacks)
 		return remote->transport->set_callbacks(remote->transport,
-			remote->callbacks.progress,
+			remote->callbacks.sideband_progress,
 			NULL,
 			remote->callbacks.payload);
 
diff --git a/src/transports/local.c b/src/transports/local.c
index f8d511ed6..2c17e6271 100644
--- a/src/transports/local.c
+++ b/src/transports/local.c
@@ -472,7 +472,7 @@ on_error:
 
 typedef struct foreach_data {
 	git_transfer_progress *stats;
-	git_transfer_progress_callback progress_cb;
+	git_transfer_progress_cb progress_cb;
 	void *progress_payload;
 	git_odb_writepack *writepack;
 } foreach_data;
@@ -489,7 +489,7 @@ static int local_download_pack(
 		git_transport *transport,
 		git_repository *repo,
 		git_transfer_progress *stats,
-		git_transfer_progress_callback progress_cb,
+		git_transfer_progress_cb progress_cb,
 		void *progress_payload)
 {
 	transport_local *t = (transport_local*)transport;
diff --git a/src/transports/smart.h b/src/transports/smart.h
index 32f0be7f2..a2b6b2a71 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -167,7 +167,7 @@ int git_smart__download_pack(
 	git_transport *transport,
 	git_repository *repo,
 	git_transfer_progress *stats,
-	git_transfer_progress_callback progress_cb,
+	git_transfer_progress_cb progress_cb,
 	void *progress_payload);
 
 /* smart.c */
diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 2bb09c750..e9376ae6f 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -153,7 +153,7 @@ static int data_pkt(git_pkt **out, const char *line, size_t len)
 	return 0;
 }
 
-static int progress_pkt(git_pkt **out, const char *line, size_t len)
+static int sideband_progress_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt_progress *pkt;
 
@@ -403,7 +403,7 @@ int git_pkt_parse_line(
 	if (*line == GIT_SIDE_BAND_DATA)
 		ret = data_pkt(head, line, len);
 	else if (*line == GIT_SIDE_BAND_PROGRESS)
-		ret = progress_pkt(head, line, len);
+		ret = sideband_progress_pkt(head, line, len);
 	else if (*line == GIT_SIDE_BAND_ERROR)
 		ret = sideband_error_pkt(head, line, len);
 	else if (!git__prefixcmp(line, "ACK"))
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index 7e8fcdd92..5dd6bab24 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -450,7 +450,7 @@ static int no_sideband(transport_smart *t, struct git_odb_writepack *writepack,
 
 struct network_packetsize_payload
 {
-	git_transfer_progress_callback callback;
+	git_transfer_progress_cb callback;
 	void *payload;
 	git_transfer_progress *stats;
 	size_t last_fired_bytes;
@@ -478,7 +478,7 @@ int git_smart__download_pack(
 	git_transport *transport,
 	git_repository *repo,
 	git_transfer_progress *stats,
-	git_transfer_progress_callback progress_cb,
+	git_transfer_progress_cb transfer_progress_cb,
 	void *progress_payload)
 {
 	transport_smart *t = (transport_smart *)transport;
@@ -490,8 +490,8 @@ int git_smart__download_pack(
 
 	memset(stats, 0, sizeof(git_transfer_progress));
 
-	if (progress_cb) {
-		npp.callback = progress_cb;
+	if (transfer_progress_cb) {
+		npp.callback = transfer_progress_cb;
 		npp.payload = progress_payload;
 		npp.stats = stats;
 		t->packetsize_cb = &network_packetsize;
@@ -504,7 +504,7 @@ int git_smart__download_pack(
 	}
 
 	if ((error = git_repository_odb__weakptr(&odb, repo)) < 0 ||
-		((error = git_odb_write_pack(&writepack, odb, progress_cb, progress_payload)) != 0))
+		((error = git_odb_write_pack(&writepack, odb, transfer_progress_cb, progress_payload)) != 0))
 		goto done;
 
 	/*
@@ -518,7 +518,7 @@ int git_smart__download_pack(
 	}
 
 	do {
-		git_pkt *pkt;
+		git_pkt *pkt = NULL;
 
 		/* Check cancellation before network call */
 		if (t->cancelled.val) {
@@ -527,40 +527,34 @@ int git_smart__download_pack(
 			goto done;
 		}
 
-		if ((error = recv_pkt(&pkt, buf)) < 0)
-			goto done;
-
-		/* Check cancellation after network call */
-		if (t->cancelled.val) {
-			giterr_clear();
-			error = GIT_EUSER;
-			goto done;
+		if ((error = recv_pkt(&pkt, buf)) >= 0) {
+			/* Check cancellation after network call */
+			if (t->cancelled.val) {
+				giterr_clear();
+				error = GIT_EUSER;
+			} else if (pkt->type == GIT_PKT_PROGRESS) {
+				if (t->progress_cb) {
+					git_pkt_progress *p = (git_pkt_progress *) pkt;
+					error = t->progress_cb(p->data, p->len, t->message_cb_payload);
+				}
+			} else if (pkt->type == GIT_PKT_DATA) {
+				git_pkt_data *p = (git_pkt_data *) pkt;
+				error = writepack->append(writepack, p->data, p->len, stats);
+			} else if (pkt->type == GIT_PKT_FLUSH) {
+				/* A flush indicates the end of the packfile */
+				git__free(pkt);
+				break;
+			}
 		}
 
-		if (pkt->type == GIT_PKT_PROGRESS) {
-			if (t->progress_cb) {
-				git_pkt_progress *p = (git_pkt_progress *) pkt;
-				error = t->progress_cb(p->data, p->len, t->message_cb_payload);
-				if (error)
-					goto done;
-			}
-			git__free(pkt);
-		} else if (pkt->type == GIT_PKT_DATA) {
-			git_pkt_data *p = (git_pkt_data *) pkt;
-			error = writepack->append(writepack, p->data, p->len, stats);
+		git__free(pkt);
+		if (error < 0)
+			goto done;
 
-			git__free(pkt);
-			if (error != 0)
-				goto done;
-		} else if (pkt->type == GIT_PKT_FLUSH) {
-			/* A flush indicates the end of the packfile */
-			git__free(pkt);
-			break;
-		}
 	} while (1);
 
 	/*
-	 * Trailing execution of progress_cb, if necessary...
+	 * Trailing execution of transfer_progress_cb, if necessary...
 	 * Only the callback through the npp datastructure currently
 	 * updates the last_fired_bytes value. It is possible that
 	 * progress has already been reported with the correct
@@ -579,7 +573,7 @@ int git_smart__download_pack(
 done:
 	if (writepack)
 		writepack->free(writepack);
-	if (progress_cb) {
+	if (transfer_progress_cb) {
 		t->packetsize_cb = NULL;
 		t->packetsize_payload = NULL;
 	}
@@ -634,7 +628,7 @@ static int add_push_report_pkt(git_push *push, git_pkt *pkt)
 
 	switch (pkt->type) {
 		case GIT_PKT_OK:
-			status = git__calloc(1, sizeof(push_status));
+			status = git__calloc(sizeof(push_status), 1);
 			GITERR_CHECK_ALLOC(status);
 			status->msg = NULL;
 			status->ref = git__strdup(((git_pkt_ok *)pkt)->ref);
@@ -696,10 +690,11 @@ static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt)
 	return 0;
 }
 
-static int parse_report(gitno_buffer *buf, git_push *push)
+static int parse_report(transport_smart *transport, git_push *push)
 {
 	git_pkt *pkt = NULL;
 	const char *line_end = NULL;
+	gitno_buffer *buf = &transport->buffer;
 	int error, recvd;
 
 	for (;;) {
@@ -738,6 +733,10 @@ static int parse_report(gitno_buffer *buf, git_push *push)
 				error = -1;
 				break;
 			case GIT_PKT_PROGRESS:
+				if (transport->progress_cb) {
+					git_pkt_progress *p = (git_pkt_progress *) pkt;
+					error = transport->progress_cb(p->data, p->len, transport->message_cb_payload);
+				}
 				break;
 			default:
 				error = add_push_report_pkt(push, pkt);
@@ -883,10 +882,7 @@ static int stream_thunk(void *buf, size_t size, void *data)
 
 		if ((current_time - payload->last_progress_report_time) >= MIN_PROGRESS_UPDATE_INTERVAL) {
 			payload->last_progress_report_time = current_time;
-			if (payload->cb(payload->pb->nr_written, payload->pb->nr_objects, payload->last_bytes, payload->cb_payload)) {
-				giterr_clear();
-				error = GIT_EUSER;
-			}
+			error = payload->cb(payload->pb->nr_written, payload->pb->nr_objects, payload->last_bytes, payload->cb_payload);
 		}
 	}
 
@@ -953,12 +949,19 @@ int git_smart__push(git_transport *transport, git_push *push)
 	 * we consider the pack to have been unpacked successfully */
 	if (!push->specs.length || !push->report_status)
 		push->unpack_ok = 1;
-	else if ((error = parse_report(&t->buffer, push)) < 0)
+	else if ((error = parse_report(t, push)) < 0)
 		goto done;
 
 	/* If progress is being reported write the final report */
 	if (push->transfer_progress_cb) {
-		push->transfer_progress_cb(push->pb->nr_written, push->pb->nr_objects, packbuilder_payload.last_bytes, push->transfer_progress_cb_payload);
+		error = push->transfer_progress_cb(
+					push->pb->nr_written,
+					push->pb->nr_objects,
+					packbuilder_payload.last_bytes,
+					push->transfer_progress_cb_payload);
+
+		if (error < 0)
+			goto done;
 	}
 
 	if (push->status.length) {