Add more clone options. Push test suite segfaults.

include/git2/clone.h

@@ -11,6 +11,7 @@
 #include "types.h"
 #include "indexer.h"
 #include "checkout.h"
+#include "remote.h"
 
 
 /**
@@ -36,8 +37,26 @@ GIT_BEGIN_DECL
  *   this is called inline with network and indexing operations, so performance
  *   may be affected.
  * - `fetch_progress_payload` is payload for fetch_progress_cb
- * - `checkout_opts` is options for the checkout step. If NULL, no checkout
- *   is performed
+ * - `checkout_opts` is options for the checkout step. If NULL, no checkout is
+ *   performed
+ *
+ *   ** "origin" remote options: **
+ * - `remote_name` is the name given to the "origin" remote.  The default is
+ *   "origin".
+ * - `pushurl` is a URL to be used for pushing.  NULL means use the fetch url.
+ * - `fetch_spec` is the fetch specification to be used for fetching.  NULL
+ *   results in the same behavior as GIT_REMOTE_DEFAULT_FETCH.
+ * - `push_spec` is the fetch specification to be used for pushing.  NULL means
+ *   use the same spec as for fetching.
+ * - `cred_acquire_cb` is a callback to be used if credentials are required
+ *   during the initial fetch.
+ * - `cred_acquire_payload` is the payload for the above callback.
+ * - `transport` is a custom transport to be used for the initial fetch.  NULL
+ *   means use the transport autodetected from the URL.
+ * - `remote_callbacks` may be used to specify custom progress callbacks for
+ *   the origin remote before the fetch is initiated.
+ * - `remote_autotag` may be used to specify the autotag setting before the
+ *   initial fetch.
  */
 
 typedef struct git_clone_options {
@@ -47,6 +66,16 @@ typedef struct git_clone_options {
 	git_transfer_progress_callback fetch_progress_cb;
 	void *fetch_progress_payload;
 	git_checkout_opts *checkout_opts;
+
+	const char *remote_name;
+	const char *pushurl;
+	const char *fetch_spec;
+	const char *push_spec;
+	git_cred_acquire_cb cred_acquire_cb;
+	void *cred_acquire_payload;
+	git_transport *transport;
+	git_remote_callbacks *remote_callbacks;
+	git_remote_autotag_option_t remote_autotag;
 } git_clone_options;
 
 #define GIT_CLONE_OPTIONS_VERSION 1
@@ -66,7 +95,7 @@ typedef struct git_clone_options {
  */
 GIT_EXTERN(int) git_clone(
 		git_repository **out,
-		git_remote *origin,
+		const char *url,
 		const char *local_path,
 		const git_clone_options *options);
 

src/clone.c

@@ -258,27 +258,68 @@ cleanup:
  * submodules?
  */
 
+static int create_and_configure_origin(
+		git_remote **out,
+		git_repository *repo,
+		const char *url,
+		const git_clone_options *options)
+{
+	int error;
+	git_remote *origin;
+
+	if ((error = git_remote_add(&origin, repo, options->remote_name, url)) < 0)
+		goto on_error;
+
+	git_remote_set_cred_acquire_cb(origin, options->cred_acquire_cb,
+			options->cred_acquire_payload);
+	git_remote_set_autotag(origin, options->remote_autotag);
+	/*
+	 * Don't write FETCH_HEAD, we'll check out the remote tracking
+	 * branch ourselves based on the server's default.
+	 */
+	git_remote_set_update_fetchhead(origin, 0);
+
+	if (options->remote_callbacks &&
+	    (error = git_remote_set_callbacks(origin, options->remote_callbacks)) < 0)
+		goto on_error;
+
+	if (options->fetch_spec &&
+	    (error = git_remote_set_fetchspec(origin, options->fetch_spec)) < 0)
+		goto on_error;
+
+	if (options->push_spec &&
+	    (error = git_remote_set_pushspec(origin, options->push_spec)) < 0)
+		goto on_error;
+
+	if (options->pushurl &&
+	    (error = git_remote_set_pushurl(origin, options->pushurl)) < 0)
+		goto on_error;
+
+	*out = origin;
+	return 0;
+
+on_error:
+	if (origin) git_remote_free(origin);
+	return error;
+}
 
 
 static int setup_remotes_and_fetch(
 		git_repository *repo,
-		git_remote *origin,
-		git_transfer_progress_callback progress_cb,
-		void *progress_payload)
+		const char *url,
+		const git_clone_options *options)
 {
 	int retcode = GIT_ERROR;
+	git_remote *origin;
 
-	/* Add the origin remote */
-	if (!git_remote_set_repository(origin, repo) && !git_remote_save(origin)) {
-		/*
-		 * Don't write FETCH_HEAD, we'll check out the remote tracking
-		 * branch ourselves based on the server's default.
-		 */
+	/* Construct an origin remote */
+	if (!create_and_configure_origin(&origin, repo, url, options)) {
 		git_remote_set_update_fetchhead(origin, 0);
 
 		/* Connect and download everything */
 		if (!git_remote_connect(origin, GIT_DIRECTION_FETCH)) {
-			if (!git_remote_download(origin, progress_cb, progress_payload)) {
+			if (!git_remote_download(origin, options->fetch_progress_cb,
+						options->fetch_progress_payload)) {
 				/* Create "origin/foo" branches for all remote branches */
 				if (!git_remote_update_tips(origin)) {
 					/* Point HEAD to the same ref as the remote's head */
@@ -321,59 +362,49 @@ static bool should_checkout(
 	return !git_repository_head_orphan(repo);
 }
 
-static int clone_internal(
+static void normalize_options(git_clone_options *dst, const git_clone_options *src)
+{
+	git_clone_options default_options = GIT_CLONE_OPTIONS_INIT;
+	if (!src) src = &default_options;
+
+	*dst = *src;
+
+	/* Provide defaults for null pointers */
+	if (!dst->remote_name) dst->remote_name = "origin";
+}
+
+int git_clone(
 	git_repository **out,
-	git_remote *origin_remote,
-	const char *path,
-	git_transfer_progress_callback fetch_progress_cb,
-	void *fetch_progress_payload,
-	git_checkout_opts *checkout_opts,
-	bool is_bare)
+	const char *url,
+	const char *local_path,
+	const git_clone_options *options)
 {
 	int retcode = GIT_ERROR;
 	git_repository *repo = NULL;
+	git_clone_options normOptions;
 
-	if (!path_is_okay(path)) {
+	assert(out && url && local_path);
+
+	normalize_options(&normOptions, options);
+	GITERR_CHECK_VERSION(&normOptions, GIT_CLONE_OPTIONS_VERSION, "git_clone_options");
+
+	if (!path_is_okay(local_path)) {
 		return GIT_ERROR;
 	}
 
-	if (!(retcode = git_repository_init(&repo, path, is_bare))) {
-		if ((retcode = setup_remotes_and_fetch(repo, origin_remote,
-						fetch_progress_cb, fetch_progress_payload)) < 0) {
+	if (!(retcode = git_repository_init(&repo, local_path, normOptions.bare))) {
+		if ((retcode = setup_remotes_and_fetch(repo, url, &normOptions)) < 0) {
 			/* Failed to fetch; clean up */
 			git_repository_free(repo);
-			git_futils_rmdir_r(path, NULL, GIT_RMDIR_REMOVE_FILES);
+			git_futils_rmdir_r(local_path, NULL, GIT_RMDIR_REMOVE_FILES);
 		} else {
 			*out = repo;
 			retcode = 0;
 		}
 	}
 
-	if (!retcode && should_checkout(repo, is_bare, checkout_opts))
-		retcode = git_checkout_head(*out, checkout_opts);
+	if (!retcode && should_checkout(repo, normOptions.bare, normOptions.checkout_opts))
+		retcode = git_checkout_head(*out, normOptions.checkout_opts);
 
 	return retcode;
 }
-
-int git_clone(
-	git_repository **out,
-	git_remote *origin,
-	const char *local_path,
-	const git_clone_options *options)
-{
-	git_clone_options dummy_options = GIT_CLONE_OPTIONS_INIT;
-
-	assert(out && origin && local_path);
-	if (!options) options = &dummy_options;
-
-	GITERR_CHECK_VERSION(options, GIT_CLONE_OPTIONS_VERSION, "git_clone_options");
-
-	return clone_internal(
-		out,
-		origin,
-		local_path,
-		options->fetch_progress_cb,
-		options->fetch_progress_payload,
-		options->checkout_opts,
-		options->bare ? 1 : 0);
-}

tests-clar/clone/empty.c

@@ -4,7 +4,6 @@
 #include "repository.h"
 
 static git_clone_options g_options;
-static git_remote *g_origin;
 static git_repository *g_repo;
 static git_repository *g_repo_cloned;
 
@@ -17,13 +16,10 @@ void test_clone_empty__initialize(void)
 
 	memset(&g_options, 0, sizeof(git_clone_options));
 	g_options.version = GIT_CLONE_OPTIONS_VERSION;
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", cl_git_fixture_url("testrepo.git"), GIT_REMOTE_DEFAULT_FETCH));
 }
 
 void test_clone_empty__cleanup(void)
 {
-	git_remote_free(g_origin);
-	g_origin = NULL;
 	cl_git_sandbox_cleanup();
 }
 
@@ -39,23 +35,15 @@ void test_clone_empty__can_clone_an_empty_local_repo_barely(void)
 {
 	cl_set_cleanup(&cleanup_repository, "./empty");
 
-	git_remote_free(g_origin);
-	g_origin = NULL;
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", "./empty_bare.git", GIT_REMOTE_DEFAULT_FETCH));
-
 	g_options.bare = true;
-	cl_git_pass(git_clone(&g_repo_cloned, g_origin, "./empty", &g_options));
+	cl_git_pass(git_clone(&g_repo_cloned, "./empty_bare.git", "./empty", &g_options));
 }
 
 void test_clone_empty__can_clone_an_empty_local_repo(void)
 {
 	cl_set_cleanup(&cleanup_repository, "./empty");
 
-	git_remote_free(g_origin);
-	g_origin = NULL;
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", "./empty_bare.git", GIT_REMOTE_DEFAULT_FETCH));
-
-	cl_git_pass(git_clone(&g_repo_cloned, g_origin, "./empty", &g_options));
+	cl_git_pass(git_clone(&g_repo_cloned, "./empty_bare.git", "./empty", &g_options));
 }
 
 void test_clone_empty__can_clone_an_empty_standard_repo(void)
@@ -64,11 +52,7 @@ void test_clone_empty__can_clone_an_empty_standard_repo(void)
 	g_repo = cl_git_sandbox_init("empty_standard_repo");
 	cl_git_remove_placeholders(git_repository_path(g_repo), "dummy-marker.txt");
 
-	git_remote_free(g_origin);
-	g_origin = NULL;
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", "./empty_standard_repo", GIT_REMOTE_DEFAULT_FETCH));
-
 	cl_set_cleanup(&cleanup_repository, "./empty");
 
-	cl_git_pass(git_clone(&g_repo_cloned, g_origin, "./empty", &g_options));
+	cl_git_pass(git_clone(&g_repo_cloned, "./empty_standard_repo", "./empty", &g_options));
 }

tests-clar/clone/network.c

@@ -9,7 +9,6 @@ CL_IN_CATEGORY("network")
 #define LIVE_EMPTYREPO_URL "http://github.com/libgit2/TestEmptyRepository"
 
 static git_repository *g_repo;
-static git_remote *g_origin;
 static git_clone_options g_options;
 
 void test_clone_network__initialize(void)
@@ -18,12 +17,6 @@ void test_clone_network__initialize(void)
 
 	memset(&g_options, 0, sizeof(git_clone_options));
 	g_options.version = GIT_CLONE_OPTIONS_VERSION;
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", LIVE_REPO_URL, GIT_REMOTE_DEFAULT_FETCH));
-}
-
-void test_clone_network__cleanup(void)
-{
-	git_remote_free(g_origin);
 }
 
 static void cleanup_repository(void *path)
@@ -42,7 +35,7 @@ void test_clone_network__network_full(void)
 
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options));
 	cl_assert(!git_repository_is_bare(g_repo));
 	cl_git_pass(git_remote_load(&origin, g_repo, "origin"));
 
@@ -57,7 +50,7 @@ void test_clone_network__network_bare(void)
 	cl_set_cleanup(&cleanup_repository, "./foo");
 	g_options.bare = true;
 
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options));
 	cl_assert(git_repository_is_bare(g_repo));
 	cl_git_pass(git_remote_load(&origin, g_repo, "origin"));
 
@@ -69,7 +62,7 @@ void test_clone_network__cope_with_already_existing_directory(void)
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
 	p_mkdir("./foo", GIT_DIR_MODE);
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options));
 }
 
 void test_clone_network__empty_repository(void)
@@ -78,10 +71,7 @@ void test_clone_network__empty_repository(void)
 
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
-	git_remote_free(g_origin);
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", LIVE_EMPTYREPO_URL, GIT_REMOTE_DEFAULT_FETCH));
-
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, LIVE_EMPTYREPO_URL, "./foo", &g_options));
 
 	cl_assert_equal_i(true, git_repository_is_empty(g_repo));
 	cl_assert_equal_i(true, git_repository_head_orphan(g_repo));
