Commit dcdb8500e3bd719010a1a65fffc391eef23dd4b4

Vicent Marti 2014-06-30T17:35:42

Merge pull request #2440 from phkelley/transports Improvements to git_transport extensibility

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
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 561df94..3938dcb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,3 +12,16 @@ v0.21 + 1
 
 * LF -> CRLF filter now runs when * text = auto (with Git for Windows 1.9.4)
 
+* The git_remote_set_transport function now sets a transport factory function,
+  rather than a pre-existing transport instance.
+
+* The git_clone_options struct no longer provides the ignore_cert_errors or
+  remote_name members for remote customization.
+
+  Instead, the git_clone_options struct has two new members, remote_cb and
+  remote_cb_payload, which allow the caller to completely override the remote
+  creation process. If needed, the caller can use this callback to give their
+  remote a name other than the default (origin) or disable cert checking.
+
+  The remote_callbacks member has been preserved for convenience, although it
+  is not used when a remote creation callback is supplied.
diff --git a/include/git2/clone.h b/include/git2/clone.h
index 05b7522..c07928a 100644
--- a/include/git2/clone.h
+++ b/include/git2/clone.h
@@ -12,6 +12,7 @@
 #include "indexer.h"
 #include "checkout.h"
 #include "remote.h"
+#include "transport.h"
 
 
 /**
@@ -52,6 +53,27 @@ typedef enum {
 } git_clone_local_t;
 
 /**
+ * The signature of a function matching git_remote_create, with an additional
+ * void* as a callback payload.
+ *
+ * Callers of git_clone may provide a function matching this signature to override
+ * the remote creation and customization process during a clone operation.
+ *
+ * @param out the resulting remote
+ * @param repo the repository in which to create the remote
+ * @param name the remote's name
+ * @param url the remote's url
+ * @param payload an opaque payload
+ * @return 0, GIT_EINVALIDSPEC, GIT_EEXISTS or an error code
+ */
+typedef int (*git_remote_create_cb)(
+	git_remote **out,
+	git_repository *repo,
+	const char *name,
+	const char *url,
+	void *payload);
+
+/**
  * Clone options structure
  *
  * Use the GIT_CLONE_OPTIONS_INIT to get the default settings, like this:
@@ -72,7 +94,11 @@ typedef struct git_clone_options {
 	git_checkout_options checkout_opts;
 
 	/**
-	 * Callbacks to use for reporting fetch progress.
+	 * Callbacks to use for reporting fetch progress, and for acquiring
+	 * credentials in the event they are needed. This parameter is ignored if
+	 * the remote_cb parameter is set; if you provide a remote creation
+	 * callback, then you have the opportunity to configure remote callbacks in
+	 * provided function.
 	 */
 	git_remote_callbacks remote_callbacks;
 
