Custom transport: minor cleanups

 * Move the transport registration mechanisms into a new header under
   'sys/' because this is advanced stuff.
 * Remove the 'priority' argument from the registration as it adds
   unnecessary complexity.  (Since transports cannot decline to operate,
   only the highest priority transport is ever executed.)  Users who
   require per-priority transports can implement that in their custom
   transport themselves.
 * Simplify registration further by taking a scheme (eg "http") instead
   of a prefix (eg "http://").

CHANGELOG.md

@@ -12,6 +12,12 @@ v0.21 + 1
 
 * LF -> CRLF filter now runs when * text = auto (with Git for Windows 1.9.4)
 
+* The git_transport structure definition has moved into the sys/transport.h
+  file.
+
+* The git_transport_register function no longer takes a priority and takes
+  a URL scheme name (eg "http") instead of a prefix like "http://"
+
 * The git_remote_set_transport function now sets a transport factory function,
   rather than a pre-existing transport instance.
 

include/git2/remote.h

@@ -419,21 +419,6 @@ GIT_EXTERN(int) git_remote_list(git_strarray *out, git_repository *repo);
 GIT_EXTERN(void) git_remote_check_cert(git_remote *remote, int check);
 
 /**
- * Sets a custom transport factory for the remote. The caller can use this
- * function to override the transport used for this remote when performing
- * network operations.
- *
- * @param remote the remote to configure
- * @param transport_cb the function to use to create a transport
- * @param payload opaque parameter passed to transport_cb
- * @return 0 or an error code
- */
-GIT_EXTERN(int) git_remote_set_transport(
-	git_remote *remote,
-	git_transport_cb transport_cb,
-	void *payload);
-
-/**
  * Argument to the completion callback which tells it which operation
  * finished.
  */

include/git2/types.h

