submodule: make _set_update() affect the configuration

Moving on with the removal of runtime-changing variables, the update
setting for a remote is whatever it was when it was looked up.

include/git2/submodule.h

@@ -490,23 +490,18 @@ GIT_EXTERN(git_submodule_update_t) git_submodule_update_strategy(
 	git_submodule *submodule);
 
 /**
- * Set the update rule for the submodule.
+ * Set the update rule for the submodule in the configuration
  *
- * The initial value comes from the ".git/config" setting of
+ * This setting won't affect any existing instances.
- * `submodule.$name.update` for this submodule (which is initialized from
- * the ".gitmodules" file).  Using this function sets the update rule in
- * memory for the submodule.  Call `git_submodule_save()` to write out the
- * new update rule.
  *
- * Calling this again with GIT_SUBMODULE_UPDATE_RESET or calling
+ * @param repo the repository to affect
- * `git_submodule_reload()` will revert the rule to the on disk value.
+ * @param name the name of the submodule to configure
- *
- * @param submodule The submodule to update
  * @param update The new value to use
- * @return old value for update
+ * @return 0 or an error code
  */
-GIT_EXTERN(git_submodule_update_t) git_submodule_set_update(
+GIT_EXTERN(int) git_submodule_set_update(
-	git_submodule *submodule,
+	git_repository *repo,
+	const char *name,
 	git_submodule_update_t update);
 
 /**

src/submodule.c

@@ -806,12 +806,6 @@ int git_submodule_save(git_submodule *submodule)
 	if (error < 0)
 		goto cleanup;
 
-	if (!(error = submodule_config_key_trunc_puts(&key, "update")) &&
-		(val = git_submodule_update_to_str(submodule->update)) != NULL)
-		error = git_config_file_set_string(mods, key.ptr, val);
-	if (error < 0)
-		goto cleanup;
-
 	if (!(error = submodule_config_key_trunc_puts(&key, "fetchRecurseSubmodules")) &&
 		(val = git_submodule_recurse_to_str(submodule->fetch_recurse)) != NULL)
 		error = git_config_file_set_string(mods, key.ptr, val);
@@ -958,24 +952,17 @@ git_submodule_ignore_t git_submodule_ignore(git_submodule *submodule)
 		GIT_SUBMODULE_IGNORE_NONE : submodule->ignore;
 }
 
-int git_submodule_set_ignore(git_repository *repo, const char *name, git_submodule_ignore_t ignore)
+static int write_var(git_repository *repo, const char *name, const char *var, const char *val)
 {
 	git_buf key = GIT_BUF_INIT;
 	git_config_backend *mods;
-	const char *val;
 	int error;
 
-	val = git_submodule_ignore_to_str(ignore);
-	if (!val) {
-		giterr_set(GITERR_SUBMODULE, "invalid ignore value");
-		return -1;
-	}
-
 	mods = open_gitmodules(repo, GITMODULES_CREATE);
 	if (!mods)
 		return -1;
 
-	if ((error = git_buf_printf(&key, "submodule.%s.ignore", name)) < 0)
+	if ((error = git_buf_printf(&key, "submodule.%s.%s", name, var)) < 0)
 		goto cleanup;
 
 	error = git_config_file_set_string(mods, key.ptr, val);
@@ -986,6 +973,22 @@ cleanup:
 	return error;
 }
 
+int git_submodule_set_ignore(git_repository *repo, const char *name, git_submodule_ignore_t ignore)
+{
+	const char *val;
+	int error;
+
+	val = git_submodule_ignore_to_str(ignore);
+	if (!val) {
+		giterr_set(GITERR_SUBMODULE, "invalid ignore value");
+		return -1;
+	}
+
+	error = write_var(repo, name, "ignore", val);
+
+	return error;
+}
+
 git_submodule_update_t git_submodule_update_strategy(git_submodule *submodule)
 {
 	assert(submodule);
@@ -993,19 +996,20 @@ git_submodule_update_t git_submodule_update_strategy(git_submodule *submodule)
 		GIT_SUBMODULE_UPDATE_CHECKOUT : submodule->update;
 }
 
-git_submodule_update_t git_submodule_set_update(
+int git_submodule_set_update(git_repository *repo, const char *name, git_submodule_update_t update)
-	git_submodule *submodule, git_submodule_update_t update)
 {
-	git_submodule_update_t old;
+	const char *val;
+	int error;
 
-	assert(submodule);
+	val = git_submodule_update_to_str(update);
+	if (!val) {
+		giterr_set(GITERR_SUBMODULE, "invalid update value");
+		return -1;
+	}
 
-	if (update == GIT_SUBMODULE_UPDATE_RESET)
+	error = write_var(repo, name, "update", val);
-		update = submodule->update_default;
 
-	old = submodule->update;
+	return error;
-	submodule->update = update;
-	return old;
 }
 
 git_submodule_recurse_t git_submodule_fetch_recurse_submodules(

tests/submodule/modify.c

@@ -139,11 +139,21 @@ void test_submodule_modify__set_ignore(void)
 	git_submodule_free(sm);
 }
 
+void test_submodule_modify__set_update(void)
+{
+	git_submodule *sm;
+
+	cl_git_pass(git_submodule_set_update(g_repo, "sm_changed_head", GIT_SUBMODULE_UPDATE_REBASE));
+
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_head"));
+	cl_assert_equal_i(GIT_SUBMODULE_UPDATE_REBASE, git_submodule_update_strategy(sm));
+	git_submodule_free(sm);
+}
+
 void test_submodule_modify__edit_and_save(void)
 {
 	git_submodule *sm1, *sm2;
 	char *old_url, *old_branch;
-	git_submodule_update_t old_update;
 	git_repository *r2;
 	git_submodule_recurse_t old_fetchrecurse;
 
@@ -155,23 +165,17 @@ void test_submodule_modify__edit_and_save(void)
 	/* modify properties of submodule */
 	cl_git_pass(git_submodule_set_url(sm1, SM_LIBGIT2_URL));
 	cl_git_pass(git_submodule_set_branch(sm1, SM_LIBGIT2_BRANCH));
-	old_update = git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_REBASE);
 	old_fetchrecurse = git_submodule_set_fetch_recurse_submodules(
 		sm1, GIT_SUBMODULE_RECURSE_YES);
 
 	cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm1));
 	cl_assert_equal_s(SM_LIBGIT2_BRANCH, git_submodule_branch(sm1));
