Commit 146b4d1c5f98aa14df086503f996d131d40b92f8

Vicent Martí 2013-10-03T08:18:41

Merge pull request #1888 from jamill/network_cancellation network cancellation improvements

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
diff --git a/include/git2/pack.h b/include/git2/pack.h
index 7ebdd5c..7488176 100644
--- a/include/git2/pack.h
+++ b/include/git2/pack.h
@@ -158,7 +158,7 @@ GIT_EXTERN(uint32_t) git_packbuilder_object_count(git_packbuilder *pb);
 GIT_EXTERN(uint32_t) git_packbuilder_written(git_packbuilder *pb);
 
 /** Packbuilder progress notification function */
-typedef void (*git_packbuilder_progress)(
+typedef int (*git_packbuilder_progress)(
 	int stage,
 	unsigned int current,
 	unsigned int total,
diff --git a/include/git2/push.h b/include/git2/push.h
index ecfd862..77ef740 100644
--- a/include/git2/push.h
+++ b/include/git2/push.h
@@ -40,7 +40,7 @@ typedef struct {
 #define GIT_PUSH_OPTIONS_INIT { GIT_PUSH_OPTIONS_VERSION }
 
 /** Push network progress notification function */
-typedef void (*git_push_transfer_progress)(
+typedef int (*git_push_transfer_progress)(
 	unsigned int current,
 	unsigned int total,
 	size_t bytes,
diff --git a/src/indexer.c b/src/indexer.c
index 3b160df..4ce69fc 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -556,6 +556,7 @@ int git_indexer_stream_add(git_indexer_stream *idx, const void *data, size_t siz
 		stats->received_objects++;
 
 		if (do_progress_callback(idx, stats) != 0) {
+			giterr_clear();
 			error = GIT_EUSER;
 			goto on_error;
 		}
diff --git a/src/pack-objects.c b/src/pack-objects.c
index 2a2f362..821f292 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -216,15 +216,19 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid,
 	assert(ret != 0);
 	kh_value(pb->object_ix, pos) = po;
 
+	pb->done = false;
+
 	if (pb->progress_cb) {
 		double current_time = git__timer();
 		if ((current_time - pb->last_progress_report_time) >= MIN_PROGRESS_UPDATE_INTERVAL) {
 			pb->last_progress_report_time = current_time;
-			pb->progress_cb(GIT_PACKBUILDER_ADDING_OBJECTS, pb->nr_objects, 0, pb->progress_cb_payload);
+			if (pb->progress_cb(GIT_PACKBUILDER_ADDING_OBJECTS, pb->nr_objects, 0, pb->progress_cb_payload)) {
+				giterr_clear();
+				return GIT_EUSER;
+			}
 		}
 	}
 
-	pb->done = false;
 	return 0;
 }
 
@@ -591,49 +595,50 @@ static int write_pack(git_packbuilder *pb,
 	enum write_one_status status;
 	struct git_pack_header ph;
 	unsigned int i = 0;
+	int error = 0;
 
 	write_order = compute_write_order(pb);
-	if (write_order == NULL)
-		goto on_error;
+	if (write_order == NULL) {
+		error = -1;
+		goto done;
+	}
 
 	/* Write pack header */
 	ph.hdr_signature = htonl(PACK_SIGNATURE);
 	ph.hdr_version = htonl(PACK_VERSION);
 	ph.hdr_entries = htonl(pb->nr_objects);
 
-	if (cb(&ph, sizeof(ph), data) < 0)
-		goto on_error;
+	if ((error = cb(&ph, sizeof(ph), data)) < 0)
+		goto done;
 
-	if (git_hash_update(&pb->ctx, &ph, sizeof(ph)) < 0)
-		goto on_error;
+	if ((error = git_hash_update(&pb->ctx, &ph, sizeof(ph))) < 0)
+		goto done;
 
 	pb->nr_remaining = pb->nr_objects;
 	do {
 		pb->nr_written = 0;
 		for ( ; i < pb->nr_objects; ++i) {
 			po = write_order[i];
-			if (write_one(&buf, pb, po, &status) < 0)
-				goto on_error;
-			if (cb(buf.ptr, buf.size, data) < 0)
-				goto on_error;
+			if ((error = write_one(&buf, pb, po, &status)) < 0)
+				goto done;
+			if ((error = cb(buf.ptr, buf.size, data)) < 0)
+				goto done;
 			git_buf_clear(&buf);
 		}
 
 		pb->nr_remaining -= pb->nr_written;
 	} while (pb->nr_remaining && i < pb->nr_objects);
 
-	git__free(write_order);
-	git_buf_free(&buf);
 
-	if (git_hash_final(&pb->pack_oid, &pb->ctx) < 0)
-		goto on_error;
+	if ((error = git_hash_final(&pb->pack_oid, &pb->ctx)) < 0)
+		goto done;
 
-	return cb(pb->pack_oid.id, GIT_OID_RAWSZ, data);
+	error = cb(pb->pack_oid.id, GIT_OID_RAWSZ, data);
 
-on_error:
+done:
 	git__free(write_order);
 	git_buf_free(&buf);
-	return -1;
+	return error;
 }
 
 static int write_pack_buf(void *buf, size_t size, void *data)
diff --git a/src/push.c b/src/push.c
index 6980792..a799db8 100644
--- a/src/push.c
+++ b/src/push.c
@@ -582,7 +582,7 @@ static int calculate_work(git_push *push)
 
 static int do_push(git_push *push)
 {
-	int error;
+	int error = 0;
 	git_transport *transport = push->remote->transport;
 
 	if (!transport->push) {
@@ -611,8 +611,6 @@ static int do_push(git_push *push)
 		(error = transport->push(transport, push)) < 0)
 		goto on_error;
 
-	error = 0;
-
 on_error:
 	git_packbuilder_free(push->pb);
 	return error;
diff --git a/src/transports/smart.c b/src/transports/smart.c
index 416eb22..a681d5f 100644
--- a/src/transports/smart.c
+++ b/src/transports/smart.c
@@ -23,8 +23,13 @@ static int git_smart__recv_cb(gitno_buffer *buf)
 
 	buf->offset += bytes_read;
 
-	if (t->packetsize_cb)
-		t->packetsize_cb(bytes_read, t->packetsize_payload);
+	if (t->packetsize_cb && !t->cancelled.val)
+		if (t->packetsize_cb(bytes_read, t->packetsize_payload)) {
+			git_atomic_set(&t->cancelled, 1);
+
+			giterr_clear();
+			return GIT_EUSER;
+		}
 
 	return (int)(buf->offset - old_len);
 }
diff --git a/src/transports/smart.h b/src/transports/smart.h
index c52401a..3519477 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -119,7 +119,7 @@ typedef struct transport_smart_caps {
 		report_status:1;
 } transport_smart_caps;
 
-typedef void (*packetsize_cb)(size_t received, void *payload);
+typedef int (*packetsize_cb)(size_t received, void *payload);
 
 typedef struct {
 	git_transport parent;
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index 156b69e..2467790 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -425,7 +425,7 @@ struct network_packetsize_payload
 	size_t last_fired_bytes;
 };
 
-static void network_packetsize(size_t received, void *payload)
+static int network_packetsize(size_t received, void *payload)
 {
 	struct network_packetsize_payload *npp = (struct network_packetsize_payload*)payload;
 
@@ -435,8 +435,12 @@ static void network_packetsize(size_t received, void *payload)
 	/* Fire notification if the threshold is reached */
 	if ((npp->stats->received_bytes - npp->last_fired_bytes) > NETWORK_XFER_THRESHOLD) {
 		npp->last_fired_bytes = npp->stats->received_bytes;
-		npp->callback(npp->stats, npp->payload);
+
+		if (npp->callback(npp->stats, npp->payload))
+			return GIT_EUSER;
 	}
+
+	return 0;
 }
 
 int git_smart__download_pack(
@@ -450,7 +454,7 @@ int git_smart__download_pack(
 	gitno_buffer *buf = &t->buffer;
 	git_odb *odb;
 	struct git_odb_writepack *writepack = NULL;
-	int error = -1;
+	int error = 0;
 	struct network_packetsize_payload npp = {0};
 
 	memset(stats, 0, sizeof(git_transfer_progress));
@@ -463,13 +467,14 @@ int git_smart__download_pack(
 		t->packetsize_payload = &npp;
 
 		/* We might have something in the buffer already from negotiate_fetch */
-		if (t->buffer.offset > 0)
-			t->packetsize_cb(t->buffer.offset, t->packetsize_payload);
+		if (t->buffer.offset > 0 && !t->cancelled.val)
+			if (t->packetsize_cb(t->buffer.offset, t->packetsize_payload))
+				git_atomic_set(&t->cancelled, 1);
 	}
 
 	if ((error = git_repository_odb__weakptr(&odb, repo)) < 0 ||
 		((error = git_odb_write_pack(&writepack, odb, progress_cb, progress_payload)) < 0))
-		goto on_error;
+		goto done;
 
 	/*
 	 * If the remote doesn't support the side-band, we can feed
@@ -477,23 +482,29 @@ int git_smart__download_pack(
 	 * check which one belongs there.
 	 */
 	if (!t->caps.side_band && !t->caps.side_band_64k) {
-		if (no_sideband(t, writepack, buf, stats) < 0)
-			goto on_error;
-
-		goto on_success;
+		error = no_sideband(t, writepack, buf, stats);
+		goto done;
 	}
 
 	do {
 		git_pkt *pkt;
 
+		/* Check cancellation before network call */
 		if (t->cancelled.val) {
 			giterr_set(GITERR_NET, "The fetch was cancelled by the user");
 			error = GIT_EUSER;
-			goto on_error;
+			goto done;
 		}
 
-		if (recv_pkt(&pkt, buf) < 0)
-			goto on_error;
+		if ((error = recv_pkt(&pkt, buf)) < 0)
+			goto done;
+
+		/* Check cancellation after network call */
+		if (t->cancelled.val) {
+			giterr_set(GITERR_NET, "The fetch was cancelled by the user");
+			error = GIT_EUSER;
+			goto done;
+		}
 
 		if (pkt->type == GIT_PKT_PROGRESS) {
 			if (t->progress_cb) {
@@ -507,7 +518,7 @@ int git_smart__download_pack(
 
 			git__free(pkt);
 			if (error < 0)
-				goto on_error;
+				goto done;
 		} else if (pkt->type == GIT_PKT_FLUSH) {
 			/* A flush indicates the end of the packfile */
 			git__free(pkt);
@@ -515,13 +526,9 @@ int git_smart__download_pack(
 		}
 	} while (1);
 
-	if (writepack->commit(writepack, stats) < 0)
-		goto on_error;
-
-on_success:
-	error = 0;
+	error = writepack->commit(writepack, stats);
 
-on_error:
+done:
 	if (writepack)
 		writepack->free(writepack);
 
@@ -828,7 +835,10 @@ 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;
-			payload->cb(payload->pb->nr_written, payload->pb->nr_objects, payload->last_bytes, payload->cb_payload);
+			if (payload->cb(payload->pb->nr_written, payload->pb->nr_objects, payload->last_bytes, payload->cb_payload)) {
+				giterr_clear();
+				error = GIT_EUSER;
+			}
 		}
 	}
 
@@ -840,7 +850,7 @@ int git_smart__push(git_transport *transport, git_push *push)
 	transport_smart *t = (transport_smart *)transport;
 	struct push_packbuilder_payload packbuilder_payload = {0};
 	git_buf pktline = GIT_BUF_INIT;
-	int error = -1, need_pack = 0;
+	int error = 0, need_pack = 0;
 	push_spec *spec;
 	unsigned int i;
 
@@ -882,34 +892,31 @@ int git_smart__push(git_transport *transport, git_push *push)
 		}
 	}
 
-	if (git_smart__get_push_stream(t, &packbuilder_payload.stream) < 0 ||
-		gen_pktline(&pktline, push) < 0 ||
-		packbuilder_payload.stream->write(packbuilder_payload.stream, git_buf_cstr(&pktline), git_buf_len(&pktline)) < 0)
-		goto on_error;
+	if ((error = git_smart__get_push_stream(t, &packbuilder_payload.stream)) < 0 ||
+		(error = gen_pktline(&pktline, push)) < 0 ||
+		(error = packbuilder_payload.stream->write(packbuilder_payload.stream, git_buf_cstr(&pktline), git_buf_len(&pktline))) < 0)
+		goto done;
 
-	if (need_pack && git_packbuilder_foreach(push->pb, &stream_thunk, &packbuilder_payload) < 0)
-		goto on_error;
+	if (need_pack &&
+		(error = git_packbuilder_foreach(push->pb, &stream_thunk, &packbuilder_payload)) < 0)
+		goto done;
 
 	/* If we sent nothing or the server doesn't support report-status, then
 	 * we consider the pack to have been unpacked successfully */
 	if (!push->specs.length || !push->report_status)
 		push->unpack_ok = 1;
-	else if (parse_report(&t->buffer, push) < 0)
-		goto on_error;
+	else if ((error = parse_report(&t->buffer, 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);
 	}
 
-	if (push->status.length &&
-		update_refs_from_report(&t->refs, &push->specs, &push->status) < 0)
-		goto on_error;
+	if (push->status.length)
+		error = update_refs_from_report(&t->refs, &push->specs, &push->status);
 
-	error = 0;
-
-on_error:
+done:
 	git_buf_free(&pktline);
-
 	return error;
 }
diff --git a/tests-clar/online/push.c b/tests-clar/online/push.c
index 05cef56..4c2bec7 100644
--- a/tests-clar/online/push.c
+++ b/tests-clar/online/push.c
@@ -20,7 +20,6 @@ static char *_remote_pass;
 static int cred_acquire_cb(git_cred **,	const char *, const char *, unsigned int, void *);
 
 static git_remote *_remote;
-static bool _cred_acquire_called;
 static record_callbacks_data _record_cbs_data = {{ 0 }};
 static git_remote_callbacks _record_cbs = RECORD_CALLBACKS_INIT(&_record_cbs_data);
 
@@ -47,8 +46,6 @@ static int cred_acquire_cb(
 	GIT_UNUSED(url);
 	GIT_UNUSED(user_from_url);
 
-	*((bool*)payload) = true;
-
 	if (GIT_CREDTYPE_SSH_PUBLICKEY & allowed_types)
 		return git_cred_ssh_keyfile_passphrase_new(cred, _remote_user, _remote_ssh_pubkey, _remote_ssh_key, _remote_ssh_passphrase);
 
@@ -251,7 +248,6 @@ void test_online_push__initialize(void)
 	git_vector delete_specs = GIT_VECTOR_INIT;
 	size_t i;
 	char *curr_del_spec;
-	_cred_acquire_called = false;
 
 	_repo = cl_git_sandbox_init("push_src");
 
@@ -349,16 +345,20 @@ void test_online_push__cleanup(void)
 	cl_git_sandbox_cleanup();
 }
 
-static void push_pack_progress_cb(int stage, unsigned int current, unsigned int total, void* payload)
+static int push_pack_progress_cb(int stage, unsigned int current, unsigned int total, void* payload)
 {
 	int *was_called = (int *) payload;
+	GIT_UNUSED(stage); GIT_UNUSED(current); GIT_UNUSED(total);
 	*was_called = 1;
+	return 0;
 }
 
-static void push_transfer_progress_cb(unsigned int current, unsigned int total, size_t bytes, void* payload)
+static int push_transfer_progress_cb(unsigned int current, unsigned int total, size_t bytes, void* payload)
 {
 	int *was_called = (int *) payload;
+	GIT_UNUSED(current); GIT_UNUSED(total); GIT_UNUSED(bytes); 
 	*was_called = 1;
+	return 0;
 }
 
 /**