Remove the callbacks struct from the remote

Having the setting be different from calling its actions was not a great
idea and made for the sake of the wrong convenience.

Instead of that, accept either fetch options, push options or the
callbacks when dealing with the remote. The fetch options are currently
only the callbacks, but more options will be moved from setters and
getters on the remote to the options.

This does mean passing the same struct along the different functions but
the typical use-case will only call git_remote_fetch() or
git_remote_push() and so won't notice much difference.

1 files changed, 71 insertions, 52 deletions

diff --git a/src/remote.c b/src/remote.c
index 91ebdd53c..85da2dc1b 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -691,15 +691,32 @@ const char* git_remote__urlfordirection(git_remote *remote, int direction)
 	return NULL;
 }
 
-int git_remote_connect(git_remote *remote, git_direction direction)
+int set_transport_callbacks(git_transport *t, const git_remote_callbacks *cbs)
+{
+	if (!t->set_callbacks || !cbs)
+		return 0;
+
+	return t->set_callbacks(t, cbs->sideband_progress, NULL,
+				cbs->certificate_check, cbs->payload);
+}
+
+int git_remote_connect(git_remote *remote, git_direction direction, const git_remote_callbacks *callbacks)
 {
 	git_transport *t;
 	const char *url;
 	int flags = GIT_TRANSPORTFLAGS_NONE;
 	int error;
+	void *payload = NULL;
+	git_cred_acquire_cb credentials;
 
 	assert(remote);
 
+	if (callbacks) {
+		GITERR_CHECK_VERSION(callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks");
+		credentials = callbacks->credentials;
+		payload     = callbacks->payload;
+	}
+
 	t = remote->transport;
 
 	url = git_remote__urlfordirection(remote, direction);
@@ -720,11 +737,8 @@ int git_remote_connect(git_remote *remote, git_direction direction)
 	if (!t && (error = git_transport_new(&t, remote, url)) < 0)
 		return error;
 
-	if (t->set_callbacks &&
-	    (error = t->set_callbacks(t, remote->callbacks.sideband_progress, NULL, remote->callbacks.certificate_check, remote->callbacks.payload)) < 0)
-		goto on_error;
-
-	if ((error = t->connect(t, url, remote->callbacks.credentials, remote->callbacks.payload, direction, flags)) != 0)
+	if ((error = set_transport_callbacks(t, callbacks)) < 0 ||
+	    (error = t->connect(t, url, credentials, payload, direction, flags)) != 0)
 		goto on_error;
 
 	remote->transport = t;
@@ -866,14 +880,24 @@ static int ls_to_vector(git_vector *out, git_remote *remote)
 	return 0;
 }
 
-int git_remote_download(git_remote *remote, const git_strarray *refspecs)
+int git_remote_download(git_remote *remote, const git_strarray *refspecs, const git_fetch_options *opts)
 {
 	int error = -1;
 	size_t i;
 	git_vector refs, specs, *to_active;
+	const git_remote_callbacks *cbs = NULL;
 
 	assert(remote);
 
+	if (opts) {
+		GITERR_CHECK_VERSION(&opts->callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks");
+		cbs = &opts->callbacks;
+	}
+
+	if (!git_remote_connected(remote) &&
+	    (error = git_remote_connect(remote, GIT_DIRECTION_FETCH, cbs)) < 0)
+		goto on_error;
+
 	if (ls_to_vector(&refs, remote) < 0)
 		return -1;
 
@@ -915,7 +939,7 @@ int git_remote_download(git_remote *remote, const git_strarray *refspecs)
 	if ((error = git_fetch_negotiate(remote)) < 0)
 		return error;
 
-	return git_fetch_download_pack(remote);
+	return git_fetch_download_pack(remote, cbs);
 
 on_error:
 	git_vector_free(&refs);
@@ -927,16 +951,23 @@ on_error:
 int git_remote_fetch(
 		git_remote *remote,
 		const git_strarray *refspecs,
+		const git_fetch_options *opts,
 		const char *reflog_message)
 {
 	int error;
 	git_buf reflog_msg_buf = GIT_BUF_INIT;
+	const git_remote_callbacks *cbs = NULL;
+
+	if (opts) {
+		GITERR_CHECK_VERSION(&opts->callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks");
+		cbs = &opts->callbacks;
+	}
 
 	/* Connect and download everything */
-	if ((error = git_remote_connect(remote, GIT_DIRECTION_FETCH)) != 0)
+	if ((error = git_remote_connect(remote, GIT_DIRECTION_FETCH, cbs)) != 0)
 		return error;
 
-	error = git_remote_download(remote, refspecs);
+	error = git_remote_download(remote, refspecs, opts);
 
 	/* We don't need to be connected anymore */
 	git_remote_disconnect(remote);
@@ -954,13 +985,13 @@ int git_remote_fetch(
 	}
 
 	/* Create "remote/foo" branches for all remote branches */
-	error = git_remote_update_tips(remote, git_buf_cstr(&reflog_msg_buf));
+	error = git_remote_update_tips(remote, cbs, git_buf_cstr(&reflog_msg_buf));
 	git_buf_free(&reflog_msg_buf);
 	if (error < 0)
 		return error;
 
 	if (remote->prune_refs)
-		error = git_remote_prune(remote);
+		error = git_remote_prune(remote, cbs);
 
 	return error;
 }
@@ -1156,7 +1187,7 @@ static int find_head(const void *_a, const void *_b)
 	return strcmp(a->name, b->name);
 }
 
-int git_remote_prune(git_remote *remote)
+int git_remote_prune(git_remote *remote, const git_remote_callbacks *callbacks)
 {
 	size_t i, j;
 	git_vector remote_refs = GIT_VECTOR_INIT;
@@ -1166,6 +1197,9 @@ int git_remote_prune(git_remote *remote)
 	int error;
 	git_oid zero_id = {{ 0 }};
 
+	if (callbacks)
+		GITERR_CHECK_VERSION(callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks");
+
 	if ((error = ls_to_vector(&remote_refs, remote)) < 0)
 		goto cleanup;
 
@@ -1242,8 +1276,8 @@ int git_remote_prune(git_remote *remote)
 		if (error < 0)
 			goto cleanup;
 
-		if (remote->callbacks.update_tips)
-			error = remote->callbacks.update_tips(refname, &id, &zero_id, remote->callbacks.payload);
+		if (callbacks && callbacks->update_tips)
+			error = callbacks->update_tips(refname, &id, &zero_id, callbacks->payload);
 
 		if (error < 0)
 			goto cleanup;
@@ -1257,6 +1291,7 @@ cleanup:
 
 static int update_tips_for_spec(
 		git_remote *remote,
+		const git_remote_callbacks *callbacks,
 		git_refspec *spec,
 		git_vector *refs,
 		const char *log_message)
@@ -1339,8 +1374,8 @@ static int update_tips_for_spec(
 
 		git_reference_free(ref);
 
-		if (remote->callbacks.update_tips != NULL) {
-			if (remote->callbacks.update_tips(refname.ptr, &old, &head->oid, remote->callbacks.payload) < 0)
+		if (callbacks && callbacks->update_tips != NULL) {
+			if (callbacks->update_tips(refname.ptr, &old, &head->oid, callbacks->payload) < 0)
 				goto on_error;
 		}
 	}
@@ -1455,6 +1490,7 @@ static int opportunistic_updates(const git_remote *remote, git_vector *refs, con
 
 int git_remote_update_tips(
 		git_remote *remote,
+		const git_remote_callbacks *callbacks,
 		const char *reflog_message)
 {
 	git_refspec *spec, tagspec;
@@ -1464,7 +1500,7 @@ int git_remote_update_tips(
 
 	/* push has its own logic hidden away in the push object */
 	if (remote->push) {
-		return git_push_update_tips(remote->push);
+		return git_push_update_tips(remote->push, callbacks);
 	}
 
 	if (git_refspec__parse(&tagspec, GIT_REFSPEC_TAGS, true) < 0)
@@ -1475,7 +1511,7 @@ int git_remote_update_tips(
 		goto out;
 
 	if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
-		if ((error = update_tips_for_spec(remote, &tagspec, &refs, reflog_message)) < 0)
+		if ((error = update_tips_for_spec(remote, callbacks, &tagspec, &refs, reflog_message)) < 0)
 			goto out;
 	}
 
@@ -1483,7 +1519,7 @@ int git_remote_update_tips(
 		if (spec->push)
 			continue;
 
-		if ((error = update_tips_for_spec(remote, spec, &refs, reflog_message)) < 0)
+		if ((error = update_tips_for_spec(remote, callbacks, spec, &refs, reflog_message)) < 0)
 			goto out;
 	}
 
@@ -1600,31 +1636,6 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 	return 0;
 }
 
-int git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *callbacks)
-{
-	assert(remote && callbacks);
-
-	GITERR_CHECK_VERSION(callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks");
-
-	memcpy(&remote->callbacks, callbacks, sizeof(git_remote_callbacks));
-
-	if (remote->transport && remote->transport->set_callbacks)
-		return remote->transport->set_callbacks(remote->transport,
-			remote->callbacks.sideband_progress,
-			NULL,
-			remote->callbacks.certificate_check,
-			remote->callbacks.payload);
-
-	return 0;
-}
-
-const git_remote_callbacks *git_remote_get_callbacks(git_remote *remote)
-{
-	assert(remote);
-
-	return &remote->callbacks;
-}
-
 int git_remote_set_transport(
 	git_remote *remote,
 	git_transport_cb transport_cb,
@@ -2321,12 +2332,15 @@ int git_remote_upload(git_remote *remote, const git_strarray *refspecs, const gi
 	int error;
 	git_push *push;
 	git_refspec *spec;
-	git_remote_callbacks *cbs;
+	const git_remote_callbacks *cbs = NULL;
 
 	assert(remote);
 
+	if (opts)
+		cbs = &opts->callbacks;
+
 	if (!git_remote_connected(remote) &&
-	    (error = git_remote_connect(remote, GIT_DIRECTION_PUSH)) < 0)
+	    (error = git_remote_connect(remote, GIT_DIRECTION_PUSH, cbs)) < 0)
 		goto cleanup;
 
 	free_refspecs(&remote->active_refspecs);
@@ -2360,11 +2374,10 @@ int git_remote_upload(git_remote *remote, const git_strarray *refspecs, const gi
 		}
 	}
 
-	if ((error = git_push_finish(push)) < 0)
+	if ((error = git_push_finish(push, cbs)) < 0)
 		goto cleanup;
 
-	cbs = &remote->callbacks;
-	if (cbs->push_update_reference &&
+	if (cbs && cbs->push_update_reference &&
 	    (error = git_push_status_foreach(push, cbs->push_update_reference, cbs->payload)) < 0)
 		goto cleanup;
 
@@ -2375,16 +2388,22 @@ cleanup:
 int git_remote_push(git_remote *remote, const git_strarray *refspecs, const git_push_options *opts)
 {
 	int error;
+	const git_remote_callbacks *cbs = NULL;
+
+	if (opts) {
+		GITERR_CHECK_VERSION(&opts->callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks");
+		cbs = &opts->callbacks;
+	}
 
 	assert(remote && refspecs);
 
-	if ((error = git_remote_connect(remote, GIT_DIRECTION_PUSH)) < 0)
+	if ((error = git_remote_connect(remote, GIT_DIRECTION_PUSH, cbs)) < 0)
 		return error;
 
 	if ((error = git_remote_upload(remote, refspecs, opts)) < 0)
 		return error;
 
-	error = git_remote_update_tips(remote, NULL);
+	error = git_remote_update_tips(remote, cbs, NULL);
 
 	git_remote_disconnect(remote);
 	return error;