Clone: in-line callbacks for progress

Also implemented in the git2 example.

5 files changed, 99 insertions, 72 deletions

diff --git a/examples/network/clone.c b/examples/network/clone.c
index 99e9ae9a2..39b3241d9 100644
--- a/examples/network/clone.c
+++ b/examples/network/clone.c
@@ -7,61 +7,69 @@
 #include <pthread.h>
 #include <unistd.h>
 
-struct dl_data {
-	git_indexer_stats fetch_stats;
-	git_indexer_stats checkout_stats;
-	git_checkout_opts opts;
-	int ret;
-	int finished;
-	const char *url;
+typedef struct progress_data {
+	git_indexer_stats fetch_progress;
+	float checkout_progress;
 	const char *path;
-};
+} progress_data;
 
-static void *clone_thread(void *ptr)
+static void print_progress(const progress_data *pd)
 {
-	struct dl_data *data = (struct dl_data *)ptr;
-	git_repository *repo = NULL;
-
-	// Kick off the clone
-	data->ret = git_clone(&repo, data->url, data->path,
-								 &data->fetch_stats, &data->opts);
-	if (repo) git_repository_free(repo);
-	data->finished = 1;
+	/*
+	int network_percent = (100*pd->fetch_progress.received) / pd->fetch_progress.total;
+	int index_percent = (100*pd->fetch_progress.processed) / pd->fetch_progress.total;
+	int checkout_percent = (int)(100.f * pd->checkout_progress);
+	printf("net %3d%%  /  idx %3d%%  /  chk %3d%%  %20s\r",
+			network_percent, index_percent, checkout_percent, pd->path);
+			*/
+	printf("net %5d /%5d  –  idx %5d /%5d  –  chk %.04f   %20s\r",
+			pd->fetch_progress.received, pd->fetch_progress.total,
+			pd->fetch_progress.processed, pd->fetch_progress.total,
+			pd->checkout_progress, pd->path);
+}
 
-	pthread_exit(&data->ret);
+static void fetch_progress(const git_indexer_stats *stats, void *payload)
+{
+	progress_data *pd = (progress_data*)payload;
+	pd->fetch_progress = *stats;
+	print_progress(pd);
+}
+static void checkout_progress(const char *path, float progress, void *payload)
+{
+	progress_data *pd = (progress_data*)payload;
+	pd->checkout_progress = progress;
+	pd->path = path;
+	print_progress(pd);
 }
 
 int do_clone(git_repository *repo, int argc, char **argv)
 {
-	struct dl_data data = {0};
-	pthread_t worker;
+	progress_data pd = {0};
+	git_repository *cloned_repo = NULL;
+	git_checkout_opts checkout_opts = {0};
+	const char *url = argv[1];
+	const char *path = argv[2];
+	int error;
 
 	// Validate args
 	if (argc < 3) {
-		printf("USAGE: %s <url> <path>\n", argv[0]);
+		printf ("USAGE: %s <url> <path>\n", argv[0]);
 		return -1;
 	}
 
-	// Data for background thread
-	data.url = argv[1];
-	data.path = argv[2];
-	data.opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING;
-	printf("Cloning '%s' to '%s'\n", data.url, data.path);
+	// Set up options
+	checkout_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING;
+	checkout_opts.progress_cb = checkout_progress;
+	checkout_opts.progress_payload = &pd;
 
-	// Create the worker thread
-	pthread_create(&worker, NULL, clone_thread, &data);
-
-	// Watch for progress information
-	do {
-		usleep(10000);
-		printf("Fetch %d/%d  –  Checkout %d/%d\n",
-				 data.fetch_stats.processed, data.fetch_stats.total,
-				 data.checkout_stats.processed, data.checkout_stats.total);
-	} while (!data.finished);
-	printf("Fetch %d/%d  –  Checkout %d/%d\n",
-			 data.fetch_stats.processed, data.fetch_stats.total,
-			 data.checkout_stats.processed, data.checkout_stats.total);
-
-	return data.ret;
+	// Do the clone
+	error = git_clone(&cloned_repo, url, path, &fetch_progress, &pd, &checkout_opts);
+	printf("\n");
+	if (error != 0) {
+		const git_error *err = giterr_last();
+		if (err) printf("ERROR %d: %s\n", err->klass, err->message);
+		else printf("ERROR %d: no detailed info\n", error);
+	}
+	else if (cloned_repo) git_repository_free(cloned_repo);
+	return error;
 }
-
diff --git a/include/git2/clone.h b/include/git2/clone.h
index 72294c581..dc49074dc 100644
--- a/include/git2/clone.h
+++ b/include/git2/clone.h
@@ -40,7 +40,8 @@ GIT_EXTERN(int) git_clone(
 		git_repository **out,
 		const char *origin_url,
 		const char *workdir_path,
-		git_indexer_stats *fetch_stats,
+		git_indexer_progress_callback fetch_progress_cb,
+		void *fetch_progress_payload,
 		git_checkout_opts *checkout_opts);
 
 /**
@@ -56,7 +57,8 @@ GIT_EXTERN(int) git_clone_bare(
 		git_repository **out,
 		const char *origin_url,
 		const char *dest_path,
-		git_indexer_stats *fetch_stats);
+		git_indexer_progress_callback fetch_progress_cb,
+		void *fetch_progress_payload);
 
 /** @} */
 GIT_END_DECL
