Merge pull request libgit2#6012 from libgit2/ethomson/custom_url

remote: introduce remote_ready_cb, deprecate resolve_url callback

Author: ethomson

include/git2/remote.h

@@ -212,18 +212,20 @@ GIT_EXTERN(const char *) git_remote_name(const git_remote *remote);
  * Get the remote's url
  *
  * If url.*.insteadOf has been configured for this URL, it will
- * return the modified URL.
+ * return the modified URL.  If `git_remote_set_instance_pushurl`
+ * has been called for this remote, then that URL will be returned.
  *
  * @param remote the remote
  * @return a pointer to the url
  */
 GIT_EXTERN(const char *) git_remote_url(const git_remote *remote);
 
 /**
- * Get the remote's url for pushing
+ * Get the remote's url for pushing.
  *
  * If url.*.pushInsteadOf has been configured for this URL, it
- * will return the modified URL.
+ * will return the modified URL.  If `git_remote_set_instance_pushurl`
+ * has been called for this remote, then that URL will be returned.
  *
  * @param remote the remote
  * @return a pointer to the url or NULL if no special url for pushing is set
@@ -257,6 +259,26 @@ GIT_EXTERN(int) git_remote_set_url(git_repository *repo, const char *remote, con
  */
 GIT_EXTERN(int) git_remote_set_pushurl(git_repository *repo, const char *remote, const char* url);
 
+/**
+ * Set the url for this particular url instance.  The URL in the
+ * configuration will be ignored, and will not be changed.
+ *
+ * @param remote the remote's name
+ * @param url the url to set
+ * @return 0 or an error value
+ */
+GIT_EXTERN(int) git_remote_set_instance_url(git_remote *remote, const char *url);
+
+/**
+ * Set the push url for this particular url instance.  The URL in the
+ * configuration will be ignored, and will not be changed.
+ *
+ * @param remote the remote's name
+ * @param url the url to set
+ * @return 0 or an error value
+ */
+GIT_EXTERN(int) git_remote_set_instance_pushurl(git_remote *remote, const char *url);
+
 /**
  * Add a fetch refspec to the remote's configuration
  *
@@ -477,6 +499,7 @@ typedef int GIT_CALLBACK(git_push_negotiation)(const git_push_update **updates,
  */
 typedef int GIT_CALLBACK(git_push_update_reference_cb)(const char *refname, const char *status, void *data);
 
+#ifndef GIT_DEPRECATE_HARD
 /**
  * Callback to resolve URLs before connecting to remote
  *
@@ -488,8 +511,22 @@ typedef int GIT_CALLBACK(git_push_update_reference_cb)(const char *refname, cons
  * @param direction GIT_DIRECTION_FETCH or GIT_DIRECTION_PUSH
  * @param payload Payload provided by the caller
  * @return 0 on success, GIT_PASSTHROUGH or an error
+ * @deprecated Use `git_remote_set_instance_url`
  */
 typedef int GIT_CALLBACK(git_url_resolve_cb)(git_buf *url_resolved, const char *url, int direction, void *payload);
+#endif
+
+/**
+ * Callback invoked immediately before we attempt to connect to the
+ * given url.  Callers may change the URL before the connection by
+ * calling `git_remote_set_instance_url` in the callback.
+ *
+ * @param remote The remote to be connected
+ * @param direction GIT_DIRECTION_FETCH or GIT_DIRECTION_PUSH
+ * @param payload Payload provided by the caller
+ * @return 0 on success, or an error
+ */
+typedef int GIT_CALLBACK(git_remote_ready_cb)(git_remote *remote, int direction, void *payload);
 
 /**
  * The callback settings structure
@@ -575,17 +612,29 @@ struct git_remote_callbacks {
 	 */
 	git_transport_cb transport;
 
+	/**
+	 * Callback when the remote is ready to connect.
+	 */
+	git_remote_ready_cb remote_ready;
+
 	/**
 	 * This will be passed to each of the callbacks in this struct
 	 * as the last parameter.
 	 */
 	void *payload;
 
+#ifdef GIT_DEPRECATE_HARD
+	void *reserved;
+#else
 	/**
 	 * Resolve URL before connecting to remote.
 	 * The returned URL will be used to connect to the remote instead.
+	 *
+	 * This callback is deprecated; users should use
+	 * git_remote_ready_cb and configure the instance URL instead.
 	 */
 	git_url_resolve_cb resolve_url;
+#endif
 };
 
 #define GIT_REMOTE_CALLBACKS_VERSION 1

src/remote.c

@@ -600,6 +600,22 @@ const char *git_remote_url(const git_remote *remote)
 	return remote->url;
 }
 