-	cl_assert_equal_i(
-		GIT_SUBMODULE_UPDATE_REBASE, git_submodule_update_strategy(sm1));
 	cl_assert_equal_i(
 		GIT_SUBMODULE_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm1));
 
 	/* revert without saving (and confirm setters return old value) */
 	cl_git_pass(git_submodule_set_url(sm1, old_url));
 	cl_git_pass(git_submodule_set_branch(sm1, old_branch));
-	cl_assert_equal_i(
-		GIT_SUBMODULE_UPDATE_REBASE,
-		git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_RESET));
 	cl_assert_equal_i(
 		GIT_SUBMODULE_RECURSE_YES, git_submodule_set_fetch_recurse_submodules(
 			sm1, GIT_SUBMODULE_RECURSE_RESET));
@@ -179,27 +183,17 @@ void test_submodule_modify__edit_and_save(void)
 	/* check that revert was successful */
 	cl_assert_equal_s(old_url, git_submodule_url(sm1));
 	cl_assert_equal_s(old_branch, git_submodule_branch(sm1));
-	cl_assert_equal_i((int)old_update, (int)git_submodule_update_strategy(sm1));
 	cl_assert_equal_i(
 		old_fetchrecurse, git_submodule_fetch_recurse_submodules(sm1));
 
 	/* modify properties of submodule (again) */
 	cl_git_pass(git_submodule_set_url(sm1, SM_LIBGIT2_URL));
 	cl_git_pass(git_submodule_set_branch(sm1, SM_LIBGIT2_BRANCH));
-	git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_REBASE);
 	git_submodule_set_fetch_recurse_submodules(sm1, GIT_SUBMODULE_RECURSE_YES);
 
 	/* call save */
 	cl_git_pass(git_submodule_save(sm1));
 
-	/* attempt to "revert" values */
-	git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_RESET);
-
-	/* but ignore and update should NOT revert because the RESET
-	 * should now be the newly saved value...
-	 */
-	cl_assert_equal_i(
-		(int)GIT_SUBMODULE_UPDATE_REBASE, (int)git_submodule_update_strategy(sm1));
 	cl_assert_equal_i(GIT_SUBMODULE_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm1));
 
 	/* call reload and check that the new values are loaded */
@@ -207,8 +201,6 @@ void test_submodule_modify__edit_and_save(void)
 
 	cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm1));
 	cl_assert_equal_s(SM_LIBGIT2_BRANCH, git_submodule_branch(sm1));
-	cl_assert_equal_i(
-		(int)GIT_SUBMODULE_UPDATE_REBASE, (int)git_submodule_update_strategy(sm1));
 	cl_assert_equal_i(GIT_SUBMODULE_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm1));
 
 	/* unset branch again and verify that the property is deleted in config */
@@ -222,8 +214,6 @@ void test_submodule_modify__edit_and_save(void)
 	cl_git_pass(git_submodule_lookup(&sm2, r2, "sm_changed_head"));
 
 	cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm2));
-	cl_assert_equal_i(
-		GIT_SUBMODULE_UPDATE_REBASE, git_submodule_update_strategy(sm2));
 	cl_assert_equal_i(
 		GIT_SUBMODULE_RECURSE_NO, git_submodule_fetch_recurse_submodules(sm2));