submodule: make `_set_url()` affect the configuration

With this one, we can get rid of the edit_and_save test.

4 files changed, 17 insertions, 61 deletions

diff --git a/include/git2/submodule.h b/include/git2/submodule.h
index 03b9efe96..16895ac3b 100644
--- a/include/git2/submodule.h
+++ b/include/git2/submodule.h
@@ -386,20 +386,18 @@ GIT_EXTERN(const char *) git_submodule_branch(git_submodule *submodule);
 GIT_EXTERN(int) git_submodule_set_branch(git_repository *repo, const char *name, const char *branch);
 
 /**
- * Set the URL for the submodule.
+ * Set the URL for the submodule in the configuration
  *
- * This sets the URL in memory for the submodule. This will be used for
- * any following submodule actions while this submodule data is in memory.
  *
- * After calling this, you may wish to call `git_submodule_save()` to write
- * the changes back to the ".gitmodules" file and `git_submodule_sync()` to
+ * After calling this, you may wish to call `git_submodule_sync()` to
  * write the changes to the checked out submodule repository.
  *
- * @param submodule Pointer to the submodule object
+ * @param repo the repository to affect
+ * @param name the name of the submodule to configure
  * @param url URL that should be used for the submodule
  * @return 0 on success, <0 on failure
  */
-GIT_EXTERN(int) git_submodule_set_url(git_submodule *submodule, const char *url);
+GIT_EXTERN(int) git_submodule_set_url(git_repository *repo, const char *name, const char *url);
 
 /**
  * Get the OID for the submodule in the index.
diff --git a/src/submodule.c b/src/submodule.c
index 3ad5d3eb3..24d31fe77 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -788,10 +788,6 @@ int git_submodule_save(git_submodule *submodule)
 		(error = git_config_file_set_string(mods, key.ptr, submodule->path)) < 0)
 		goto cleanup;
 
-	if ((error = submodule_config_key_trunc_puts(&key, "url")) < 0 ||
-		(error = git_config_file_set_string(mods, key.ptr, submodule->url)) < 0)
-		goto cleanup;
-
 	/* update internal defaults */
 
 	submodule->ignore_default = submodule->ignore;
@@ -890,16 +886,11 @@ int git_submodule_set_branch(git_repository *repo, const char *name, const char
 	return write_var(repo, name, "branch", branch);
 }
 
-int git_submodule_set_url(git_submodule *submodule, const char *url)
+int git_submodule_set_url(git_repository *repo, const char *name, const char *url)
 {
-	assert(submodule && url);
-
-	git__free(submodule->url);
+	assert(repo && name && url);
 
-	submodule->url = git__strdup(url);
-	GITERR_CHECK_ALLOC(submodule->url);
-
-	return 0;
+	return write_var(repo, name, "url", url);
 }
 
 const git_oid *git_submodule_index_id(git_submodule *submodule)
diff --git a/tests/submodule/init.c b/tests/submodule/init.c
index dbde0f284..9e0cf5753 100644
--- a/tests/submodule/init.c
+++ b/tests/submodule/init.c
@@ -23,10 +23,10 @@ void test_submodule_init__absolute_url(void)
 	cl_assert(git_path_dirname_r(&absolute_url, git_repository_workdir(g_repo)) > 0);
 	cl_git_pass(git_buf_joinpath(&absolute_url, absolute_url.ptr, "testrepo.git"));
 
-	cl_git_pass(git_submodule_lookup(&sm, g_repo, "testrepo"));
-
 	/* write the absolute url to the .gitmodules file*/
-	cl_git_pass(git_submodule_set_url(sm, absolute_url.ptr));
+	cl_git_pass(git_submodule_set_url(g_repo, "testrepo", absolute_url.ptr));
+
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "testrepo"));
 
 	/* verify that the .gitmodules is set with an absolute path*/
 	cl_assert_equal_s(absolute_url.ptr, git_submodule_url(sm));
diff --git a/tests/submodule/modify.c b/tests/submodule/modify.c
index d0902b3e1..9c387a3b5 100644
--- a/tests/submodule/modify.c
+++ b/tests/submodule/modify.c
@@ -185,47 +185,14 @@ void test_submodule_modify__set_branch(void)
 	git_submodule_free(sm);
 }
 
-void test_submodule_modify__edit_and_save(void)
+void test_submodule_modify__set_url(void)
 {
-	git_submodule *sm1, *sm2;
-	char *old_url;
-	git_repository *r2;
-
-	cl_git_pass(git_submodule_lookup(&sm1, g_repo, "sm_changed_head"));
-
-	old_url = git__strdup(git_submodule_url(sm1));
-
-	/* modify properties of submodule */
-	cl_git_pass(git_submodule_set_url(sm1, SM_LIBGIT2_URL));
-	cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm1));
-
-	/* revert without saving (and confirm setters return old value) */
-	cl_git_pass(git_submodule_set_url(sm1, old_url));
-
-	/* check that revert was successful */
-	cl_assert_equal_s(old_url, git_submodule_url(sm1));
-
-	/* modify properties of submodule (again) */
-	cl_git_pass(git_submodule_set_url(sm1, SM_LIBGIT2_URL));
-
-	/* call save */
-	cl_git_pass(git_submodule_save(sm1));
-
-	/* call reload and check that the new values are loaded */
-	cl_git_pass(git_submodule_reload(sm1, 0));
-
-	cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm1));
-
-	/* open a second copy of the repo and compare submodule */
-	cl_git_pass(git_repository_open(&r2, "submod2"));
-	cl_git_pass(git_submodule_lookup(&sm2, r2, "sm_changed_head"));
-
-	cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm2));
+	git_submodule *sm;
 
-	git_submodule_free(sm1);
-	git_submodule_free(sm2);
-	git_repository_free(r2);
-	git__free(old_url);
+	cl_git_pass(git_submodule_set_url(g_repo, "sm_changed_head", SM_LIBGIT2_URL));
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_head"));
+	cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm));
+	git_submodule_free(sm);
 }
 
 void test_submodule_modify__save_last(void)