Merge pull request #1493 from carlosmn/remotes

Revamp the refspec handling

include/git2/refspec.h

@@ -36,6 +36,14 @@ GIT_EXTERN(const char *) git_refspec_src(const git_refspec *refspec);
 GIT_EXTERN(const char *) git_refspec_dst(const git_refspec *refspec);
 
 /**
+ * Get the refspec's string
+ *
+ * @param refspec the refspec
+ * @returns the refspec's original string
+ */
+GIT_EXTERN(const char *) git_refspec_string(const git_refspec *refspec);
+
+/**
  * Get the force update setting
  *
  * @param refspec the refspec

include/git2/remote.h

@@ -142,30 +142,44 @@ GIT_EXTERN(int) git_remote_set_url(git_remote *remote, const char* url);
 GIT_EXTERN(int) git_remote_set_pushurl(git_remote *remote, const char* url);
 
 /**
- * Set the remote's fetch refspec
+ * Add a fetch refspec to the remote
  *
  * @param remote the remote
- * @apram spec the new fetch refspec
+ * @apram refspec the new fetch refspec
  * @return 0 or an error value
  */
-GIT_EXTERN(int) git_remote_set_fetchspec(git_remote *remote, const char *spec);
+GIT_EXTERN(int) git_remote_add_fetch(git_remote *remote, const char *refspec);
 
 /**
- * Get the fetch refspec
+ * Get the remote's list of fetch refspecs
  *
- * @param remote the remote
- * @return a pointer to the fetch refspec or NULL if it doesn't exist
+ * The memory is owned by the user and should be freed with
+ * `git_strarray_free`.
+ *
+ * @param array pointer to the array in which to store the strings
+ * @param remote the remote to query
  */
-GIT_EXTERN(const git_refspec *) git_remote_fetchspec(const git_remote *remote);
+GIT_EXTERN(int) git_remote_get_fetch_refspecs(git_strarray *array, git_remote *remote);
 
 /**
- * Set the remote's push refspec
+ * Add a push refspec to the remote
  *
  * @param remote the remote
- * @param spec the new push refspec
+ * @param refspec the new push refspec
  * @return 0 or an error value
  */
