Remote: deprecate dangling, prevent saving in-memory

include/git2/remote.h

@@ -24,14 +24,6 @@
  */
 GIT_BEGIN_DECL
 
-/**
- * Use this when creating a remote with git_remote_new to get the default fetch
- * behavior produced by git_remote_add.  It corresponds to this fetchspec (note
- * the spaces between '/' and '*' to avoid C compiler errors):
- * "+refs/heads/ *:refs/remotes/<remote_name>/ *"
- */
-#define GIT_REMOTE_DEFAULT_FETCH ""
-
 typedef int (*git_remote_rename_problem_cb)(const char *problematic_refspec, void *payload);
 /*
  * TODO: This functions still need to be implemented:

src/remote.c

@@ -83,14 +83,14 @@ cleanup:
 	return error;
 }
 
-int git_remote_create_inmemory(git_remote **out, git_repository *repo, const char *name, const char *url, const char *fetch)
+static int create_internal(git_remote **out, git_repository *repo, const char *name, const char *url, const char *fetch)
 {
 	git_remote *remote;
 	git_buf fetchbuf = GIT_BUF_INIT;
 	int error = -1;
 
 	/* name is optional */
-	assert(out && url);
+	assert(out && repo && url);
 
 	remote = git__calloc(1, sizeof(git_remote));
 	GITERR_CHECK_ALLOC(remote);
@@ -113,13 +113,6 @@ int git_remote_create_inmemory(git_remote **out, git_repository *repo, const cha
 
 		remote->name = git__strdup(name);
 		GITERR_CHECK_ALLOC(remote->name);
-
-		/* An empty name indicates to use a sensible default for the fetchspec. */
-		if (fetch && !(*fetch)) {
-			if (git_buf_printf(&fetchbuf, "+refs/heads/*:refs/remotes/%s/*", remote->name) < 0)
-				goto on_error;
-			fetch = git_buf_cstr(&fetchbuf);
-		}
 	}
 
 	if (fetch != NULL) {
@@ -142,6 +135,46 @@ on_error:
 	return error;
 }
 
+int git_remote_create(git_remote **out, git_repository *repo, const char *name, const char *url)
+{
+	git_buf buf = GIT_BUF_INIT;
+	int error;
+
+	if ((error = ensure_remote_name_is_valid(name)) < 0)
+		return error;
+
+	if (git_buf_printf(&buf, "+refs/heads/*:refs/remotes/%s/*", name) < 0)
+		return -1;
+
+	if (create_internal(out, repo, name, url, git_buf_cstr(&buf)) < 0)
+		goto on_error;
+
+	git_buf_free(&buf);
+
+	if (git_remote_save(*out) < 0)
+		goto on_error;
+
+	return 0;
+
+on_error:
+	git_buf_free(&buf);
+	git_remote_free(*out);
+	return -1;
+}
+
+int git_remote_create_inmemory(git_remote **out, git_repository *repo, const char *name, const char *url, const char *fetch)
+{
+	int error;
+	git_remote *remote;
+
+	if ((error = create_internal(&remote, repo, name, url, fetch)) < 0)
+		return error;
+
+	remote->inmem = true;
+	*out = remote;
+	return 0;
+}
+
 int git_remote_set_repository(git_remote *remote, git_repository *repo)
 {
 	assert(repo);
@@ -312,9 +345,9 @@ int git_remote_save(const git_remote *remote)
 
 	assert(remote);
 
-	if (!remote->repo) {
-		giterr_set(GITERR_INVALID, "Can't save a dangling remote.");
-		return GIT_ERROR;
+	if (remote->inmem) {
+		giterr_set(GITERR_INVALID, "Can't save an in-memory remote.");
+		return GIT_EINVALIDSPEC;
 	}
 
 	if ((error = ensure_remote_name_is_valid(remote->name)) < 0)
@@ -461,8 +494,7 @@ int git_remote_set_fetchspec(git_remote *remote, const char *spec)
 		return -1;
 
 	git_refspec__free(&remote->fetch);
-	remote->fetch.src = refspec.src;
-	remote->fetch.dst = refspec.dst;
+	memcpy(&remote->fetch, &refspec, sizeof(git_refspec));
 
 	return 0;
 }
@@ -773,11 +805,6 @@ int git_remote_update_tips(git_remote *remote)
 
 	assert(remote);
 
-	if (!remote->repo) {
-		giterr_set(GITERR_INVALID, "Can't update tips on a dangling remote.");
-		return GIT_ERROR;
-	}
-
 	spec = &remote->fetch;
 	
 	if (git_repository_odb__weakptr(&odb, remote->repo) < 0)
@@ -998,33 +1025,6 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 	return 0;
 }
 