@@ -83,23 +109,11 @@ typedef struct git_clone_options {
 	int bare;
 
 	/**
-	 * Set to 1 if errors validating the remote host's certificate
-	 * should be ignored.
-	 */
-	int ignore_cert_errors;
-
-	/**
 	 * Whether to use a fetch or copy the object database.
 	 */
 	git_clone_local_t local;
 
 	/**
-	 * The name to be given to the remote that will be
-	 * created. The default is "origin".
-	 */
-	const char *remote_name;
-
-	/**
 	 * The name of the branch to checkout. NULL means use the
 	 * remote's default branch.
 	 */
@@ -110,6 +124,20 @@ typedef struct git_clone_options {
 	 * use the default signature using the config.
 	 */
 	git_signature *signature;
+
+	/**
+	 * A callback used to create the git_remote, prior to its being
+	 * used to perform the clone operation. See the documentation for
+	 * git_remote_create_cb for details. This parameter may be NULL,
+	 * indicating that git_clone should provide default behavior.
+	 */
+	git_remote_create_cb remote_cb;
+
+	/**
+	 * An opaque payload to pass to the git_remote creation callback.
+	 * This parameter is ignored unless remote_cb is non-NULL.
+	 */
+	void *remote_cb_payload;
 } git_clone_options;
 
 #define GIT_CLONE_OPTIONS_VERSION 1
diff --git a/include/git2/remote.h b/include/git2/remote.h
index c72c9c8..c8b6ac9 100644
--- a/include/git2/remote.h
+++ b/include/git2/remote.h
@@ -419,20 +419,19 @@ GIT_EXTERN(int) git_remote_list(git_strarray *out, git_repository *repo);
 GIT_EXTERN(void) git_remote_check_cert(git_remote *remote, int check);
 
 /**
- * Sets a custom transport for the remote. The caller can use this function
- * to bypass the automatic discovery of a transport by URL scheme (i.e.
- * http://, https://, git://) and supply their own transport to be used
- * instead. After providing the transport to a remote using this function,
- * the transport object belongs exclusively to that remote, and the remote will
- * free it when it is freed with git_remote_free.
+ * Sets a custom transport factory for the remote. The caller can use this
+ * function to override the transport used for this remote when performing
+ * network operations.
  *
  * @param remote the remote to configure
- * @param transport the transport object for the remote to use
+ * @param transport_cb the function to use to create a transport
+ * @param payload opaque parameter passed to transport_cb
  * @return 0 or an error code
  */
 GIT_EXTERN(int) git_remote_set_transport(
 	git_remote *remote,
-	git_transport *transport);
+	git_transport_cb transport_cb,
+	void *payload);
 
 /**
  * Argument to the completion callback which tells it which operation
diff --git a/src/clone.c b/src/clone.c
index 6c4fb67..a4ed1a2 100644
--- a/src/clone.c
+++ b/src/clone.c
@@ -229,6 +229,22 @@ cleanup:
 	return retcode;
 }
 
+static int default_remote_create(
+		git_remote **out,
+		git_repository *repo,
+		const char *name,
+		const char *url,
+		void *payload)
+{
+	int error;
+	git_remote_callbacks *callbacks = payload;
+
+	if ((error = git_remote_create(out, repo, name, url)) < 0)
+		return error;
+
+	return git_remote_set_callbacks(*out, callbacks);
+}
+
 /*
  * submodules?
  */