@@ -98,7 +88,7 @@ void test_clone_network__can_prevent_the_checkout_of_a_standard_repo(void)
 	git_buf path = GIT_BUF_INIT;
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options));
 
 	cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "master.txt"));
 	cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&path)));
@@ -137,7 +127,7 @@ void test_clone_network__can_checkout_a_cloned_repo(void)
 
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options));
 
 	cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "master.txt"));
 	cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&path)));

tests-clar/clone/nonetwork.c

@@ -6,7 +6,6 @@
 #define LIVE_REPO_URL "git://github.com/libgit2/TestGitRepository"
 
 static git_clone_options g_options;
-static git_remote *g_origin;
 static git_repository *g_repo;
 
 void test_clone_nonetwork__initialize(void)
@@ -15,12 +14,6 @@ void test_clone_nonetwork__initialize(void)
 
 	memset(&g_options, 0, sizeof(git_clone_options));
 	g_options.version = GIT_CLONE_OPTIONS_VERSION;
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", cl_git_fixture_url("testrepo.git"), GIT_REMOTE_DEFAULT_FETCH));
-}
-
-void test_clone_nonetwork__cleanup(void)
-{
-	git_remote_free(g_origin);
 }
 
 static void cleanup_repository(void *path)
@@ -36,38 +29,33 @@ static void cleanup_repository(void *path)
 void test_clone_nonetwork__bad_url(void)
 {
 	/* Clone should clean up the mess if the URL isn't a git repository */
-	git_remote_free(g_origin);
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", "not_a_repo", GIT_REMOTE_DEFAULT_FETCH));
-
-	cl_git_fail(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
 	cl_assert(!git_path_exists("./foo"));
 	g_options.bare = true;
-	cl_git_fail(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
 	cl_assert(!git_path_exists("./foo"));
 }
 
 void test_clone_nonetwork__local(void)
 {
 	cl_set_cleanup(&cleanup_repository, "./foo");
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 }
 
 void test_clone_nonetwork__local_absolute_path(void)
 {
-	const char *local_src = cl_fixture("testrepo.git");
-	git_remote_free(g_origin);
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", local_src, GIT_REMOTE_DEFAULT_FETCH));
-
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	const char *local_src = cl_fixture("testrepo.git");
+	cl_git_pass(git_clone(&g_repo, local_src, "./foo", &g_options));
 }
 
 void test_clone_nonetwork__local_bare(void)
 {
 	cl_set_cleanup(&cleanup_repository, "./foo");
+
 	g_options.bare = true;
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 }
 
 void test_clone_nonetwork__fail_when_the_target_is_a_file(void)
@@ -75,7 +63,7 @@ void test_clone_nonetwork__fail_when_the_target_is_a_file(void)
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
 	cl_git_mkfile("./foo", "Bar!");
-	cl_git_fail(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_fail(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 }
 
 void test_clone_nonetwork__fail_with_already_existing_but_non_empty_directory(void)
@@ -84,5 +72,5 @@ void test_clone_nonetwork__fail_with_already_existing_but_non_empty_directory(vo
 
 	p_mkdir("./foo", GIT_DIR_MODE);
 	cl_git_mkfile("./foo/bar", "Baz!");
-	cl_git_fail(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_fail(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 }

tests-clar/fetchhead/network.c

@@ -10,7 +10,6 @@ CL_IN_CATEGORY("network")
 #define LIVE_REPO_URL "git://github.com/libgit2/TestGitRepository"
 
 static git_repository *g_repo;
-static git_remote *g_origin;
 static git_clone_options g_options;
 
 void test_fetchhead_network__initialize(void)
@@ -19,12 +18,6 @@ void test_fetchhead_network__initialize(void)
 
 	memset(&g_options, 0, sizeof(git_clone_options));
 	g_options.version = GIT_CLONE_OPTIONS_VERSION;
-	cl_git_pass(git_remote_new(&g_origin, NULL, "origin", LIVE_REPO_URL, GIT_REMOTE_DEFAULT_FETCH));
-}
-
-void test_fetchhead_network__cleanup(void)
-{
-	git_remote_free(g_origin);
 }
 
 static void cleanup_repository(void *path)
@@ -42,7 +35,7 @@ static void fetchhead_test_clone(void)
 {
 	cl_set_cleanup(&cleanup_repository, "./foo");
 
-	cl_git_pass(git_clone(&g_repo, g_origin, "./foo", &g_options));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options));
 }
 
 static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fetchhead)

tests-clar/network/fetch.c

@@ -87,13 +87,12 @@ void test_network_fetch__doesnt_retrieve_a_pack_when_the_repository_is_up_to_dat
 {
 	git_repository *_repository;
 	bool invoked = false;
-	git_remote *remote, *origin;
+	git_remote *remote;
 	git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
-
 	opts.bare = true;
-	cl_git_pass(git_remote_new(&origin, NULL, "origin", "https://github.com/libgit2/TestGitRepository.git", GIT_REMOTE_DEFAULT_FETCH));
 
-	cl_git_pass(git_clone(&_repository, origin, "./fetch/lg2", &opts));
+	cl_git_pass(git_clone(&_repository, "https://github.com/libgit2/TestGitRepository.git",
+				"./fetch/lg2", &opts));
 	git_repository_free(_repository);
 
 	cl_git_pass(git_repository_open(&_repository, "./fetch/lg2"));
@@ -111,6 +110,5 @@ void test_network_fetch__doesnt_retrieve_a_pack_when_the_repository_is_up_to_dat
 	git_remote_disconnect(remote);
 
 	git_remote_free(remote);
-	git_remote_free(origin);
 	git_repository_free(_repository);
 }

tests-clar/network/push.c

@@ -496,7 +496,7 @@ void test_network_push__bad_refspecs(void)
 	git_push *push;
 
 	if (_remote) {
-		cl_git_pass(git_remote_connect(_remote, GIT_DIRECTION_PUSH));
+//		cl_git_pass(git_remote_connect(_remote, GIT_DIRECTION_PUSH));
 		cl_git_pass(git_push_new(&push, _remote));
 
 		/* Unexpanded branch names not supported */