-int git_remote_create(git_remote **out, git_repository *repo, const char *name, const char *url)
-{
-	git_buf buf = GIT_BUF_INIT;
-	int error;
-
-	if ((error = ensure_remote_name_is_valid(name)) < 0)
-		return error;
-
-	if (git_buf_printf(&buf, "+refs/heads/*:refs/remotes/%s/*", name) < 0)
-		return -1;
-
-	if (git_remote_create_inmemory(out, repo, name, url, git_buf_cstr(&buf)) < 0)
-		goto on_error;
-
-	git_buf_free(&buf);
-
-	if (git_remote_save(*out) < 0)
-		goto on_error;
-
-	return 0;
-
-on_error:
-	git_buf_free(&buf);
-	git_remote_free(*out);
-	return -1;
-}
-
 void git_remote_check_cert(git_remote *remote, int check)
 {
 	assert(remote);
@@ -1343,6 +1343,7 @@ int git_remote_rename(
 
 			remote->name = git__strdup(new_name);
 
+			if (remote->inmem) return 0;
 			return git_remote_save(remote);
 		}
 

src/remote.h

@@ -19,6 +19,7 @@ struct git_remote {
 	char *name;
 	char *url;
 	char *pushurl;
+	bool inmem;
 	git_vector refs;
 	struct git_refspec fetch;
 	struct git_refspec push;

tests-clar/network/remoterename.c

@@ -148,23 +148,6 @@ void test_network_remoterename__cannot_overwrite_an_existing_remote(void)
 	cl_assert_equal_i(GIT_EEXISTS, git_remote_rename(_remote, "test_with_pushurl", dont_call_me_cb, NULL));
 }
 
-void test_network_remoterename__renaming_an_inmemory_remote_persists_it(void)
-{
-	git_remote *remote;
-
-	assert_config_entry_existence(_repo, "remote.durable.url", false);
-
-	cl_git_pass(git_remote_create_inmemory(&remote, _repo, NULL, "git://github.com/libgit2/durable.git", NULL));
-
-	assert_config_entry_existence(_repo, "remote.durable.url", false);
-
-	cl_git_pass(git_remote_rename(remote, "durable", dont_call_me_cb, NULL));
-
-	assert_config_entry_value(_repo, "remote.durable.url", "git://github.com/libgit2/durable.git");
-
-	git_remote_free(remote);
-}
-
 void test_network_remoterename__renaming_an_inmemory_nameless_remote_notifies_the_inability_to_update_the_fetch_refspec(void)
 {
 	git_remote *remote;

tests-clar/network/remotes.c

@@ -108,7 +108,7 @@ void test_network_remotes__save(void)
 	_remote = NULL;
 
 	/* Set up the remote and save it to config */
-	cl_git_pass(git_remote_create_inmemory(&_remote, _repo, "upstream", "git://github.com/libgit2/libgit2", NULL));
+	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/*"));
 	cl_git_pass(git_remote_set_pushurl(_remote, "git://github.com/libgit2/libgit2_push"));
@@ -123,7 +123,7 @@ void test_network_remotes__save(void)
 	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(git_refspec_force(_refspec) == 0);
+	cl_assert_equal_i(0, git_refspec_force(_refspec));
 
 	_refspec = git_remote_pushspec(_remote);
 	cl_assert(_refspec != NULL);
@@ -251,13 +251,13 @@ void test_network_remotes__cannot_add_a_nameless_remote(void)
 		git_remote_create(&remote, _repo, NULL, "git://github.com/libgit2/libgit2"));
 }
 
-void test_network_remotes__cannot_save_a_nameless_remote(void)
+void test_network_remotes__cant_save_inmem_remote(void)
 {
 	git_remote *remote;
 
 	cl_git_pass(git_remote_create_inmemory(&remote, _repo, NULL, "git://github.com/libgit2/libgit2", NULL));
 
-	cl_assert_equal_i(GIT_EINVALIDSPEC, git_remote_save(remote));
+	cl_git_fail(git_remote_save(remote));
 	git_remote_free(remote);
 }
 
@@ -344,20 +344,3 @@ void test_network_remotes__check_structure_version(void)
 	err = giterr_last();
 	cl_assert_equal_i(GITERR_INVALID, err->klass);
 }
-
-void test_network_remotes__dangling(void)
-{
-	git_remote_free(_remote);
-	_remote = NULL;
-
-	cl_git_pass(git_remote_create_inmemory(&_remote, NULL, "upstream", "git://github.com/libgit2/libgit2", NULL));
-
-	cl_git_pass(git_remote_rename(_remote, "newname", NULL, NULL));
-	cl_assert_equal_s(git_remote_name(_remote), "newname");
-
-	cl_git_fail(git_remote_save(_remote));
-	cl_git_fail(git_remote_update_tips(_remote));
-
-	cl_git_pass(git_remote_set_repository(_remote, _repo));
-	cl_git_pass(git_remote_save(_remote));
-}