-GIT_EXTERN(int) git_remote_set_pushspec(git_remote *remote, const char *spec);
+GIT_EXTERN(int) git_remote_add_push(git_remote *remote, const char *refspec);
+
+/**
+ * Get the remote's list of push refspecs
+ *
+ * The memory is owned by the user and should be freed with
+ * `git_strarray_free`.
+ *
+ * @param array pointer to the array in which to store the strings
+ * @param remote the remote to query
+ */
+GIT_EXTERN(int) git_remote_get_push_refspecs(git_strarray *array, git_remote *remote);
 
 /**
  * Get the push refspec
@@ -177,6 +191,15 @@ GIT_EXTERN(int) git_remote_set_pushspec(git_remote *remote, const char *spec);
 GIT_EXTERN(const git_refspec *) git_remote_pushspec(const git_remote *remote);
 
 /**
+ * Clear the refspecs
+ *
+ * Remove all configured fetch and push refspecs from the remote.
+ *
+ * @param remote the remote
+ */
+GIT_EXTERN(void) git_remote_clear_refspecs(git_remote *remote);
+
+/**
  * Open a connection to a remote
  *
  * The transport is selected based on the URL. The direction argument

src/branch.c

@@ -11,6 +11,7 @@
 #include "config.h"
 #include "refspec.h"
 #include "refs.h"
+#include "remote.h"
 
 #include "git2/branch.h"
 
@@ -283,12 +284,10 @@ int git_branch_upstream__name(
 		if ((error = git_remote_load(&remote, repo, remote_name)) < 0)
 			goto cleanup;
 
-		refspec = git_remote_fetchspec(remote);
-		if (refspec == NULL
-			|| refspec->src == NULL
-			|| refspec->dst == NULL) {
-				error = GIT_ENOTFOUND;
-				goto cleanup;
+		refspec = git_remote__matching_refspec(remote, merge_name);
+		if (!refspec) {
+			error = GIT_ENOTFOUND;
+			goto cleanup;
 		}
 
 		if (git_refspec_transform_r(&buf, refspec, merge_name) < 0)
@@ -333,11 +332,8 @@ static int remote_name(git_buf *buf, git_repository *repo, const char *canonical
 		if ((error = git_remote_load(&remote, repo, remote_list.strings[i])) < 0)
 			continue;
 
-		fetchspec = git_remote_fetchspec(remote);
-
-		/* Defensivly check that we have a fetchspec */
-		if (fetchspec &&
-			git_refspec_dst_matches(fetchspec, canonical_branch_name)) {
+		fetchspec = git_remote__matching_dst_refspec(remote, canonical_branch_name);
+		if (fetchspec) {
 			/* If we have not already set out yet, then set
 			 * it to the matching remote name. Otherwise
 			 * multiple remotes match this reference, and it
@@ -522,9 +518,9 @@ int git_branch_set_upstream(git_reference *branch, const char *upstream_name)
 		if (git_remote_load(&remote, repo, git_buf_cstr(&value)) < 0)
 			goto on_error;
 
-		fetchspec = git_remote_fetchspec(remote);
+		fetchspec = git_remote__matching_dst_refspec(remote, git_reference_name(upstream));
 		git_buf_clear(&value);
-		if (git_refspec_transform_l(&value, fetchspec, git_reference_name(upstream)) < 0)
+		if (!fetchspec || git_refspec_transform_l(&value, fetchspec, git_reference_name(upstream)) < 0)
 			goto on_error;
 
 		git_remote_free(remote);

src/clone.c

@@ -187,6 +187,7 @@ static int get_head_callback(git_remote_head *head, void *payload)
 static int update_head_to_remote(git_repository *repo, git_remote *remote)
 {
 	int retcode = -1;
+	git_refspec dummy_spec;
 	git_remote_head *remote_head;
 	struct head_info head_info;
 	git_buf remote_master_name = GIT_BUF_INIT;
@@ -211,8 +212,13 @@ static int update_head_to_remote(git_repository *repo, git_remote *remote)
 	git_oid_cpy(&head_info.remote_head_oid, &remote_head->oid);
 	git_buf_init(&head_info.branchname, 16);
 	head_info.repo = repo;
-	head_info.refspec = git_remote_fetchspec(remote);
+	head_info.refspec = git_remote__matching_refspec(remote, GIT_REFS_HEADS_MASTER_FILE);
 	head_info.found = 0;
+
+	if (head_info.refspec == NULL) {
+		memset(&dummy_spec, 0, sizeof(git_refspec));
+		head_info.refspec = &dummy_spec;
+	}
 	
 	/* Determine the remote tracking reference name from the local master */
 	if (git_refspec_transform_r(
@@ -318,11 +324,11 @@ static int create_and_configure_origin(
 		goto on_error;
 
 	if (options->fetch_spec &&
-	    (error = git_remote_set_fetchspec(origin, options->fetch_spec)) < 0)
+	    (error = git_remote_add_fetch(origin, options->fetch_spec)) < 0)
 		goto on_error;
 
 	if (options->push_spec &&
-	    (error = git_remote_set_pushspec(origin, options->push_spec)) < 0)
+	    (error = git_remote_add_push(origin, options->push_spec)) < 0)
 		goto on_error;
 
 	if (options->pushurl &&

src/config_file.c

@@ -482,8 +482,10 @@ static int config_set_multivar(
 
 	pos = git_strmap_lookup_index(b->values, key);
 	if (!git_strmap_valid_index(b->values, pos)) {
+		/* If we don't have it, behave like a normal set */
+		result = config_set(cfg, name, value);
 		git__free(key);
-		return GIT_ENOTFOUND;
+		return result;
 	}
 
 	var = git_strmap_value_at(b->values, pos);

src/fetch.c

@@ -34,7 +34,7 @@ static int filter_ref__cb(git_remote_head *head, void *payload)
 
 	if (!p->found_head && strcmp(head->name, GIT_HEAD_FILE) == 0)
 		p->found_head = 1;
-	else if (git_refspec_src_matches(p->spec, head->name))
+	else if (git_remote__matching_refspec(p->remote, head->name))
 			match = 1;
 	else if (p->remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL &&
 		 git_refspec_src_matches(p->tagspec, head->name))
@@ -68,7 +68,6 @@ static int filter_wants(git_remote *remote)
 	 * not interested in any particular branch but just the remote's
 	 * HEAD, which will be stored in FETCH_HEAD after the fetch.
 	 */
-	p.spec = git_remote_fetchspec(remote);
 	p.tagspec = &tagspec;
 	p.found_head = 0;
 	p.remote = remote;

src/push.c

@@ -177,9 +177,9 @@ int git_push_add_refspec(git_push *push, const char *refspec)
 
 int git_push_update_tips(git_push *push)
 {
-	git_refspec *fetch_spec = &push->remote->fetch;
 	git_buf remote_ref_name = GIT_BUF_INIT;
 	size_t i, j;
+	git_refspec *fetch_spec;
 	push_spec *push_spec;
 	git_reference *remote_ref;
 	push_status *status;
@@ -191,7 +191,8 @@ int git_push_update_tips(git_push *push)
 			continue;
 
 		/* Find the corresponding remote ref */
-		if (!git_refspec_src_matches(fetch_spec, status->ref))
+		fetch_spec = git_remote__matching_refspec(push->remote, status->ref);
+		if (!fetch_spec)
 			continue;
 
 		if ((error = git_refspec_transform_r(&remote_ref_name, fetch_spec, status->ref)) < 0)

src/refspec.c

@@ -25,6 +25,7 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 	assert(refspec && input);
 
 	memset(refspec, 0x0, sizeof(git_refspec));
+	refspec->push = !is_fetch;
 
 	lhs = input;
 	if (*lhs == '+') {
@@ -119,6 +120,9 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 		}
 	}
 
+	refspec->string = git__strdup(input);
+	GITERR_CHECK_ALLOC(refspec->string);
+
 	return 0;
 
  invalid:
@@ -132,6 +136,7 @@ void git_refspec__free(git_refspec *refspec)
 
 	git__free(refspec->src);
 	git__free(refspec->dst);
+	git__free(refspec->string);
 }
 
 const char *git_refspec_src(const git_refspec *refspec)
@@ -144,6 +149,11 @@ const char *git_refspec_dst(const git_refspec *refspec)
 	return refspec == NULL ? NULL : refspec->dst;
 }
 
+const char *git_refspec_string(const git_refspec *refspec)
+{
+	return refspec == NULL ? NULL : refspec->string;
+}
+
 int git_refspec_force(const git_refspec *refspec)
 {
 	assert(refspec);

src/refspec.h

@@ -11,10 +11,11 @@
 #include "buffer.h"
 
 struct git_refspec {
-	struct git_refspec *next;
+	char *string;
 	char *src;
 	char *dst;
 	unsigned int force :1,
+		push : 1,
 		pattern :1,
 		matching :1;
 };

src/remote.h

@@ -20,8 +20,7 @@ struct git_remote {
 	char *url;
 	char *pushurl;
 	git_vector refs;
-	struct git_refspec fetch;
-	struct git_refspec push;
+	git_vector refspecs;
 	git_cred_acquire_cb cred_acquire_cb;
 	void *cred_acquire_payload;
 	git_transport *transport;
@@ -37,4 +36,7 @@ struct git_remote {
 const char* git_remote__urlfordirection(struct git_remote *remote, int direction);
 int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_url);
 
+git_refspec *git_remote__matching_refspec(git_remote *remote, const char *refname);
+git_refspec *git_remote__matching_dst_refspec(git_remote *remote, const char *refname);
+
 #endif

tests-clar/clone/nonetwork.c

@@ -2,6 +2,7 @@
 
 #include "git2/clone.h"
 #include "repository.h"
+#include "remote.h"
 
 #define LIVE_REPO_URL "git://github.com/libgit2/TestGitRepository"
 
@@ -148,7 +149,7 @@ void test_clone_nonetwork__custom_fetch_spec(void)
 	cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 
 	cl_git_pass(git_remote_load(&g_remote, g_repo, "origin"));
-	actual_fs = git_remote_fetchspec(g_remote);
+	actual_fs = git_vector_get(&g_remote->refspecs, 0);
 	cl_assert_equal_s("refs/heads/master", git_refspec_src(actual_fs));
 	cl_assert_equal_s("refs/heads/foo", git_refspec_dst(actual_fs));
 
@@ -164,7 +165,7 @@ void test_clone_nonetwork__custom_push_spec(void)
 	cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));
 
 	cl_git_pass(git_remote_load(&g_remote, g_repo, "origin"));
-	actual_fs = git_remote_pushspec(g_remote);
+	actual_fs = git_vector_get(&g_remote->refspecs, g_remote->refspecs.length - 1);
 	cl_assert_equal_s("refs/heads/master", git_refspec_src(actual_fs));
 	cl_assert_equal_s("refs/heads/foo", git_refspec_dst(actual_fs));
 }

tests-clar/config/multivar.c

@@ -97,6 +97,22 @@ void test_config_multivar__add(void)
 	git_config_free(cfg);
 }
 
+void test_config_multivar__add_new(void)
+{
+	const char *var = "a.brand.new";
+	git_config *cfg;
+	int n;
+
+	cl_git_pass(git_config_open_ondisk(&cfg, "config/config11"));
+
+	cl_git_pass(git_config_set_multivar(cfg, var, "", "variable"));
+	n = 0;
+	cl_git_pass(git_config_get_multivar(cfg, var, NULL, cb, &n));
+	cl_assert(n == 1);
+
+	git_config_free(cfg);
+}
+
 void test_config_multivar__replace(void)
 {
 	git_config *cfg;

tests-clar/network/remote/remotes.c

@@ -13,7 +13,7 @@ void test_network_remote_remotes__initialize(void)
 
 	cl_git_pass(git_remote_load(&_remote, _repo, "test"));
 
-	_refspec = git_remote_fetchspec(_remote);
+	_refspec = git_vector_get(&_remote->refspecs, 0);
 	cl_assert(_refspec != NULL);
 }
 
@@ -109,31 +109,58 @@ void test_network_remote_remotes__refspec_parsing(void)
 	cl_assert_equal_s(git_refspec_dst(_refspec), "refs/remotes/test/*");
 }
 
