Merge pull request #1981 from jamill/download_cancel_tweaks

Updates to cancellation logic during download and indexing of packfile.

2 files changed, 39 insertions, 19 deletions

diff --git a/src/indexer.c b/src/indexer.c
index df1ce7cfb..852a04120 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -386,8 +386,13 @@ on_error:
 
 static int do_progress_callback(git_indexer *idx, git_transfer_progress *stats)
 {
-	if (!idx->progress_cb) return 0;
-	return idx->progress_cb(stats, idx->progress_payload);
+	if (idx->progress_cb &&
+		idx->progress_cb(stats, idx->progress_payload)) {
+			giterr_clear();
+			return GIT_EUSER;
+	}
+
+	return 0;
 }
 
 /* Hash everything but the last 20B of input */
@@ -491,7 +496,9 @@ int git_indexer_append(git_indexer *idx, const void *data, size_t size, git_tran
 		stats->indexed_deltas = 0;
 		processed = stats->indexed_objects = 0;
 		stats->total_objects = total_objects;
-		do_progress_callback(idx, stats);
+
+		if ((error = do_progress_callback(idx, stats)) < 0)
+			return error;
 	}
 
 	/* Now that we have data in the pack, let's try to parse it */
@@ -573,11 +580,8 @@ int git_indexer_append(git_indexer *idx, const void *data, size_t size, git_tran
 		}
 		stats->received_objects++;
 
-		if (do_progress_callback(idx, stats) != 0) {
-			giterr_clear();
-			error = GIT_EUSER;
+		if ((error = do_progress_callback(idx, stats)) < 0)
 			goto on_error;
-		}
 	}
 
 	return 0;
@@ -749,7 +753,7 @@ static int resolve_deltas(git_indexer *idx, git_transfer_progress *stats)
 {
 	unsigned int i;
 	struct delta_info *delta;
-	int progressed = 0;
+	int progressed = 0, progress_cb_result;
 
 	while (idx->deltas.length > 0) {
 		progressed = 0;
@@ -767,7 +771,8 @@ static int resolve_deltas(git_indexer *idx, git_transfer_progress *stats)
 			stats->indexed_objects++;
 			stats->indexed_deltas++;
 			progressed = 1;
-			do_progress_callback(idx, stats);
+			if ((progress_cb_result = do_progress_callback(idx, stats)) < 0)
+				return progress_cb_result;
 
 			/*
 			 * Remove this delta from the list and
@@ -841,6 +846,7 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
 {
 	git_mwindow *w = NULL;
 	unsigned int i, long_offsets = 0, left;
+	int error;
 	struct git_pack_idx_header hdr;
 	git_buf filename = GIT_BUF_INIT;
 	struct entry *entry;
@@ -877,8 +883,8 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
 	/* Freeze the number of deltas */
 	stats->total_deltas = stats->total_objects - stats->indexed_objects;
 
-	if (resolve_deltas(idx, stats) < 0)
-		return -1;
+	if ((error = resolve_deltas(idx, stats)) < 0)
+		return error;
 
 	if (stats->indexed_objects != stats->total_objects) {
 		giterr_set(GITERR_INDEXER, "early EOF");
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index 3bf1f9329..a4046ee43 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -521,7 +521,7 @@ int git_smart__download_pack(
 
 		/* Check cancellation before network call */
 		if (t->cancelled.val) {
-			giterr_set(GITERR_NET, "The fetch was cancelled by the user");
+			giterr_clear();
 			error = GIT_EUSER;
 			goto done;
 		}
@@ -531,7 +531,7 @@ int git_smart__download_pack(
 
 		/* Check cancellation after network call */
 		if (t->cancelled.val) {
-			giterr_set(GITERR_NET, "The fetch was cancelled by the user");
+			giterr_clear();
 			error = GIT_EUSER;
 			goto done;
 		}
@@ -540,8 +540,9 @@ int git_smart__download_pack(
 			if (t->progress_cb) {
 				git_pkt_progress *p = (git_pkt_progress *) pkt;
 				if (t->progress_cb(p->data, p->len, t->message_cb_payload)) {
-					giterr_set(GITERR_NET, "The fetch was cancelled by the user");
-					return GIT_EUSER;
+					giterr_clear();
+					error = GIT_EUSER;
+					goto done;
 				}
 			}
 			git__free(pkt);
@@ -559,16 +560,29 @@ int git_smart__download_pack(
 		}
 	} while (1);
 
+	/*
+	 * Trailing execution of 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 
+	 * "received_bytes" value, but until (if?) this is unified
+	 * then we will report progress again to be sure that the
+	 * correct last received_bytes value is reported.
+	 */
+	if (npp.callback && npp.stats->received_bytes > npp.last_fired_bytes) {
+		if (npp.callback(npp.stats, npp.payload) < 0) {
+			giterr_clear();
+			error = GIT_EUSER;
+			goto done;
+		}
+	}
+
 	error = writepack->commit(writepack, stats);
 
 done:
 	if (writepack)
 		writepack->free(writepack);
 
-	/* Trailing execution of progress_cb, if necessary */
-	if (npp.callback && npp.stats->received_bytes > npp.last_fired_bytes)
-		npp.callback(npp.stats, npp.payload);
-
 	return error;
 }