@@ -241,8 +257,9 @@ static int create_and_configure_origin(
 {
 	int error;
 	git_remote *origin = NULL;
-	const char *name;
 	char buf[GIT_PATH_MAX];
+	git_remote_create_cb remote_create = options->remote_cb;
+	void *payload = options->remote_cb_payload;
 
 	/* If the path exists and is a dir, the url should be the absolute path */
 	if (git_path_root(url) < 0 && git_path_exists(url) && git_path_isdir(url)) {
@@ -252,14 +269,12 @@ static int create_and_configure_origin(
 		url = buf;
 	}
 
-	name = options->remote_name ? options->remote_name : "origin";
-	if ((error = git_remote_create(&origin, repo, name, url)) < 0)
-		goto on_error;
-
-	if (options->ignore_cert_errors)
-		git_remote_check_cert(origin, 0);
+	if (!remote_create) {
+		remote_create = default_remote_create;
+		payload = (void *)&options->remote_callbacks;
+	}
 
-	if ((error = git_remote_set_callbacks(origin, &options->remote_callbacks)) < 0)
+	if ((error = remote_create(&origin, repo, "origin", url, payload)) < 0)
 		goto on_error;
 
 	if ((error = git_remote_save(origin)) < 0)
diff --git a/src/remote.c b/src/remote.c
index 47b61b1..e9dafa0 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -267,9 +267,11 @@ int git_remote_dup(git_remote **dest, git_remote *source)
 
 	if (source->pushurl != NULL) {
 		remote->pushurl = git__strdup(source->pushurl);
-		GITERR_CHECK_ALLOC(remote->pushurl);		
+		GITERR_CHECK_ALLOC(remote->pushurl);
 	}
 
+	remote->transport_cb = source->transport_cb;
+	remote->transport_cb_payload = source->transport_cb_payload;
 	remote->repo = source->repo;
 	remote->download_tags = source->download_tags;
 	remote->check_cert = source->check_cert;
@@ -659,8 +661,14 @@ int git_remote_connect(git_remote *remote, git_direction direction)
 		return -1;
 	}
 
-	/* A transport could have been supplied in advance with
-	 * git_remote_set_transport */
+	/* If we don't have a transport object yet, and the caller specified a
+	 * custom transport factory, use that */
+	if (!t && remote->transport_cb &&
+		(error = remote->transport_cb(&t, remote, remote->transport_cb_payload)) < 0)
+		return error;
+
+	/* If we still don't have a transport, then use the global
+	 * transport registrations which map URI schemes to transport factories */
 	if (!t && (error = git_transport_new(&t, remote, url)) < 0)
 		return error;
 
@@ -1262,18 +1270,20 @@ const git_remote_callbacks *git_remote_get_callbacks(git_remote *remote)
 	return &remote->callbacks;
 }
 
-int git_remote_set_transport(git_remote *remote, git_transport *transport)
+int git_remote_set_transport(
+	git_remote *remote,
+	git_transport_cb transport_cb,
+	void *payload)
 {
-	assert(remote && transport);
-
-	GITERR_CHECK_VERSION(transport, GIT_TRANSPORT_VERSION, "git_transport");
+	assert(remote);
 
 	if (remote->transport) {
 		giterr_set(GITERR_NET, "A transport is already bound to this remote");
 		return -1;
 	}
 
-	remote->transport = transport;
+	remote->transport_cb = transport_cb;
+	remote->transport_cb_payload = payload;
 	return 0;
 }
 
diff --git a/src/remote.h b/src/remote.h
index 4164a14..47c4f72 100644
--- a/src/remote.h
+++ b/src/remote.h
@@ -22,6 +22,8 @@ struct git_remote {
 	git_vector refs;
 	git_vector refspecs;
 	git_vector active_refspecs;
+	git_transport_cb transport_cb;
+	void *transport_cb_payload;
 	git_transport *transport;
 	git_repository *repo;
 	git_remote_callbacks callbacks;
diff --git a/src/transport.c b/src/transport.c
index 2194b18..fbcda5a 100644
--- a/src/transport.c
+++ b/src/transport.c
@@ -133,10 +133,11 @@ int git_transport_new(git_transport **out, git_remote *owner, const char *url)
 		return -1;
 	}
 
-	error = fn(&transport, owner, param);
-	if (error < 0)
+	if ((error = fn(&transport, owner, param)) < 0)
 		return error;
 
+	GITERR_CHECK_VERSION(transport, GIT_TRANSPORT_VERSION, "git_transport");
+
 	*out = transport;
 
 	return 0;
diff --git a/tests/clone/nonetwork.c b/tests/clone/nonetwork.c
index 4bdc6e1..ab3e8f5 100644
--- a/tests/clone/nonetwork.c
+++ b/tests/clone/nonetwork.c
@@ -110,12 +110,25 @@ void test_clone_nonetwork__fail_with_already_existing_but_non_empty_directory(vo
 	cl_git_fail(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 }
 
+int custom_origin_name_remote_create(
+	git_remote **out,
+	git_repository *repo,
+	const char *name,
+	const char *url,
+	void *payload)
+{
+	GIT_UNUSED(name);
+	GIT_UNUSED(payload);
+
+	return git_remote_create(out, repo, "my_origin", url);
+}
+
 void test_clone_nonetwork__custom_origin_name(void)
 {
-       g_options.remote_name = "my_origin";
-       cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
+	g_options.remote_cb = custom_origin_name_remote_create;
+	cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 
-       cl_git_pass(git_remote_load(&g_remote, g_repo, "my_origin"));
+	cl_git_pass(git_remote_load(&g_remote, g_repo, "my_origin"));
 }
 
 void test_clone_nonetwork__defaults(void)
diff --git a/tests/clone/transport.c b/tests/clone/transport.c
new file mode 100644
index 0000000..27568f2
--- /dev/null
+++ b/tests/clone/transport.c
@@ -0,0 +1,50 @@
+#include "clar_libgit2.h"
+
+#include "git2/clone.h"
+#include "git2/transport.h"
+#include "fileops.h"
+
+static int custom_transport(
+	git_transport **out,
+	git_remote *owner,
+	void *payload)
+{
+	*((int*)payload) = 1;
+
+	return git_transport_local(out, owner, payload);
+}
+
+static int custom_transport_remote_create(
+	git_remote **out,
+	git_repository *repo,
+	const char *name,
+	const char *url,
+	void *payload)
+{
+	int error;
+
+	if ((error = git_remote_create(out, repo, name, url)) < 0)
+		return error;
+
+	if ((error = git_remote_set_transport(*out, custom_transport, payload)) < 0)
+		return error;
+
+	return 0;
+}
+
+void test_clone_transport__custom_transport(void)
+{
+	git_repository *repo;
+	git_clone_options clone_opts = GIT_CLONE_OPTIONS_INIT;
+	int custom_transport_used = 0;
+
+	clone_opts.remote_cb = custom_transport_remote_create;
+	clone_opts.remote_cb_payload = &custom_transport_used;
+
+	cl_git_pass(git_clone(&repo, cl_fixture("testrepo.git"), "./custom_transport.git", &clone_opts));
+	git_repository_free(repo);
+
+	cl_git_pass(git_futils_rmdir_r("./custom_transport.git", NULL, GIT_RMDIR_REMOVE_FILES));
+
+	cl_assert(custom_transport_used == 1);
+}
diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c
index 333b52a..21c5711 100644
--- a/tests/network/remote/remotes.c
+++ b/tests/network/remote/remotes.c
@@ -72,18 +72,17 @@ void test_network_remote_remotes__error_when_not_found(void)
 void test_network_remote_remotes__error_when_no_push_available(void)
 {
 	git_remote *r;
-	git_transport *t;
 	git_push *p;
 
 	cl_git_pass(git_remote_create_anonymous(&r, _repo, cl_fixture("testrepo.git"), NULL));
 
-	cl_git_pass(git_transport_local(&t,r,NULL));
+	cl_git_pass(git_remote_set_transport(r, git_transport_local, NULL));
+
+	cl_git_pass(git_remote_connect(r, GIT_DIRECTION_PUSH));
 
 	/* Make sure that push is really not available */
-	t->push = NULL;
-	cl_git_pass(git_remote_set_transport(r, t));
+	r->transport->push = NULL;
 
-	cl_git_pass(git_remote_connect(r, GIT_DIRECTION_PUSH));
 	cl_git_pass(git_push_new(&p, r));
 	cl_git_pass(git_push_add_refspec(p, "refs/heads/master"));
 	cl_git_fail_with(git_push_finish(p), GIT_ERROR);
@@ -438,27 +437,6 @@ void test_network_remote_remotes__returns_ENOTFOUND_when_neither_url_nor_pushurl
 		git_remote_load(&remote, _repo, "no-remote-url"), GIT_ENOTFOUND);
 }
 
-void test_network_remote_remotes__check_structure_version(void)
-{
-	git_transport transport = GIT_TRANSPORT_INIT;
-	const git_error *err;
-
-	git_remote_free(_remote);
-	_remote = NULL;
-	cl_git_pass(git_remote_create_anonymous(&_remote, _repo, "test-protocol://localhost", NULL));
-
-	transport.version = 0;
-	cl_git_fail(git_remote_set_transport(_remote, &transport));
-	err = giterr_last();
-	cl_assert_equal_i(GITERR_INVALID, err->klass);
-
-	giterr_clear();
-	transport.version = 1024;
-	cl_git_fail(git_remote_set_transport(_remote, &transport));
-	err = giterr_last();
-	cl_assert_equal_i(GITERR_INVALID, err->klass);
-}
-
 void assert_cannot_create_remote(const char *name, int expected_error)
 {
 	git_remote *remote = NULL;