+int git_remote_set_instance_url(git_remote *remote, const char *url)
+{
+	char *tmp;
+
+	GIT_ASSERT_ARG(remote);
+	GIT_ASSERT_ARG(url);
+
+	if ((tmp = git__strdup(url)) == NULL)
+		return -1;
+
+	git__free(remote->url);
+	remote->url = tmp;
+
+	return 0;
+}
+
 static int set_url(git_repository *repo, const char *remote, const char *pattern, const char *url)
 {
 	git_config *cfg;
@@ -645,13 +661,37 @@ const char *git_remote_pushurl(const git_remote *remote)
 	return remote->pushurl;
 }
 
+int git_remote_set_instance_pushurl(git_remote *remote, const char *url)
+{
+	char *tmp;
+
+	GIT_ASSERT_ARG(remote);
+	GIT_ASSERT_ARG(url);
+
+	if ((tmp = git__strdup(url)) == NULL)
+		return -1;
+
+	git__free(remote->pushurl);
+	remote->pushurl = tmp;
+
+	return 0;
+}
+
 int git_remote_set_pushurl(git_repository *repo, const char *remote, const char* url)
 {
 	return set_url(repo, remote, CONFIG_PUSHURL_FMT, url);
 }
 
-static int resolve_url(git_buf *resolved_url, const char *url, int direction, const git_remote_callbacks *callbacks)
+static int resolve_url(
+	git_buf *resolved_url,
+	const char *url,
+	int direction,
+	const git_remote_callbacks *callbacks)
 {
+#ifdef GIT_DEPRECATE_HARD
+	GIT_UNUSED(direction);
+	GIT_UNUSED(callbacks);
+#else
 	int status, error;
 
 	if (callbacks && callbacks->resolve_url) {
@@ -666,22 +706,35 @@ static int resolve_url(git_buf *resolved_url, const char *url, int direction, co
 			return status;
 		}
 	}
+#endif
 
 	return git_buf_sets(resolved_url, url);
 }
 
-int git_remote__urlfordirection(git_buf *url_out, struct git_remote *remote, int direction, const git_remote_callbacks *callbacks)
+int git_remote__urlfordirection(
+	git_buf *url_out,
+	struct git_remote *remote,
+	int direction,
+	const git_remote_callbacks *callbacks)
 {
 	const char *url = NULL;
 
 	GIT_ASSERT_ARG(remote);
 	GIT_ASSERT_ARG(direction == GIT_DIRECTION_FETCH || direction == GIT_DIRECTION_PUSH);
 
-	if (direction == GIT_DIRECTION_FETCH) {
+	if (callbacks && callbacks->remote_ready) {
+		int status = callbacks->remote_ready(remote, direction, callbacks->payload);
+
+		if (status != 0 && status != GIT_PASSTHROUGH) {
+			git_error_set_after_callback_function(status, "git_remote_ready_cb");
+			return status;
+		}
+	}
+
+	if (direction == GIT_DIRECTION_FETCH)
 		url = remote->url;
-	} else if (direction == GIT_DIRECTION_PUSH) {
+	else if (direction == GIT_DIRECTION_PUSH)
 		url = remote->pushurl ? remote->pushurl : remote->url;
-	}
 
 	if (!url) {
 		git_error_set(GIT_ERROR_INVALID,
@@ -690,6 +743,7 @@ int git_remote__urlfordirection(git_buf *url_out, struct git_remote *remote, int
 			direction == GIT_DIRECTION_FETCH ? "fetch" : "push");
 		return GIT_EINVALID;
 	}
+
 	return resolve_url(url_out, url, direction, callbacks);
 }
 

tests/network/remote/remotes.c

@@ -56,6 +56,49 @@ void test_network_remote_remotes__parsing(void)
 	git_buf_dispose(&url);
 }
 
