submodule: make `_set_update_fetch_recurse_submodules()` affect the config

Similarly to the other ones. In this test we copy over testing
`RECURSE_YES` which shows an error in our handling of the `YES` variant
which we may have to port to the rest.

include/git2/submodule.h

@@ -519,18 +519,18 @@ GIT_EXTERN(git_submodule_recurse_t) git_submodule_fetch_recurse_submodules(
 	git_submodule *submodule);
 
 /**
- * Set the fetchRecurseSubmodules rule for a submodule.
+ * Set the fetchRecurseSubmodules rule for a submodule in the configuration
  *
- * This sets the submodule.<name>.fetchRecurseSubmodules value for
- * the submodule.  You should call `git_submodule_save()` if you want
- * to persist the new value.
+ * This setting won't affect any existing instances.
  *
- * @param submodule The submodule to modify
+ * @param repo the repository to affect
+ * @param name the submodule to configure
  * @param fetch_recurse_submodules Boolean value
  * @return old value for fetchRecurseSubmodules
  */
-GIT_EXTERN(git_submodule_recurse_t) git_submodule_set_fetch_recurse_submodules(
-	git_submodule *submodule,
+GIT_EXTERN(int) git_submodule_set_fetch_recurse_submodules(
+	git_repository *repo,
+	const char *name,
 	git_submodule_recurse_t fetch_recurse_submodules);
 
 /**

src/submodule.c

@@ -769,7 +769,6 @@ int git_submodule_save(git_submodule *submodule)
 	int error = 0;
 	git_config_backend *mods;
 	git_buf key = GIT_BUF_INIT;
-	const char *val;
 
 	assert(submodule);
 
@@ -806,12 +805,6 @@ int git_submodule_save(git_submodule *submodule)
 	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);
-	if (error < 0)
-		goto cleanup;
-
 	/* update internal defaults */
 
 	submodule->ignore_default = submodule->ignore;
@@ -965,7 +958,11 @@ static int write_var(git_repository *repo, const char *name, const char *var, co
 	if ((error = git_buf_printf(&key, "submodule.%s.%s", name, var)) < 0)
 		goto cleanup;
 
+	if (val)
 		error = git_config_file_set_string(mods, key.ptr, val);
+	else
+		error = git_config_file_delete(mods, key.ptr);
+
 	git_buf_free(&key);
 
 cleanup:
@@ -1019,20 +1016,31 @@ git_submodule_recurse_t git_submodule_fetch_recurse_submodules(
 	return submodule->fetch_recurse;
 }
 
-git_submodule_recurse_t git_submodule_set_fetch_recurse_submodules(
-	git_submodule *submodule,
-	git_submodule_recurse_t fetch_recurse_submodules)
+int git_submodule_set_fetch_recurse_submodules(git_repository *repo, const char *name, git_submodule_recurse_t recurse)
 {
-	git_submodule_recurse_t old;
+	const char *val;
+	int error;
 
-	assert(submodule);
+	assert(repo && name);
+
+	val = git_submodule_recurse_to_str(recurse);
+	if (!val) {
+		switch (recurse) {
+		case GIT_SUBMODULE_RECURSE_YES:
+			val = "true";
+			break;
+		case GIT_SUBMODULE_RECURSE_NO:
+			val = NULL;
+			break;
+		default:
+			giterr_set(GITERR_SUBMODULE, "invalid recurse value");
+			return -1;
+		}
+	}
 
-	if (fetch_recurse_submodules == GIT_SUBMODULE_RECURSE_RESET)
-		fetch_recurse_submodules = submodule->fetch_recurse_default;
+	error = write_var(repo, name, "fetchRecurseSubmodules", val);
 
-	old = submodule->fetch_recurse;
-	submodule->fetch_recurse = fetch_recurse_submodules;
-	return old;
+	return error;
 }
 
 static int submodule_repo_create(

tests/submodule/modify.c

@@ -150,12 +150,27 @@ void test_submodule_modify__set_update(void)
 	git_submodule_free(sm);
 }
 
+void test_submodule_modify__set_fetch_recurse_submodules(void)
+{
+	git_submodule *sm;
+
+	cl_git_pass(git_submodule_set_fetch_recurse_submodules(g_repo, "sm_changed_head", GIT_SUBMODULE_RECURSE_YES));
+
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_head"));
+	cl_assert_equal_i(GIT_SUBMODULE_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm));
+	git_submodule_free(sm);
+
+	git_submodule_set_fetch_recurse_submodules(g_repo, "sm_changed_head", GIT_SUBMODULE_RECURSE_ONDEMAND);
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "sm_changed_head"));
+	cl_assert_equal_i(GIT_SUBMODULE_RECURSE_ONDEMAND, git_submodule_fetch_recurse_submodules(sm));
+	git_submodule_free(sm);
+}
+
 void test_submodule_modify__edit_and_save(void)
 {
 	git_submodule *sm1, *sm2;
 	char *old_url, *old_branch;
 	git_repository *r2;
-	git_submodule_recurse_t old_fetchrecurse;
 
 	cl_git_pass(git_submodule_lookup(&sm1, g_repo, "sm_changed_head"));
 
@@ -165,43 +180,29 @@ 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_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_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_RECURSE_YES, git_submodule_set_fetch_recurse_submodules(
-			sm1, GIT_SUBMODULE_RECURSE_RESET));
 
 	/* 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(
-		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_fetch_recurse_submodules(sm1, GIT_SUBMODULE_RECURSE_YES);
 
 	/* call save */
 	cl_git_pass(git_submodule_save(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 */
 	cl_git_pass(git_submodule_reload(sm1, 0));
 
 	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_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm1));
 
 	/* unset branch again and verify that the property is deleted in config */
 	cl_git_pass(git_submodule_set_branch(sm1, NULL));
@@ -214,19 +215,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_RECURSE_NO, git_submodule_fetch_recurse_submodules(sm2));
-
-	/* set fetchRecurseSubmodules on-demand */
-	cl_git_pass(git_submodule_reload(sm1, 0));
-	git_submodule_set_fetch_recurse_submodules(sm1, GIT_SUBMODULE_RECURSE_ONDEMAND);
-	cl_assert_equal_i(
-		GIT_SUBMODULE_RECURSE_ONDEMAND, git_submodule_fetch_recurse_submodules(sm1));
-	/* call save */
-	cl_git_pass(git_submodule_save(sm1));
-	cl_git_pass(git_submodule_reload(sm1, 0));
-	cl_assert_equal_i(
-		GIT_SUBMODULE_RECURSE_ONDEMAND, git_submodule_fetch_recurse_submodules(sm1));
 
 	git_submodule_free(sm1);
 	git_submodule_free(sm2);