diff --git a/src/clone.c b/src/clone.c
index 5f4858d84..61e5e8567 100644
--- a/src/clone.c
+++ b/src/clone.c
@@ -248,7 +248,11 @@ cleanup:
 
 
 
-static int setup_remotes_and_fetch(git_repository *repo, const char *origin_url)
+static int setup_remotes_and_fetch(
+		git_repository *repo,
+		const char *origin_url,
+		git_indexer_progress_callback progress_cb,
+		void *progress_payload)
 {
 	int retcode = GIT_ERROR;
 	git_remote *origin = NULL;
@@ -258,7 +262,7 @@ static int setup_remotes_and_fetch(git_repository *repo, const char *origin_url)
 	if (!git_remote_add(&origin, repo, GIT_REMOTE_ORIGIN, origin_url)) {
 		/* Connect and download everything */
 		if (!git_remote_connect(origin, GIT_DIR_FETCH)) {
-			if (!git_remote_download(origin, &bytes, NULL, NULL)) {
+			if (!git_remote_download(origin, &bytes, progress_cb, 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 */
@@ -306,22 +310,21 @@ static int clone_internal(
 	git_repository **out,
 	const char *origin_url,
 	const char *path,
-	git_indexer_stats *fetch_stats,
+	git_indexer_progress_callback fetch_progress_cb,
+	void *fetch_progress_payload,
 	git_checkout_opts *checkout_opts,
 	bool is_bare)
 {
 	int retcode = GIT_ERROR;
 	git_repository *repo = NULL;
-	git_indexer_stats dummy_stats;
-
-	if (!fetch_stats) fetch_stats = &dummy_stats;
 
 	if (!path_is_okay(path)) {
 		return GIT_ERROR;
 	}
 
 	if (!(retcode = git_repository_init(&repo, path, is_bare))) {
-		if ((retcode = setup_remotes_and_fetch(repo, origin_url)) < 0) {
+		if ((retcode = setup_remotes_and_fetch(repo, origin_url,
+						fetch_progress_cb, fetch_progress_payload)) < 0) {
 			/* Failed to fetch; clean up */
 			git_repository_free(repo);
 			git_futils_rmdir_r(path, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS);
@@ -341,7 +344,8 @@ int git_clone_bare(
 		git_repository **out,
 		const char *origin_url,
 		const char *dest_path,
-		git_indexer_stats *fetch_stats)
+		git_indexer_progress_callback fetch_progress_cb,
+		void *fetch_progress_payload)
 {
 	assert(out && origin_url && dest_path);
 
@@ -349,7 +353,8 @@ int git_clone_bare(
 		out,
 		origin_url,
 		dest_path,
-		fetch_stats,
+		fetch_progress_cb,
+		fetch_progress_payload,
 		NULL,
 		1);
 }
@@ -359,7 +364,8 @@ int git_clone(
 		git_repository **out,
 		const char *origin_url,
 		const char *workdir_path,
-		git_indexer_stats *fetch_stats,
+		git_indexer_progress_callback fetch_progress_cb,
+		void *fetch_progress_payload,
 		git_checkout_opts *checkout_opts)
 {
 	assert(out && origin_url && workdir_path);
@@ -368,7 +374,8 @@ int git_clone(
 		out,
 		origin_url,
 		workdir_path,
-		fetch_stats,
+		fetch_progress_cb,
+		fetch_progress_payload,
 		checkout_opts,
 		0);
 }
diff --git a/tests-clar/clone/network.c b/tests-clar/clone/network.c
index 72be0747c..62d4110c6 100644
--- a/tests-clar/clone/network.c
+++ b/tests-clar/clone/network.c
@@ -29,7 +29,7 @@ void test_clone_network__network_full(void)
 
 	cl_set_cleanup(&cleanup_repository, "./test2");
 
-	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./test2", NULL, NULL));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./test2", NULL, NULL, NULL));
 	cl_assert(!git_repository_is_bare(g_repo));
 	cl_git_pass(git_remote_load(&origin, g_repo, "origin"));
 
@@ -43,7 +43,7 @@ void test_clone_network__network_bare(void)
 
 	cl_set_cleanup(&cleanup_repository, "./test");
 
-	cl_git_pass(git_clone_bare(&g_repo, LIVE_REPO_URL, "./test", NULL));
+	cl_git_pass(git_clone_bare(&g_repo, LIVE_REPO_URL, "./test", NULL, NULL));
 	cl_assert(git_repository_is_bare(g_repo));
 	cl_git_pass(git_remote_load(&origin, g_repo, "origin"));
 
@@ -55,7 +55,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, LIVE_REPO_URL, "./foo", NULL, NULL));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", NULL, NULL, NULL));
 	git_repository_free(g_repo); g_repo = NULL;
 }
 
@@ -65,7 +65,7 @@ void test_clone_network__empty_repository(void)
 
 	cl_set_cleanup(&cleanup_repository, "./empty");
 
-	cl_git_pass(git_clone(&g_repo, LIVE_EMPTYREPO_URL, "./empty", NULL, NULL));
+	cl_git_pass(git_clone(&g_repo, LIVE_EMPTYREPO_URL, "./empty", NULL, NULL, NULL));
 
 	cl_assert_equal_i(true, git_repository_is_empty(g_repo));
 	cl_assert_equal_i(true, git_repository_head_orphan(g_repo));
@@ -83,7 +83,7 @@ void test_clone_network__can_prevent_the_checkout_of_a_standard_repo(void)
 
 	cl_set_cleanup(&cleanup_repository, "./no-checkout");
 
-	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./no-checkout", NULL, NULL));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./no-checkout", NULL, NULL, NULL));
 
 	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)));