+static int remote_ready_callback(git_remote *remote, int direction, void *payload)
+{
+	if (direction == GIT_DIRECTION_PUSH) {
+		const char *url = git_remote_pushurl(remote);
+
+		cl_assert_equal_p(url, NULL);;
+		cl_assert_equal_s(payload, "payload");
+		return git_remote_set_instance_pushurl(remote, "push_url");
+	}
+
+	if (direction == GIT_DIRECTION_FETCH) {
+		const char *url = git_remote_url(remote);
+
+		cl_assert_equal_s(url, "git://github.com/libgit2/libgit2");
+		cl_assert_equal_s(payload, "payload");
+		return git_remote_set_instance_url(remote, "fetch_url");
+	}
+
+	return -1;
+}
+
+void test_network_remote_remotes__remote_ready(void)
+{
+	git_buf url = GIT_BUF_INIT;
+
+	git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
+	callbacks.remote_ready = remote_ready_callback;
+	callbacks.payload = "payload";
+
+	cl_assert_equal_s(git_remote_name(_remote), "test");
+	cl_assert_equal_s(git_remote_url(_remote), "git://github.com/libgit2/libgit2");
+	cl_assert(git_remote_pushurl(_remote) == NULL);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_FETCH, &callbacks));
+	cl_assert_equal_s(url.ptr, "fetch_url");
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, &callbacks));
+	cl_assert_equal_s(url.ptr, "push_url");
+
+	git_buf_dispose(&url);
+}
+
+#ifndef GIT_DEPRECATE_HARD
 static int urlresolve_callback(git_buf *url_resolved, const char *url, int direction, void *payload)
 {
 	cl_assert(strcmp(url, "git://github.com/libgit2/libgit2") == 0);
@@ -69,9 +112,11 @@ static int urlresolve_callback(git_buf *url_resolved, const char *url, int direc
 
 	return GIT_OK;
 }
+#endif
 
 void test_network_remote_remotes__urlresolve(void)
 {
+#ifndef GIT_DEPRECATE_HARD
 	git_buf url = GIT_BUF_INIT;
 
 	git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
@@ -89,8 +134,10 @@ void test_network_remote_remotes__urlresolve(void)
 	cl_assert_equal_s(url.ptr, "pushresolve");
 
 	git_buf_dispose(&url);
+#endif
 }
 
+#ifndef GIT_DEPRECATE_HARD
 static int urlresolve_passthrough_callback(git_buf *url_resolved, const char *url, int direction, void *payload)
 {
 	GIT_UNUSED(url_resolved);
@@ -99,9 +146,11 @@ static int urlresolve_passthrough_callback(git_buf *url_resolved, const char *ur
 	GIT_UNUSED(payload);
 	return GIT_PASSTHROUGH;
 }
+#endif
 
 void test_network_remote_remotes__urlresolve_passthrough(void)
 {
+#ifndef GIT_DEPRECATE_HARD
 	git_buf url = GIT_BUF_INIT;
 	const char *orig_url = "git://github.com/libgit2/libgit2";
 
@@ -118,6 +167,52 @@ void test_network_remote_remotes__urlresolve_passthrough(void)
 	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, &callbacks));
 	cl_assert_equal_s(url.ptr, orig_url);
 
+	git_buf_dispose(&url);
+#endif
+}
+
+void test_network_remote_remotes__instance_url(void)
+{
+	git_buf url = GIT_BUF_INIT;
+	const char *orig_url = "git://github.com/libgit2/libgit2";
+
+	cl_assert_equal_s(git_remote_name(_remote), "test");
+	cl_assert_equal_s(git_remote_url(_remote), orig_url);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_FETCH, NULL));
+	cl_assert_equal_s(url.ptr, orig_url);
+	git_buf_clear(&url);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, NULL));
+	cl_assert_equal_s(url.ptr, orig_url);
+	git_buf_clear(&url);
+
+	/* Setting the instance url updates the fetch and push URLs */
+	git_remote_set_instance_url(_remote, "https://github.com/new/remote/url");
+	cl_assert_equal_s(git_remote_url(_remote), "https://github.com/new/remote/url");
+	cl_assert_equal_p(git_remote_pushurl(_remote), NULL);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_FETCH, NULL));
+	cl_assert_equal_s(url.ptr, "https://github.com/new/remote/url");
+	git_buf_clear(&url);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, NULL));
+	cl_assert_equal_s(url.ptr, "https://github.com/new/remote/url");
+	git_buf_clear(&url);
+
+	/* Setting the instance push url updates only the push URL */
+	git_remote_set_instance_pushurl(_remote, "https://github.com/new/push/url");
+	cl_assert_equal_s(git_remote_url(_remote), "https://github.com/new/remote/url");
+	cl_assert_equal_s(git_remote_pushurl(_remote), "https://github.com/new/push/url");
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_FETCH, NULL));
+	cl_assert_equal_s(url.ptr, "https://github.com/new/remote/url");
+	git_buf_clear(&url);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, NULL));
+	cl_assert_equal_s(url.ptr, "https://github.com/new/push/url");
+	git_buf_clear(&url);
+
 	git_buf_dispose(&url);
 }
 

tests/online/push_util.h

@@ -12,7 +12,7 @@ extern const git_oid OID_ZERO;
  * @param data pointer to a record_callbacks_data instance
  */
 #define RECORD_CALLBACKS_INIT(data) \
-	{ GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, cred_acquire_cb, NULL, NULL, record_update_tips_cb, NULL, NULL, NULL, NULL, NULL, data, NULL }
+	{ GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, cred_acquire_cb, NULL, NULL, record_update_tips_cb, NULL, NULL, NULL, NULL, NULL, NULL, data, NULL }
 
 typedef struct {
 	char *name;