clone: remote git_clone_into{,_local} from the public API

As git_clone now has callbacks to configure the details of the
repository and remote, remove the lower-level functions from the public
API, as they lack some of the logic from git_clone proper.

CHANGELOG.md

@@ -29,3 +29,6 @@ v0.21 + 1
 * The git_clone_options struct now provides repository_cb and
   repository_cb_payload to allow the user to create a repository with
   custom options.
+
+* git_clone_into and git_clone_local_into have been removed from the
+  public API in favour of git_clone callbacks

include/git2/clone.h

@@ -210,59 +210,6 @@ GIT_EXTERN(int) git_clone(
 	const char *local_path,
 	const git_clone_options *options);
 
-/**
- * Clone into a repository
- *
- * After creating the repository and remote and configuring them for
- * paths and callbacks respectively, you can call this function to
- * perform the clone operation and optionally checkout files.
- *
- * @param repo the repository to use
- * @param remote the remote repository to clone from
- * @param co_opts options to use during checkout
- * @param branch the branch to checkout after the clone, pass NULL for the
- *        remote's default branch
- * @param signature The identity used when updating the reflog.
- * @return 0 on success, any non-zero return value from a callback
- *         function, or a negative value to indicate an error (use
- *         `giterr_last` for a detailed error message)
- */
-GIT_EXTERN(int) git_clone_into(
-	git_repository *repo,
-	git_remote *remote,
-	const git_checkout_options *co_opts,
-	const char *branch,
-	const git_signature *signature);
-
-/**
- * Perform a local clone into a repository
- *
- * A "local clone" bypasses any git-aware protocols and simply copies
- * over the object database from the source repository. It is often
- * faster than a git-aware clone, but no verification of the data is
- * performed, and can copy over too much data.
- *
- * @param repo the repository to use
- * @param remote the remote repository to clone from
- * @param co_opts options to use during checkout
- * @param branch the branch to checkout after the clone, pass NULL for the
- *        remote's default branch
- * @param link wether to use hardlinks instead of copying
- * objects. This is only possible if both repositories are on the same
- * filesystem.
- * @param signature the identity used when updating the reflog
- * @return 0 on success, any non-zero return value from a callback
- *         function, or a negative value to indicate an error (use
- *         `giterr_last` for a detailed error message)
- */
-GIT_EXTERN(int) git_clone_local_into(
-	git_repository *repo,
-	git_remote *remote,
-	const git_checkout_options *co_opts,
-	const char *branch,
-	int link,
-	const git_signature *signature);
-
 /** @} */
 GIT_END_DECL
 #endif

src/clone.c

@@ -24,6 +24,8 @@
 #include "repository.h"
 #include "odb.h"
 