@@ -244,6 +244,16 @@ typedef struct git_transfer_progress {
 typedef int (*git_transfer_progress_cb)(const git_transfer_progress *stats, void *payload);
 
 /**
+ * Type for messages delivered by the transport.  Return a negative value
+ * to cancel the network operation.
+ *
+ * @param str The message from the transport
+ * @param len The length of the message
+ * @param payload Payload provided by the caller
+ */
+typedef int (*git_transport_message_cb)(const char *str, int len, void *payload);
+
+/**
  * Opaque structure representing a submodule.
  */
 typedef struct git_submodule git_submodule;

src/remote.h

@@ -9,6 +9,7 @@
 
 #include "git2/remote.h"
 #include "git2/transport.h"
+#include "git2/sys/transport.h"
 
 #include "refspec.h"
 #include "vector.h"

src/transport.c

@@ -9,11 +9,11 @@
 #include "git2/remote.h"
 #include "git2/net.h"
 #include "git2/transport.h"
+#include "git2/sys/transport.h"
 #include "path.h"
 
 typedef struct transport_definition {
 	char *prefix;
-	unsigned priority;
 	git_transport_cb fn;
 	void *param;
 } transport_definition;
@@ -24,51 +24,54 @@ static git_smart_subtransport_definition git_subtransport_definition = { git_sma
 static git_smart_subtransport_definition ssh_subtransport_definition = { git_smart_subtransport_ssh, 0 };
 #endif
 
-static transport_definition local_transport_definition = { "file://", 1, git_transport_local, NULL };
+static transport_definition local_transport_definition = { "file://", git_transport_local, NULL };
 #ifdef GIT_SSH
-static transport_definition ssh_transport_definition = { "ssh://", 1, git_transport_smart, &ssh_subtransport_definition };
+static transport_definition ssh_transport_definition = { "ssh://", git_transport_smart, &ssh_subtransport_definition };
 #else
-static transport_definition dummy_transport_definition = { NULL, 1, git_transport_dummy, NULL };
+static transport_definition dummy_transport_definition = { NULL, git_transport_dummy, NULL };
 #endif
 
 static transport_definition transports[] = {
-	{"git://", 1, git_transport_smart, &git_subtransport_definition},
-	{"http://", 1, git_transport_smart, &http_subtransport_definition},
-	{"https://", 1, git_transport_smart, &http_subtransport_definition},
-	{"file://", 1, git_transport_local, NULL},
+	{ "git://",   git_transport_smart, &git_subtransport_definition },
+	{ "http://",  git_transport_smart, &http_subtransport_definition },
+	{ "https://", git_transport_smart, &http_subtransport_definition },
+	{ "file://",  git_transport_local, NULL },
 #ifdef GIT_SSH
-	{"ssh://", 1, git_transport_smart, &ssh_subtransport_definition},
+	{ "ssh://",   git_transport_smart, &ssh_subtransport_definition },
 #endif
-	{NULL, 0, 0}
+	{ NULL, 0, 0 }
 };
 
-static git_vector additional_transports = GIT_VECTOR_INIT;
+static git_vector custom_transports = GIT_VECTOR_INIT;
 
 #define GIT_TRANSPORT_COUNT (sizeof(transports)/sizeof(transports[0])) - 1
 
-static int transport_find_fn(const char *url, git_transport_cb *callback, void **param)
+static int transport_find_fn(
+	git_transport_cb *out,
+	const char *url,
+	void **param)
 {
 	size_t i = 0;
-	unsigned priority = 0;
-	transport_definition *definition = NULL, *definition_iter;
+	transport_definition *d, *definition = NULL;
 
-	// First, check to see if it's an obvious URL, which a URL scheme
-	for (i = 0; i < GIT_TRANSPORT_COUNT; ++i) {
-		definition_iter = &transports[i];
-
-		if (strncasecmp(url, definition_iter->prefix, strlen(definition_iter->prefix)))
-			continue;
-
-		if (definition_iter->priority > priority)
-			definition = definition_iter;
+	/* Find a user transport who wants to deal with this URI */
+	git_vector_foreach(&custom_transports, i, d) {
+		if (strncasecmp(url, d->prefix, strlen(d->prefix)) == 0) {
+			definition = d;
+			break;
+		}
 	}
 
-	git_vector_foreach(&additional_transports, i, definition_iter) {
-		if (strncasecmp(url, definition_iter->prefix, strlen(definition_iter->prefix)))
-			continue;
+	/* Find a system transport for this URI */
+	if (!definition) {
+		for (i = 0; i < GIT_TRANSPORT_COUNT; ++i) {
+			d = &transports[i];
 
-		if (definition_iter->priority > priority)
-			definition = definition_iter;
+			if (strncasecmp(url, d->prefix, strlen(d->prefix)) == 0) {
+				definition = d;
+				break;
+			}
+		}
 	}
 
 #ifdef GIT_WIN32
@@ -90,7 +93,7 @@ static int transport_find_fn(const char *url, git_transport_cb *callback, void *
 #ifdef GIT_SSH
 		definition = &ssh_transport_definition;
 #else
-        definition = &dummy_transport_definition;
+		definition = &dummy_transport_definition;
 #endif
 
 #ifndef GIT_WIN32
@@ -100,9 +103,9 @@ static int transport_find_fn(const char *url, git_transport_cb *callback, void *
 #endif
 
 	if (!definition)
-		return -1;
+		return GIT_ENOTFOUND;
 
-	*callback = definition->fn;
+	*out = definition->fn;
 	*param = definition->param;
 
 	return 0;
@@ -128,10 +131,11 @@ int git_transport_new(git_transport **out, git_remote *owner, const char *url)
 	void *param;
 	int error;
 
-	if (transport_find_fn(url, &fn, &param) < 0) {
+	if ((error = transport_find_fn(&fn, url, &param)) == GIT_ENOTFOUND) {
 		giterr_set(GITERR_NET, "Unsupported URL protocol");
 		return -1;
-	}
+	} else if (error < 0)
+		return error;
 
 	if ((error = fn(&transport, owner, param)) < 0)
 		return error;
@@ -144,59 +148,79 @@ int git_transport_new(git_transport **out, git_remote *owner, const char *url)
 }
 
 int git_transport_register(
-	const char *prefix,
-	unsigned priority,
+	const char *scheme,
 	git_transport_cb cb,
 	void *param)
 {
-	transport_definition *d;
-
-	d = git__calloc(sizeof(transport_definition), 1);
-	GITERR_CHECK_ALLOC(d);
+	git_buf prefix = GIT_BUF_INIT;
+	transport_definition *d, *definition = NULL;
+	size_t i;
+	int error = 0;
 
-	d->prefix = git__strdup(prefix);
+	assert(scheme);
+	assert(cb);
 
-	if (!d->prefix)
+	if ((error = git_buf_printf(&prefix, "%s://", scheme)) < 0)
 		goto on_error;
 
-	d->priority = priority;
-	d->fn = cb;
-	d->param = param;
+	git_vector_foreach(&custom_transports, i, d) {
+		if (strcasecmp(d->prefix, prefix.ptr) == 0) {
+			error = GIT_EEXISTS;
+			goto on_error;
+		}
+	}
+
+	definition = git__calloc(1, sizeof(transport_definition));
+	GITERR_CHECK_ALLOC(definition);
+
+	definition->prefix = git_buf_detach(&prefix);
+	definition->fn = cb;
+	definition->param = param;
 
-	if (git_vector_insert(&additional_transports, d) < 0)
+	if (git_vector_insert(&custom_transports, definition) < 0)
 		goto on_error;
 
 	return 0;
 
 on_error:
-	git__free(d->prefix);
-	git__free(d);
-	return -1;
+	git_buf_free(&prefix);
+	git__free(definition);
+	return error;
 }
 
-int git_transport_unregister(
-	const char *prefix,
-	unsigned priority)
+int git_transport_unregister(const char *scheme)
 {
+	git_buf prefix = GIT_BUF_INIT;
 	transport_definition *d;
-	unsigned i;
+	size_t i;
+	int error = 0;
+
+	assert(scheme);
 
-	git_vector_foreach(&additional_transports, i, d) {
-		if (d->priority == priority && !strcasecmp(d->prefix, prefix)) {
-			if (git_vector_remove(&additional_transports, i) < 0)
-				return -1;
+	if ((error = git_buf_printf(&prefix, "%s://", scheme)) < 0)
+		goto done;
+
+	git_vector_foreach(&custom_transports, i, d) {
+		if (strcasecmp(d->prefix, prefix.ptr) == 0) {
+			if ((error = git_vector_remove(&custom_transports, i)) < 0)
+				goto done;
 
 			git__free(d->prefix);
 			git__free(d);
 
-			if (!additional_transports.length)
-				git_vector_free(&additional_transports);
+			if (!custom_transports.length)
+				git_vector_free(&custom_transports);
 
-			return 0;
+			error = 0;
+			goto done;
 		}
 	}
 
-	return GIT_ENOTFOUND;
+	error = GIT_ENOTFOUND;
+
+done:
+	git_buf_free(&prefix);
+	return error;
 }
 
 /* from remote.h */
@@ -205,7 +229,7 @@ int git_remote_valid_url(const char *url)
 	git_transport_cb fn;
 	void *param;
 
-	return !transport_find_fn(url, &fn, &param);
+	return !transport_find_fn(&fn, url, &param);
 }
 
 int git_remote_supported_url(const char* url)
@@ -213,7 +237,7 @@ int git_remote_supported_url(const char* url)
 	git_transport_cb fn;
 	void *param;
 
-	if (transport_find_fn(url, &fn, &param) < 0)
+	if (transport_find_fn(&fn, url, &param) < 0)
 		return 0;
 
 	return fn != &git_transport_dummy;

src/transports/git.c

@@ -8,6 +8,7 @@
 #include "git2.h"
 #include "buffer.h"
 #include "netops.h"
+#include "git2/sys/transport.h"
 
 #define OWNING_SUBTRANSPORT(s) ((git_subtransport *)(s)->parent.subtransport)
 

src/transports/smart.h

@@ -9,6 +9,7 @@
 #include "netops.h"
 #include "buffer.h"
 #include "push.h"
+#include "git2/sys/transport.h"
 
 #define GIT_SIDE_BAND_DATA     1
 #define GIT_SIDE_BAND_PROGRESS 2

tests/clone/transport.c

@@ -2,6 +2,7 @@
 
 #include "git2/clone.h"
 #include "git2/transport.h"
+#include "git2/sys/transport.h"
 #include "fileops.h"
 
 static int custom_transport(

tests/structinit/structinit.c

@@ -2,6 +2,7 @@
 #include <git2/sys/config.h>
 #include <git2/sys/odb_backend.h>
 #include <git2/sys/refdb_backend.h>
+#include <git2/sys/transport.h>
 
 #define STRINGIFY(s) #s