diff --git a/CHANGELOG.md b/CHANGELOG.md index 561df946a..3938dcbcf 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 05b7522ce..c07928add 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" /** @@ -51,6 +52,27 @@ typedef enum { GIT_CLONE_LOCAL_NO_LINKS, } 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 * @@ -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; @@ -82,23 +108,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 c72c9c8cc..c8b6ac97a 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 6c4fb6727..a4ed1a29c 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 (!remote_create) { + remote_create = default_remote_create; + payload = (void *)&options->remote_callbacks; + } - if (options->ignore_cert_errors) - git_remote_check_cert(origin, 0); - - 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 47b61b1b1..e9dafa0ea 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 4164a14b3..47c4f7221 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 2194b1864..fbcda5a53 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 4bdc6e13b..ab3e8f50d 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 000000000..27568f228 --- /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 333b52a5b..21c57119a 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)); - - /* Make sure that push is really not available */ - t->push = NULL; - cl_git_pass(git_remote_set_transport(r, t)); + 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 */ + r->transport->push = NULL; + 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;