-void test_network_remote_remotes__set_fetchspec(void)
+void test_network_remote_remotes__add_fetchspec(void)
 {
-	cl_git_pass(git_remote_set_fetchspec(_remote, "refs/*:refs/*"));
-	_refspec = git_remote_fetchspec(_remote);
+	size_t size;
+
+	size = _remote->refspecs.length;
+	cl_assert_equal_i(size, _remote->refspecs.length);
+
+	cl_git_pass(git_remote_add_fetch(_remote, "refs/*:refs/*"));
+
+	size++;
+	cl_assert_equal_i(size, _remote->refspecs.length);
+
+	_refspec = git_vector_get(&_remote->refspecs, size-1);
 	cl_assert_equal_s(git_refspec_src(_refspec), "refs/*");
 	cl_assert_equal_s(git_refspec_dst(_refspec), "refs/*");
+	cl_assert_equal_s(git_refspec_string(_refspec), "refs/*:refs/*");
+	cl_assert_equal_i(_refspec->push, false);
 }
 
-void test_network_remote_remotes__set_pushspec(void)
+void test_network_remote_remotes__add_pushspec(void)
 {
-	cl_git_pass(git_remote_set_pushspec(_remote, "refs/*:refs/*"));
-	_refspec = git_remote_pushspec(_remote);
+	size_t size;
+
+	size = _remote->refspecs.length;
+
+	cl_git_pass(git_remote_add_push(_remote, "refs/*:refs/*"));
+	size++;
+	cl_assert_equal_i(size, _remote->refspecs.length);
+
+	_refspec = git_vector_get(&_remote->refspecs, size-1);
 	cl_assert_equal_s(git_refspec_src(_refspec), "refs/*");
 	cl_assert_equal_s(git_refspec_dst(_refspec), "refs/*");
+	cl_assert_equal_s(git_refspec_string(_refspec), "refs/*:refs/*");
+
+	cl_assert_equal_i(_refspec->push, true);
 }
 
 void test_network_remote_remotes__save(void)
 {
+	git_strarray array;
+	const char *fetch_refspec = "refs/heads/*:refs/remotes/upstream/*";
+	const char *push_refspec = "refs/heads/*:refs/heads/*";
+
 	git_remote_free(_remote);
 	_remote = NULL;
 
 	/* Set up the remote and save it to config */
 	cl_git_pass(git_remote_create(&_remote, _repo, "upstream", "git://github.com/libgit2/libgit2"));
-	cl_git_pass(git_remote_set_fetchspec(_remote, "refs/heads/*:refs/remotes/upstream/*"));
-	cl_git_pass(git_remote_set_pushspec(_remote, "refs/heads/*:refs/heads/*"));
+	git_remote_clear_refspecs(_remote);
+
+	cl_git_pass(git_remote_add_fetch(_remote, fetch_refspec));
+	cl_git_pass(git_remote_add_push(_remote, push_refspec));
 	cl_git_pass(git_remote_set_pushurl(_remote, "git://github.com/libgit2/libgit2_push"));
 	cl_git_pass(git_remote_save(_remote));
 	git_remote_free(_remote);
@@ -142,19 +169,17 @@ void test_network_remote_remotes__save(void)
 	/* Load it from config and make sure everything matches */
 	cl_git_pass(git_remote_load(&_remote, _repo, "upstream"));
 
-	_refspec = git_remote_fetchspec(_remote);
-	cl_assert(_refspec != NULL);
-	cl_assert_equal_s(git_refspec_src(_refspec), "refs/heads/*");
-	cl_assert_equal_s(git_refspec_dst(_refspec), "refs/remotes/upstream/*");
-	cl_assert_equal_i(0, git_refspec_force(_refspec));
-
-	_refspec = git_remote_pushspec(_remote);
-	cl_assert(_refspec != NULL);
-	cl_assert_equal_s(git_refspec_src(_refspec), "refs/heads/*");
-	cl_assert_equal_s(git_refspec_dst(_refspec), "refs/heads/*");
+	cl_git_pass(git_remote_get_fetch_refspecs(&array, _remote));
+	cl_assert_equal_i(1, array.count);
+	cl_assert_equal_s(fetch_refspec, array.strings[0]);
+	git_strarray_free(&array);
 
+	cl_git_pass(git_remote_get_push_refspecs(&array, _remote));
+	cl_assert_equal_i(1, array.count);
+	cl_assert_equal_s(push_refspec, array.strings[0]);
 	cl_assert_equal_s(git_remote_url(_remote), "git://github.com/libgit2/libgit2");
 	cl_assert_equal_s(git_remote_pushurl(_remote), "git://github.com/libgit2/libgit2_push");
+	git_strarray_free(&array);
 
 	/* remove the pushurl again and see if we can save that too */
 	cl_git_pass(git_remote_set_pushurl(_remote, NULL));
@@ -265,7 +290,7 @@ void test_network_remote_remotes__add(void)
 	_remote = NULL;
 
 	cl_git_pass(git_remote_load(&_remote, _repo, "addtest"));
-	_refspec = git_remote_fetchspec(_remote);
+	_refspec = git_vector_get(&_remote->refspecs, 0);
 	cl_assert_equal_s("refs/heads/*", git_refspec_src(_refspec));
 	cl_assert(git_refspec_force(_refspec) == 1);
 	cl_assert_equal_s("refs/remotes/addtest/*", git_refspec_dst(_refspec));
@@ -386,3 +411,43 @@ void test_network_remote_remotes__cannot_create_a_remote_which_name_is_invalid(v
 	assert_cannot_create_remote(".lock", GIT_EINVALIDSPEC);
 	assert_cannot_create_remote("a.lock", GIT_EINVALIDSPEC);
 }
+
+static const char *fetch_refspecs[] = {
+	"+refs/heads/*:refs/remotes/origin/*",
+	"refs/tags/*:refs/tags/*",
+	"+refs/pull/*:refs/pull/*",
+};
+
+static const char *push_refspecs[] = {
+	"refs/heads/*:refs/heads/*",
+	"refs/tags/*:refs/tags/*",
+	"refs/notes/*:refs/notes/*",
+};
+
+void test_network_remote_remotes__query_refspecs(void)
+{
+	git_remote *remote;
+	git_strarray array;
+	int i;
+
+	cl_git_pass(git_remote_create_inmemory(&remote, _repo, NULL, "git://github.com/libgit2/libgit2"));
+
+	for (i = 0; i < 3; i++) {
+		cl_git_pass(git_remote_add_fetch(remote, fetch_refspecs[i]));
+		cl_git_pass(git_remote_add_push(remote, push_refspecs[i]));
+	}
+
+	cl_git_pass(git_remote_get_fetch_refspecs(&array, remote));
+	for (i = 0; i < 3; i++) {
+		cl_assert_equal_s(fetch_refspecs[i], array.strings[i]);
+	}
+	git_strarray_free(&array);
+
+	cl_git_pass(git_remote_get_push_refspecs(&array, remote));
+	for (i = 0; i < 3; i++) {
+		cl_assert_equal_s(push_refspecs[i], array.strings[i]);
+	}
+	git_strarray_free(&array);
+
+	git_remote_free(remote);
+}

tests-clar/online/fetchhead.c

@@ -42,8 +42,10 @@ static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fet
 	cl_git_pass(git_remote_load(&remote, g_repo, "origin"));
 	git_remote_set_autotag(remote, GIT_REMOTE_DOWNLOAD_TAGS_AUTO);
 
-	if(fetchspec != NULL)
-		git_remote_set_fetchspec(remote, fetchspec);
+	if(fetchspec != NULL) {
+		git_remote_clear_refspecs(remote);
+		git_remote_add_fetch(remote, fetchspec);
+	}
 
 	cl_git_pass(git_remote_connect(remote, GIT_DIRECTION_FETCH));
 	cl_git_pass(git_remote_download(remote, NULL, NULL));

tests-clar/online/push.c

@@ -160,7 +160,7 @@ static int tracking_branch_list_cb(const char *branch_name, git_branch_t branch_
  */
 static void verify_tracking_branches(git_remote *remote, expected_ref expected_refs[], size_t expected_refs_len)
 {
-	git_refspec *fetch_spec = &remote->fetch;
+	git_refspec *fetch_spec;
 	size_t i, j;
 	git_buf msg = GIT_BUF_INIT;
 	git_buf ref_name = GIT_BUF_INIT;
@@ -179,7 +179,8 @@ static void verify_tracking_branches(git_remote *remote, expected_ref expected_r
 		/* Convert remote reference name into tracking branch name.
 		 * If the spec is not under refs/heads/, then skip.
 		 */
-		if (!git_refspec_src_matches(fetch_spec, expected_refs[i].name))
+		fetch_spec = git_remote__matching_refspec(remote, expected_refs[i].name);
+		if (!fetch_spec)
 			continue;
 
 		cl_git_pass(git_refspec_transform_r(&ref_name, fetch_spec, expected_refs[i].name));

tests-clar/refs/branches/remote.c

@@ -69,7 +69,8 @@ void test_refs_branches_remote__ambiguous_remote_returns_error(void)
 	cl_git_pass(git_remote_create(&remote, g_repo, "addtest", "http://github.com/libgit2/libgit2"));
 
 	/* Update the remote fetch spec */
-	cl_git_pass(git_remote_set_fetchspec(remote, "refs/heads/*:refs/remotes/test/*"));
+	git_remote_clear_refspecs(remote);
+	cl_git_pass(git_remote_add_fetch(remote, "refs/heads/*:refs/remotes/test/*"));
 	cl_git_pass(git_remote_save(remote));
 
 	git_remote_free(remote);