@@ -91,27 +91,36 @@ void test_clone_network__can_prevent_the_checkout_of_a_standard_repo(void)
 	git_buf_free(&path);
 }
 
-static void progress(const char *path, float progress, void *payload)
+static void checkout_progress(const char *path, float progress, void *payload)
 {
 	GIT_UNUSED(path); GIT_UNUSED(progress);
 	bool *was_called = (bool*)payload;
 	(*was_called) = true;
 }
 
+static void fetch_progress(const git_indexer_stats *stats, void *payload)
+{
+	GIT_UNUSED(stats);
+	bool *was_called = (bool*)payload;
+	(*was_called) = true;
+}
+
 void test_clone_network__can_checkout_a_cloned_repo(void)
 {
 	git_checkout_opts opts = {0};
 	git_buf path = GIT_BUF_INIT;
 	git_reference *head;
-	bool progress_cb_was_called = false;
+	bool checkout_progress_cb_was_called = false,
+		  fetch_progress_cb_was_called = false;
 
 	opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING;
-	opts.progress_cb = &progress;
-	opts.progress_payload = &progress_cb_was_called;
+	opts.progress_cb = &checkout_progress;
+	opts.progress_payload = &checkout_progress_cb_was_called;
 
 	cl_set_cleanup(&cleanup_repository, "./default-checkout");
 
-	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./default-checkout", NULL, &opts));
+	cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./default-checkout", 
+				&fetch_progress, &fetch_progress_cb_was_called, &opts));
 
 	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)));
@@ -120,7 +129,8 @@ void test_clone_network__can_checkout_a_cloned_repo(void)
 	cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(head));
 	cl_assert_equal_s("refs/heads/master", git_reference_target(head));
 
-	cl_assert_equal_i(true, progress_cb_was_called);
+	cl_assert_equal_i(true, checkout_progress_cb_was_called);
+	cl_assert_equal_i(true, fetch_progress_cb_was_called);
 
 	git_reference_free(head);
 	git_buf_free(&path);
diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c
index b8d0ac11a..3984f3fe7 100644
--- a/tests-clar/clone/nonetwork.c
+++ b/tests-clar/clone/nonetwork.c
@@ -63,9 +63,9 @@ static void build_local_file_url(git_buf *out, const char *fixture)
 void test_clone_nonetwork__bad_url(void)
 {
 	/* Clone should clean up the mess if the URL isn't a git repository */
-	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", NULL, NULL));
+	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", NULL, NULL, NULL));
 	cl_assert(!git_path_exists("./foo"));
-	cl_git_fail(git_clone_bare(&g_repo, "not_a_repo", "./foo.git", NULL));
+	cl_git_fail(git_clone_bare(&g_repo, "not_a_repo", "./foo.git", NULL, NULL));
 	cl_assert(!git_path_exists("./foo.git"));
 }
 
@@ -77,7 +77,7 @@ void test_clone_nonetwork__local(void)
 #if DO_LOCAL_TEST
 	cl_set_cleanup(&cleanup_repository, "./local");
 
-	cl_git_pass(git_clone(&g_repo, git_buf_cstr(&src), "./local", NULL, NULL));
+	cl_git_pass(git_clone(&g_repo, git_buf_cstr(&src), "./local", NULL, NULL, NULL));
 #endif
 
 	git_buf_free(&src);
@@ -91,7 +91,7 @@ void test_clone_nonetwork__local_bare(void)
 #if DO_LOCAL_TEST
 	cl_set_cleanup(&cleanup_repository, "./local.git");
 
-	cl_git_pass(git_clone_bare(&g_repo, git_buf_cstr(&src), "./local.git", NULL));
+	cl_git_pass(git_clone_bare(&g_repo, git_buf_cstr(&src), "./local.git", NULL, NULL));
 #endif
 
 	git_buf_free(&src);
@@ -102,7 +102,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, LIVE_REPO_URL, "./foo", NULL, NULL));
+	cl_git_fail(git_clone(&g_repo, LIVE_REPO_URL, "./foo", NULL, NULL, NULL));
 }
 
 void test_clone_nonetwork__fail_with_already_existing_but_non_empty_directory(void)
@@ -111,5 +111,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, LIVE_REPO_URL, "./foo", NULL, NULL));
+	cl_git_fail(git_clone(&g_repo, LIVE_REPO_URL, "./foo", NULL, NULL, NULL));
 }