+static int clone_local_into(git_repository *repo, git_remote *remote, const git_checkout_options *co_opts, const char *branch, int link, const git_signature *signature);
+
 static int create_branch(
 	git_reference **branch,
 	git_repository *repo,
@@ -329,7 +331,7 @@ static int checkout_branch(git_repository *repo, git_remote *remote, const git_c
 	return error;
 }
 
-int git_clone_into(git_repository *repo, git_remote *_remote, const git_checkout_options *co_opts, const char *branch, const git_signature *signature)
+static int clone_into(git_repository *repo, git_remote *_remote, const git_checkout_options *co_opts, const char *branch, const git_signature *signature)
 {
 	int error;
 	git_buf reflog_message = GIT_BUF_INIT;
@@ -434,11 +436,11 @@ int git_clone(
 	if (!(error = create_and_configure_origin(&origin, repo, url, &options))) {
 		if (git_clone__should_clone_local(url, options.local)) {
 			int link = options.local != GIT_CLONE_LOCAL_NO_LINKS;
-			error = git_clone_local_into(
+			error = clone_local_into(
 				repo, origin, &options.checkout_opts,
 				options.checkout_branch, link, options.signature);
 		} else {
-			error = git_clone_into(
+			error = clone_into(
 				repo, origin, &options.checkout_opts,
 				options.checkout_branch, options.signature);
 		}
@@ -498,7 +500,7 @@ static bool can_link(const char *src, const char *dst, int link)
 #endif
 }
 
-int git_clone_local_into(git_repository *repo, git_remote *remote, const git_checkout_options *co_opts, const char *branch, int link, const git_signature *signature)
+static int clone_local_into(git_repository *repo, git_remote *remote, const git_checkout_options *co_opts, const char *branch, int link, const git_signature *signature)
 {
 	int error, flags;
 	git_repository *src;

tests/clone/local.c

@@ -31,31 +31,24 @@ void test_clone_local__should_clone_local(void)
 void test_clone_local__hardlinks(void)
 {
 	git_repository *repo;
-	git_remote *remote;
-	git_signature *sig;
+	git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
 	git_buf buf = GIT_BUF_INIT;
 	struct stat st;
 
-
 	/*
 	 * In this first clone, we just copy over, since the temp dir
 	 * will often be in a different filesystem, so we cannot
 	 * link. It also allows us to control the number of links
 	 */
-	cl_git_pass(git_repository_init(&repo, "./clone.git", true));
-	cl_git_pass(git_remote_create(&remote, repo, "origin", cl_fixture("testrepo.git")));
-	cl_git_pass(git_signature_now(&sig, "foo", "bar"));
-	cl_git_pass(git_clone_local_into(repo, remote, NULL, NULL, false, sig));
-
-	git_remote_free(remote);
+	opts.bare = true;
+	opts.local = GIT_CLONE_LOCAL_NO_LINKS;
+	cl_git_pass(git_clone(&repo, cl_fixture("testrepo.git"), "./clone.git", &opts));
 	git_repository_free(repo);
 
 	/* This second clone is in the same filesystem, so we can hardlink */
 
-	cl_git_pass(git_repository_init(&repo, "./clone2.git", true));
-	cl_git_pass(git_buf_puts(&buf, cl_git_path_url("clone.git")));
-	cl_git_pass(git_remote_create(&remote, repo, "origin", buf.ptr));
-	cl_git_pass(git_clone_local_into(repo, remote, NULL, NULL, true, sig));
+	opts.local = GIT_CLONE_LOCAL;
+	cl_git_pass(git_clone(&repo, cl_git_path_url("clone.git"), "./clone2.git", &opts));
 
 #ifndef GIT_WIN32
 	git_buf_clear(&buf);
@@ -65,14 +58,11 @@ void test_clone_local__hardlinks(void)
 	cl_assert_equal_i(2, st.st_nlink);
 #endif
 
-	git_remote_free(remote);
 	git_repository_free(repo);
 	git_buf_clear(&buf);
 
-	cl_git_pass(git_repository_init(&repo, "./clone3.git", true));
-	cl_git_pass(git_buf_puts(&buf, cl_git_path_url("clone.git")));
-	cl_git_pass(git_remote_create(&remote, repo, "origin", buf.ptr));
-	cl_git_pass(git_clone_local_into(repo, remote, NULL, NULL, false, sig));
+	opts.local = GIT_CLONE_LOCAL_NO_LINKS;
+	cl_git_pass(git_clone(&repo, cl_git_path_url("clone.git"), "./clone3.git", &opts));
 
 	git_buf_clear(&buf);
 	cl_git_pass(git_buf_join_n(&buf, '/', 4, git_repository_path(repo), "objects", "08", "b041783f40edfe12bb406c9c9a8a040177c125"));
@@ -80,7 +70,6 @@ void test_clone_local__hardlinks(void)
 	cl_git_pass(p_stat(buf.ptr, &st));
 	cl_assert_equal_i(1, st.st_nlink);
 
-	git_remote_free(remote);
 	git_repository_free(repo);
 
 	/* this one should automatically use links */
@@ -95,7 +84,6 @@ void test_clone_local__hardlinks(void)
 #endif
 
 	git_buf_free(&buf);
-	git_signature_free(sig);
 	git_repository_free(repo);
 
 	cl_git_pass(git_futils_rmdir_r("./clone.git", NULL, GIT_RMDIR_REMOVE_FILES));

tests/clone/nonetwork.c

@@ -279,23 +279,6 @@ void test_clone_nonetwork__clone_updates_reflog_properly(void)
 	assert_correct_reflog("refs/heads/master");
 }
 
-void test_clone_nonetwork__clone_into_updates_reflog_properly(void)
-{
-	git_remote *remote;
-	git_signature *sig;
-	cl_git_pass(git_signature_now(&sig, "Me", "foo@example.com"));
-
-	cl_git_pass(git_repository_init(&g_repo, "./foo", false));
-	cl_git_pass(git_remote_create(&remote, g_repo, "origin", cl_git_fixture_url("testrepo.git")));
-	cl_git_pass(git_clone_into(g_repo, remote, NULL, NULL, sig));
-
-	assert_correct_reflog("HEAD");
-	assert_correct_reflog("refs/heads/master");
-
-	git_remote_free(remote);
-	git_signature_free(sig);
-}
-
 static void cleanup_repository(void *path)
 {
 	if (g_repo) {

tests/network/fetchlocal.c

@@ -87,28 +87,43 @@ void test_network_fetchlocal__partial(void)
 	git_remote_free(origin);
 }
 
-void test_network_fetchlocal__clone_into_mirror(void)
+static int remote_mirror_cb(git_remote **out, git_repository *repo,
+			    const char *name, const char *url, void *payload)
 {
-	git_buf path = GIT_BUF_INIT;
-	git_repository *repo;
+	int error;
 	git_remote *remote;
-	git_reference *head;
 
-	cl_git_pass(git_repository_init(&repo, "./foo.git", true));
-	cl_git_pass(git_remote_create(&remote, repo, "origin", cl_git_fixture_url("testrepo.git")));
+	GIT_UNUSED(payload);
+
+	if ((error = git_remote_create(&remote, repo, name, url)) < 0)
+		return error;
 
 	git_remote_clear_refspecs(remote);
-	cl_git_pass(git_remote_add_fetch(remote, "+refs/*:refs/*"));
 
-	cl_git_pass(git_clone_into(repo, remote, NULL, NULL, NULL));
+	if ((error = git_remote_add_fetch(remote, "+refs/*:refs/*")) < 0) {
+		git_remote_free(remote);
+		return error;
+	}
+
+	*out = remote;
+	return 0;
+}
+
+void test_network_fetchlocal__clone_into_mirror(void)
+{
+	git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
+	git_repository *repo;
+	git_reference *head;
+
+	opts.bare = true;
+	opts.remote_cb = remote_mirror_cb;
+	cl_git_pass(git_clone(&repo, cl_git_fixture_url("testrepo.git"), "./foo.git", &opts));
 
 	cl_git_pass(git_reference_lookup(&head, repo, "HEAD"));
 	cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(head));
 	cl_assert_equal_s("refs/heads/master", git_reference_symbolic_target(head));
 
-	git_remote_free(remote);
 	git_reference_free(head);
 	git_repository_free(repo);
-	git_buf_free(&path);
 	cl_fixture_cleanup("./foo.git");
 }

tests/online/clone.c

@@ -124,65 +124,49 @@ void test_online_clone__can_checkout_a_cloned_repo(void)
 	git_buf_free(&path);
 }
 
-void test_online_clone__clone_into(void)
+static int remote_mirror_cb(git_remote **out, git_repository *repo,
+			    const char *name, const char *url, void *payload)
 {
-	git_buf path = GIT_BUF_INIT;
+	int error;
 	git_remote *remote;
-	git_reference *head;
-	git_checkout_options checkout_opts = GIT_CHECKOUT_OPTIONS_INIT;
-	git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
-
-	bool checkout_progress_cb_was_called = false,
-		  fetch_progress_cb_was_called = false;
-
-	checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE;
-	checkout_opts.progress_cb = &checkout_progress;
-	checkout_opts.progress_payload = &checkout_progress_cb_was_called;
+	git_remote_callbacks *callbacks = (git_remote_callbacks *) payload;
 
-	cl_git_pass(git_repository_init(&g_repo, "./foo", false));
-	cl_git_pass(git_remote_create(&remote, g_repo, "origin", LIVE_REPO_URL));
 
-	callbacks.transfer_progress = &fetch_progress;
-	callbacks.payload = &fetch_progress_cb_was_called;
-	git_remote_set_callbacks(remote, &callbacks);
+	if ((error = git_remote_create(&remote, repo, name, url)) < 0)
+		return error;
 
-	cl_git_pass(git_clone_into(g_repo, remote, &checkout_opts, NULL, NULL));
-
-	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)));
+	if ((error = git_remote_set_callbacks(remote, callbacks)) < 0) {
+		git_remote_free(remote);
+		return error;
+	}
 
-	cl_git_pass(git_reference_lookup(&head, g_repo, "HEAD"));
-	cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(head));
-	cl_assert_equal_s("refs/heads/master", git_reference_symbolic_target(head));
+	git_remote_clear_refspecs(remote);
 
-	cl_assert_equal_i(true, checkout_progress_cb_was_called);
-	cl_assert_equal_i(true, fetch_progress_cb_was_called);
+	if ((error = git_remote_add_fetch(remote, "+refs/*:refs/*")) < 0) {
+		git_remote_free(remote);
+		return error;
+	}
 
-	git_remote_free(remote);
-	git_reference_free(head);
-	git_buf_free(&path);
+	*out = remote;
+	return 0;
 }
 
 void test_online_clone__clone_mirror(void)
 {
-	git_buf path = GIT_BUF_INIT;
-	git_remote *remote;
+	git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
 	git_reference *head;
 	git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
 
 	bool fetch_progress_cb_was_called = false;
 
-	cl_git_pass(git_repository_init(&g_repo, "./foo.git", true));
-	cl_git_pass(git_remote_create(&remote, g_repo, "origin", LIVE_REPO_URL));
-
 	callbacks.transfer_progress = &fetch_progress;
 	callbacks.payload = &fetch_progress_cb_was_called;
-	git_remote_set_callbacks(remote, &callbacks);
 
-	git_remote_clear_refspecs(remote);
-	cl_git_pass(git_remote_add_fetch(remote, "+refs/*:refs/*"));
+	opts.bare = true;
+	opts.remote_cb = remote_mirror_cb;
+	opts.remote_cb_payload = &callbacks;
 
-	cl_git_pass(git_clone_into(g_repo, remote, NULL, NULL, NULL));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo.git", &opts));
 
 	cl_git_pass(git_reference_lookup(&head, g_repo, "HEAD"));
 	cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(head));
@@ -190,9 +174,7 @@ void test_online_clone__clone_mirror(void)
 
 	cl_assert_equal_i(true, fetch_progress_cb_was_called);
 
-	git_remote_free(remote);
 	git_reference_free(head);
-	git_buf_free(&path);
 	git_repository_free(g_repo);
 	g_repo = NULL;