Merge branch 'bw/submodule-config-cleanup'

Code clean-up to avoid mixing values read from the .gitmodules file
and values read from the .git/config file.

* bw/submodule-config-cleanup:
  submodule: remove gitmodules_config
  unpack-trees: improve loading of .gitmodules
  submodule-config: lazy-load a repository's .gitmodules file
  submodule-config: move submodule-config functions to submodule-config.c
  submodule-config: remove support for overlaying repository config
  diff: stop allowing diff to have submodules configured in .git/config
  submodule: remove submodule_config callback routine
  unpack-trees: don't respect submodule.update
  submodule: don't rely on overlayed config when setting diffopts
  fetch: don't overlay config with submodule-config
  submodule--helper: don't overlay config in update-clone
  submodule--helper: don't overlay config in remote_submodule_branch
  add, reset: ensure submodules can be added or reset
  submodule: don't use submodule_from_name
  t7411: check configuration parsing errors

25 files changed, 192 insertions, 373 deletions

diff --git a/builtin/add.c b/builtin/add.c
index 5d5773d5c..c20548e4f 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -116,6 +116,7 @@ int add_files_to_cache(const char *prefix,
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
+	rev.diffopt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	return !!data.add_errors;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2d75ac66c..5c202b7af 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -861,7 +861,7 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	}
 
 	if (starts_with(var, "submodule."))
-		return submodule_config(var, value, NULL);
+		return git_default_submodule_config(var, value, NULL);
 
 	return git_xmerge_config(var, value, NULL);
 }
@@ -1182,7 +1182,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	opts.prefix = prefix;
 	opts.show_progress = -1;
 
-	gitmodules_config();
 	git_config(git_checkout_config, &opts);
 
 	opts.track = BRANCH_TRACK_UNSPECIFIED;
diff --git a/builtin/commit.c b/builtin/commit.c
index b79bcfd5b..b3b04f5dd 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -195,7 +195,6 @@ static void determine_whence(struct wt_status *s)
 static void status_init_config(struct wt_status *s, config_fn_t fn)
 {
 	wt_status_prepare(s);
-	gitmodules_config();
 	git_config(fn, s);
 	determine_whence(s);
 	init_diff_ui_defaults();
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 17bf84d18..e88493ffe 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -26,7 +26,6 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	init_revisions(&rev, prefix);
-	gitmodules_config();
 	rev.abbrev = 0;
 	precompose_argv(argc, argv);
 
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 185e6f9b5..9d772f8f2 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -23,7 +23,6 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	init_revisions(&rev, prefix);
-	gitmodules_config();
 	rev.abbrev = 0;
 	precompose_argv(argc, argv);
 
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 31d2cb410..d66499909 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -110,7 +110,6 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	init_revisions(opt, prefix);
-	gitmodules_config();
 	opt->abbrev = 0;
 	opt->diff = 1;
 	opt->disable_stdin = 1;
diff --git a/builtin/diff.c b/builtin/diff.c
index 7cde6abbc..7e3ebcea3 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -315,8 +315,6 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 			no_index = DIFF_NO_INDEX_IMPLICIT;
 	}
 
-	if (!no_index)
-		gitmodules_config();
 	init_diff_ui_defaults();
 	git_config(git_diff_ui_config, NULL);
 	precompose_argv(argc, argv);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d84c26391..132e3224e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1360,11 +1360,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (depth || deepen_since || deepen_not.nr)
 		deepen = 1;
 
-	if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
-		gitmodules_config();
-		git_config(submodule_config, NULL);
-	}
-
 	if (all) {
 		if (argc == 1)
 			die(_("fetch --all does not take a repository argument"));
diff --git a/builtin/grep.c b/builtin/grep.c
index a70d8e2fb..19e23946a 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1048,10 +1048,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	}
 #endif
 
-	if (recurse_submodules) {
-		gitmodules_config();
-	}
-
 	if (show_in_pager && (cached || list.nr))
 		die(_("--open-files-in-pager only works on the worktree"));
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index c6126eae5..e1339e6d1 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -19,6 +19,7 @@
 #include "pathspec.h"
 #include "run-command.h"
 #include "submodule.h"
+#include "submodule-config.h"
 
 static int abbrev;
 static int show_deleted;
@@ -210,8 +211,6 @@ static void show_submodule(struct repository *superproject,
 	if (repo_read_index(&submodule) < 0)
 		die("index file corrupt");
 
-	repo_read_gitmodules(&submodule);
-
 	show_files(&submodule, dir);
 
 	repo_clear(&submodule);
@@ -609,9 +608,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	if (recurse_submodules)
-		repo_read_gitmodules(the_repository);
-
 	if (recurse_submodules &&
 	    (show_stage || show_deleted || show_others || show_unmerged ||
 	     show_killed || show_modified || show_resolve_undo || with_tree))
diff --git a/builtin/mv.c b/builtin/mv.c
index 94fbaaa5d..ffdd5f01a 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -131,7 +131,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	struct stat st;
 	struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
 
-	gitmodules_config();
 	git_config(git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, builtin_mv_options,
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index d5f618d08..bf87a2710 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -164,8 +164,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 	argc = parse_options(argc, argv, unused_prefix, read_tree_options,
 			     read_tree_usage, 0);
 
-	load_submodule_cache();
-
 	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
 
 	prefix_set = opts.prefix ? 1 : 0;
diff --git a/builtin/reset.c b/builtin/reset.c
index 4a02d7407..d72c7d1c9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -156,6 +156,7 @@ static int read_from_tree(const struct pathspec *pathspec,
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &intent_to_add;
+	opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
 
 	if (do_diff_cache(tree_oid, &opt))
 		return 1;
@@ -308,8 +309,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
-	load_submodule_cache();
-
 	unborn = !strcmp(rev, "HEAD") && get_oid("HEAD", &oid);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */
diff --git a/builtin/rm.c b/builtin/rm.c
index 4057e73fa..d91451fea 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -255,7 +255,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	struct pathspec pathspec;
 	char *seen;
 
-	gitmodules_config();
 	git_config(git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, builtin_rm_options,
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0ff9dd0b8..818fe74f0 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -275,8 +275,6 @@ static void module_list_active(struct module_list *list)
 	int i;
 	struct module_list active_modules = MODULE_LIST_INIT;
 
-	gitmodules_config();
-
 	for (i = 0; i < list->nr; i++) {
 		const struct cache_entry *ce = list->entries[i];
 
@@ -337,9 +335,6 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 	struct strbuf sb = STRBUF_INIT;
 	char *upd = NULL, *url = NULL, *displaypath;
 
-	/* Only loads from .gitmodules, no overlay with .git/config */
-	gitmodules_config();
-
 	if (prefix && get_super_prefix())
 		die("BUG: cannot have prefix and superprefix");
 	else if (prefix)
@@ -475,7 +470,6 @@ static int module_name(int argc, const char **argv, const char *prefix)
 	if (argc != 2)
 		usage(_("git submodule--helper name <path>"));
 
-	gitmodules_config();
 	sub = submodule_from_path(&null_oid, argv[1]);
 
 	if (!sub)
@@ -780,6 +774,10 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 					   struct strbuf *out)
 {
 	const struct submodule *sub = NULL;
+	const char *url = NULL;
+	const char *update_string;
+	enum submodule_update_type update_type;
+	char *key;
 	struct strbuf displaypath_sb = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	const char *displaypath = NULL;
@@ -808,9 +806,17 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		goto cleanup;
 	}
 
+	key = xstrfmt("submodule.%s.update", sub->name);
+	if (!repo_config_get_string_const(the_repository, key, &update_string)) {
+		update_type = parse_submodule_update_type(update_string);
+	} else {
+		update_type = sub->update_strategy.type;
+	}
+	free(key);
+
 	if (suc->update.type == SM_UPDATE_NONE
 	    || (suc->update.type == SM_UPDATE_UNSPECIFIED
-		&& sub->update_strategy.type == SM_UPDATE_NONE)) {
+		&& update_type == SM_UPDATE_NONE)) {
 		strbuf_addf(out, _("Skipping submodule '%s'"), displaypath);
 		strbuf_addch(out, '\n');
 		goto cleanup;
@@ -823,6 +829,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 	}
 
 	strbuf_reset(&sb);
+	strbuf_addf(&sb, "submodule.%s.url", sub->name);
+	if (repo_config_get_string_const(the_repository, sb.buf, &url))
+		url = sub->url;
+
+	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/.git", ce->name);
 	needs_cloning = !file_exists(sb.buf);
 
@@ -851,7 +862,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		argv_array_push(&child->args, "--depth=1");
 	argv_array_pushl(&child->args, "--path", sub->path, NULL);
 	argv_array_pushl(&child->args, "--name", sub->name, NULL);
-	argv_array_pushl(&child->args, "--url", sub->url, NULL);
+	argv_array_pushl(&child->args, "--url", url, NULL);
 	if (suc->references.nr) {
 		struct string_list_item *item;
 		for_each_string_list_item(item, &suc->references)
@@ -1025,10 +1036,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)
 	if (pathspec.nr)
 		suc.warn_if_uninitialized = 1;
 
-	/* Overlay the parsed .gitmodules file with .git/config */
-	gitmodules_config();
-	git_config(submodule_config, NULL);
-
 	run_processes_parallel(max_jobs,
 			       update_clone_get_next_task,
 			       update_clone_start_failure,
@@ -1066,17 +1073,22 @@ static int resolve_relative_path(int argc, const char **argv, const char *prefix
 static const char *remote_submodule_branch(const char *path)
 {
 	const struct submodule *sub;
-	gitmodules_config();
-	git_config(submodule_config, NULL);
+	const char *branch = NULL;
+	char *key;
 
 	sub = submodule_from_path(&null_oid, path);
 	if (!sub)
 		return NULL;
 
-	if (!sub->branch)
+	key = xstrfmt("submodule.%s.branch", sub->name);
+	if (repo_config_get_string_const(the_repository, key, &branch))
+		branch = sub->branch;
+	free(key);
+
+	if (!branch)
 		return "master";
 
-	if (!strcmp(sub->branch, ".")) {
+	if (!strcmp(branch, ".")) {
 		unsigned char sha1[20];
 		const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
 
@@ -1094,7 +1106,7 @@ static const char *remote_submodule_branch(const char *path)
 		return refname;
 	}
 
-	return sub->branch;
+	return branch;
 }
 
 static int resolve_remote_submodule_branch(int argc, const char **argv,
@@ -1213,9 +1225,6 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, embed_gitdir_options,
 			     git_submodule_helper_usage, 0);
 
-	gitmodules_config();
-	git_config(submodule_config, NULL);
-
 	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
 		return 1;
 
@@ -1231,8 +1240,6 @@ static int is_active(int argc, const char **argv, const char *prefix)
 	if (argc != 2)
 		die("submodule--helper is-active takes exactly 1 argument");
 
-	gitmodules_config();
-
 	return !is_submodule_active(the_repository, argv[1]);
 }
 
diff --git a/diff.c b/diff.c
index 24bec4111..a74cc0848 100644
--- a/diff.c
+++ b/diff.c
@@ -401,9 +401,6 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
-	if (starts_with(var, "submodule."))
-		return parse_submodule_config_option(var, value);
-
 	if (git_diff_heuristic_config(var, value, cb) < 0)
 		return -1;
 
diff --git a/submodule-config.c b/submodule-config.c
index 0c839019e..2aa8a1747 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -18,6 +18,7 @@ struct submodule_cache {
 	struct hashmap for_path;
 	struct hashmap for_name;
 	unsigned initialized:1;
+	unsigned gitmodules_read:1;
 };
 
 /*
@@ -99,6 +100,7 @@ static void submodule_cache_clear(struct submodule_cache *cache)
 	hashmap_free(&cache->for_path, 1);
 	hashmap_free(&cache->for_name, 1);
 	cache->initialized = 0;
+	cache->gitmodules_read = 0;
 }
 
 void submodule_cache_free(struct submodule_cache *cache)
@@ -455,9 +457,9 @@ static int parse_config(const char *var, const char *value, void *data)
 	return ret;
 }
 
-int gitmodule_oid_from_commit(const struct object_id *treeish_name,
-				      struct object_id *gitmodules_oid,
-				      struct strbuf *rev)
+static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
+				     struct object_id *gitmodules_oid,
+				     struct strbuf *rev)
 {
 	int ret = 0;
 
@@ -558,13 +560,11 @@ static void submodule_cache_check_init(struct repository *repo)
 	submodule_cache_init(repo->submodule_cache);
 }
 
-int submodule_config_option(struct repository *repo,
-			    const char *var, const char *value)
+static int gitmodules_cb(const char *var, const char *value, void *data)
 {
+	struct repository *repo = data;
 	struct parse_config_parameter parameter;
 
-	submodule_cache_check_init(repo);
-
 	parameter.cache = repo->submodule_cache;
 	parameter.treeish_name = NULL;
 	parameter.gitmodules_sha1 = null_sha1;
@@ -573,22 +573,63 @@ int submodule_config_option(struct repository *repo,
 	return parse_config(var, value, &parameter);
 }
 
-int parse_submodule_config_option(const char *var, const char *value)
+void repo_read_gitmodules(struct repository *repo)
 {
-	return submodule_config_option(the_repository, var, value);
+	submodule_cache_check_init(repo);
+
+	if (repo->worktree) {
+		char *gitmodules;
+
+		if (repo_read_index(repo) < 0)
+			return;
+
+		gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
+
+		if (!is_gitmodules_unmerged(repo->index))
+			git_config_from_file(gitmodules_cb, gitmodules, repo);
+
+		free(gitmodules);
+	}
+
+	repo->submodule_cache->gitmodules_read = 1;
+}
+
+void gitmodules_config_oid(const struct object_id *commit_oid)
+{
+	struct strbuf rev = STRBUF_INIT;
+	struct object_id oid;
+
+	submodule_cache_check_init(the_repository);
+
+	if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) {
+		git_config_from_blob_oid(gitmodules_cb, rev.buf,
+					 &oid, the_repository);
+	}
+	strbuf_release(&rev);
+
+	the_repository->submodule_cache->gitmodules_read = 1;
+}
+
+static void gitmodules_read_check(struct repository *repo)
+{
+	submodule_cache_check_init(repo);
+
+	/* read the repo's .gitmodules file if it hasn't been already */
+	if (!repo->submodule_cache->gitmodules_read)
+		repo_read_gitmodules(repo);
 }
 
 const struct submodule *submodule_from_name(const struct object_id *treeish_name,
 		const char *name)
 {
-	submodule_cache_check_init(the_repository);
+	gitmodules_read_check(the_repository);
 	return config_from(the_repository->submodule_cache, treeish_name, name, lookup_name);
 }
 
 const struct submodule *submodule_from_path(const struct object_id *treeish_name,
 		const char *path)
 {
-	submodule_cache_check_init(the_repository);
+	gitmodules_read_check(the_repository);
 	return config_from(the_repository->submodule_cache, treeish_name, path, lookup_path);
 }
 
@@ -596,7 +637,7 @@ const struct submodule *submodule_from_cache(struct repository *repo,
 					     const struct object_id *treeish_name,
 					     const char *key)
 {
-	submodule_cache_check_init(repo);
+	gitmodules_read_check(repo);
 	return config_from(repo->submodule_cache, treeish_name,
 			   key, lookup_path);
 }
diff --git a/submodule-config.h b/submodule-config.h
index cccd34b92..e3845831f 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -34,9 +34,8 @@ extern int option_fetch_parse_recurse_submodules(const struct option *opt,
 						 const char *arg, int unset);
 extern int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
 extern int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
-extern int parse_submodule_config_option(const char *var, const char *value);
-extern int submodule_config_option(struct repository *repo,
-				   const char *var, const char *value);
+extern void repo_read_gitmodules(struct repository *repo);
+extern void gitmodules_config_oid(const struct object_id *commit_oid);
 extern const struct submodule *submodule_from_name(
 		const struct object_id *commit_or_tree, const char *name);
 extern const struct submodule *submodule_from_path(
@@ -44,9 +43,6 @@ extern const struct submodule *submodule_from_path(
 extern const struct submodule *submodule_from_cache(struct repository *repo,
 						    const struct object_id *treeish_name,
 						    const char *key);
-extern int gitmodule_oid_from_commit(const struct object_id *commit_oid,
-				     struct object_id *gitmodules_oid,
-				     struct strbuf *rev);
 extern void submodule_free(void);
 
 #endif /* SUBMODULE_CONFIG_H */
diff --git a/submodule.c b/submodule.c
index d53181ce7..3cea8221e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -165,31 +165,18 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 {
 	const struct submodule *submodule = submodule_from_path(&null_oid, path);
 	if (submodule) {
-		if (submodule->ignore)
-			handle_ignore_submodules_arg(diffopt, submodule->ignore);
-		else if (is_gitmodules_unmerged(&the_index))
-			DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
-	}
-}
+		const char *ignore;
+		char *key;
 
-/* For loading from the .gitmodules file. */
-static int git_modules_config(const char *var, const char *value, void *cb)
-{
-	if (starts_with(var, "submodule."))
-		return parse_submodule_config_option(var, value);
-	return 0;
-}
+		key = xstrfmt("submodule.%s.ignore", submodule->name);
+		if (repo_config_get_string_const(the_repository, key, &ignore))
+			ignore = submodule->ignore;
+		free(key);
 
-/* Loads all submodule settings from the config. */
-int submodule_config(const char *var, const char *value, void *cb)
-{
-	if (!strcmp(var, "submodule.recurse")) {
-		int v = git_config_bool(var, value) ?
-			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
-		config_update_recurse_submodules = v;
-		return 0;
-	} else {
-		return git_modules_config(var, value, cb);
+		if (ignore)
+			handle_ignore_submodules_arg(diffopt, ignore);
+		else if (is_gitmodules_unmerged(&the_index))
+			DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
 	}
 }
 
@@ -221,55 +208,6 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
 	return 0;
 }
 
-void load_submodule_cache(void)
-{
-	if (config_update_recurse_submodules == RECURSE_SUBMODULES_OFF)
-		return;
-
-	gitmodules_config();
-	git_config(submodule_config, NULL);
-}
-
-static int gitmodules_cb(const char *var, const char *value, void *data)
-{
-	struct repository *repo = data;
-	return submodule_config_option(repo, var, value);
-}
-
-void repo_read_gitmodules(struct repository *repo)
-{
-	if (repo->worktree) {
-		char *gitmodules;
-
-		if (repo_read_index(repo) < 0)
-			return;
-
-		gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
-
-		if (!is_gitmodules_unmerged(repo->index))
-			git_config_from_file(gitmodules_cb, gitmodules, repo);
-
-		free(gitmodules);
-	}
-}
-
-void gitmodules_config(void)
-{
-	repo_read_gitmodules(the_repository);
-}
-
-void gitmodules_config_oid(const struct object_id *commit_oid)
-{
-	struct strbuf rev = STRBUF_INIT;
-	struct object_id oid;
-
-	if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) {
-		git_config_from_blob_oid(submodule_config, rev.buf,
-					 &oid, NULL);
-	}
-	strbuf_release(&rev);
-}
-
 /*
  * Determine if a submodule has been initialized at a given 'path'
  */
@@ -398,24 +336,38 @@ void die_path_inside_submodule(const struct index_state *istate,
 	}
 }
 
-int parse_submodule_update_strategy(const char *value,
-		struct submodule_update_strategy *dst)
+enum submodule_update_type parse_submodule_update_type(const char *value)
 {
-	free((void*)dst->command);
-	dst->command = NULL;
 	if (!strcmp(value, "none"))
-		dst->type = SM_UPDATE_NONE;
+		return SM_UPDATE_NONE;
 	else if (!strcmp(value, "checkout"))
-		dst->type = SM_UPDATE_CHECKOUT;
+		return SM_UPDATE_CHECKOUT;
 	else if (!strcmp(value, "rebase"))
-		dst->type = SM_UPDATE_REBASE;
+		return SM_UPDATE_REBASE;
 	else if (!strcmp(value, "merge"))
-		dst->type = SM_UPDATE_MERGE;
-	else if (skip_prefix(value, "!", &value)) {
-		dst->type = SM_UPDATE_COMMAND;
-		dst->command = xstrdup(value);
-	} else
+		return SM_UPDATE_MERGE;
+	else if (*value == '!')
+		return SM_UPDATE_COMMAND;
+	else
+		return SM_UPDATE_UNSPECIFIED;
+}
+
+int parse_submodule_update_strategy(const char *value,
+		struct submodule_update_strategy *dst)
+{
+	enum submodule_update_type type;
+
+	free((void*)dst->command);
+	dst->command = NULL;
+
+	type = parse_submodule_update_type(value);
+	if (type == SM_UPDATE_UNSPECIFIED)
 		return -1;
+
+	dst->type = type;
+	if (type == SM_UPDATE_COMMAND)
+		dst->command = xstrdup(value + 1);
+
 	return 0;
 }
 
@@ -1130,7 +1082,6 @@ int submodule_touches_in_range(struct object_id *excl_oid,
 	struct argv_array args = ARGV_ARRAY_INIT;
 	int ret;
 
-	gitmodules_config();
 	/* No need to check if there are no submodules configured */
 	if (!submodule_from_path(NULL, NULL))
 		return 0;
@@ -1179,19 +1130,27 @@ static int get_next_submodule(struct child_process *cp,
 			continue;
 
 		submodule = submodule_from_path(&null_oid, ce->name);
-		if (!submodule)
-			submodule = submodule_from_name(&null_oid, ce->name);
 
 		default_argv = "yes";
 		if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) {
-			if (submodule &&
-			    submodule->fetch_recurse !=
-						RECURSE_SUBMODULES_NONE) {
-				if (submodule->fetch_recurse ==
-						RECURSE_SUBMODULES_OFF)
+			int fetch_recurse = RECURSE_SUBMODULES_NONE;
+
+			if (submodule) {
+				char *key;
+				const char *value;
+
+				fetch_recurse = submodule->fetch_recurse;
+				key = xstrfmt("submodule.%s.fetchRecurseSubmodules", submodule->name);
+				if (!repo_config_get_string_const(the_repository, key, &value)) {
+					fetch_recurse = parse_fetch_recurse_submodules_arg(key, value);
+				}
+				free(key);
+			}
+
+			if (fetch_recurse != RECURSE_SUBMODULES_NONE) {
+				if (fetch_recurse == RECURSE_SUBMODULES_OFF)
 					continue;
-				if (submodule->fetch_recurse ==
-						RECURSE_SUBMODULES_ON_DEMAND) {
+				if (fetch_recurse == RECURSE_SUBMODULES_ON_DEMAND) {
 					if (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))
 						continue;
 					default_argv = "on-demand";
@@ -2029,7 +1988,6 @@ int submodule_to_gitdir(struct strbuf *buf, const char *submodule)
 		strbuf_addstr(buf, git_dir);
 	}
 	if (!is_git_directory(buf->buf)) {
-		gitmodules_config();
 		sub = submodule_from_path(&null_oid, submodule);
 		if (!sub) {
 			ret = -1;
diff --git a/submodule.h b/submodule.h
index 5f26b5404..6b52133c8 100644
--- a/submodule.h
+++ b/submodule.h
@@ -40,16 +40,11 @@ extern int remove_path_from_gitmodules(const char *path);
 extern void stage_updated_gitmodules(void);
 extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
 		const char *path);
-extern int submodule_config(const char *var, const char *value, void *cb);
 extern int git_default_submodule_config(const char *var, const char *value, void *cb);
 
 struct option;
 int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
 						     const char *arg, int unset);
-void load_submodule_cache(void);
-extern void gitmodules_config(void);
-extern void repo_read_gitmodules(struct repository *repo);
-extern void gitmodules_config_oid(const struct object_id *commit_oid);
 extern int is_submodule_active(struct repository *repo, const char *path);
 /*
  * Determine if a submodule has been populated at a given 'path' by checking if
@@ -62,6 +57,7 @@ extern void die_in_unpopulated_submodule(const struct index_state *istate,
 					 const char *prefix);
 extern void die_path_inside_submodule(const struct index_state *istate,
 				      const struct pathspec *ps);
+extern enum submodule_update_type parse_submodule_update_type(const char *value);
 extern int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
 extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
diff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c
index e13fbcc1b..f23db3b19 100644
--- a/t/helper/test-submodule-config.c
+++ b/t/helper/test-submodule-config.c
@@ -10,11 +10,6 @@ static void die_usage(int argc, const char **argv, const char *msg)
 	exit(1);
 }
 
-static int git_test_config(const char *var, const char *value, void *cb)
-{
-	return parse_submodule_config_option(var, value);
-}
-
 int cmd_main(int argc, const char **argv)
 {
 	const char **arg = argv;
@@ -37,8 +32,6 @@ int cmd_main(int argc, const char **argv)
 		die_usage(argc, argv, "Wrong number of arguments.");
 
 	setup_git_directory();
-	gitmodules_config();
-	git_config(git_test_config, NULL);
 
 	while (*arg) {
 		struct object_id commit_oid;
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 518bf9524..2ffd11a14 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -113,35 +113,6 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
 	! test -s actual4
 '
 
-test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.git/config]' '
-	git config diff.ignoreSubmodules all &&
-	git diff HEAD >actual &&
-	! test -s actual &&
-	git config submodule.subname.ignore none &&
-	git config submodule.subname.path sub &&
-	git diff HEAD >actual &&
-	sed -e "1,/^@@/d" actual >actual.body &&
-	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body &&
-	git config submodule.subname.ignore all &&
-	git diff HEAD >actual2 &&
-	! test -s actual2 &&
-	git config submodule.subname.ignore untracked &&
-	git diff HEAD >actual3 &&
-	sed -e "1,/^@@/d" actual3 >actual3.body &&
-	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual3.body &&
-	git config submodule.subname.ignore dirty &&
-	git diff HEAD >actual4 &&
-	! test -s actual4 &&
-	git diff HEAD --ignore-submodules=none >actual &&
-	sed -e "1,/^@@/d" actual >actual.body &&
-	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body &&
-	git config --remove-section submodule.subname &&
-	git config --unset diff.ignoreSubmodules
-'
-
 test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' '
 	git config diff.ignoreSubmodules dirty &&
 	git diff HEAD >actual &&
@@ -208,24 +179,6 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
 	! test -s actual4
 '
 
-test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.git/config]' '
-	git config submodule.subname.ignore all &&
-	git config submodule.subname.path sub &&
-	git diff HEAD >actual2 &&
-	! test -s actual2 &&
-	git config submodule.subname.ignore untracked &&
-	git diff HEAD >actual3 &&
-	! test -s actual3 &&
-	git config submodule.subname.ignore dirty &&
-	git diff HEAD >actual4 &&
-	! test -s actual4 &&
-	git diff --ignore-submodules=none HEAD >actual &&
-	sed -e "1,/^@@/d" actual >actual.body &&
-	expect_from_to >expect.body $subprev $subprev-dirty &&
-	test_cmp expect.body actual.body &&
-	git config --remove-section submodule.subname
-'
-
 test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.gitmodules]' '
 	git config --add -f .gitmodules submodule.subname.ignore all &&
 	git config --add -f .gitmodules submodule.subname.path sub &&
@@ -261,26 +214,6 @@ test_expect_success 'git diff between submodule commits' '
 	! test -s actual
 '
 
-test_expect_success 'git diff between submodule commits [.git/config]' '
-	git diff HEAD^..HEAD >actual &&
-	sed -e "1,/^@@/d" actual >actual.body &&
-	expect_from_to >expect.body $subtip $subprev &&
-	test_cmp expect.body actual.body &&
-	git config submodule.subname.ignore dirty &&
-	git config submodule.subname.path sub &&
-	git diff HEAD^..HEAD >actual &&
-	sed -e "1,/^@@/d" actual >actual.body &&
-	expect_from_to >expect.body $subtip $subprev &&
-	test_cmp expect.body actual.body &&
-	git config submodule.subname.ignore all &&
-	git diff HEAD^..HEAD >actual &&
-	! test -s actual &&
-	git diff --ignore-submodules=dirty HEAD^..HEAD >actual &&
-	sed -e "1,/^@@/d" actual >actual.body &&
-	expect_from_to >expect.body $subtip $subprev &&
-	git config --remove-section submodule.subname
-'
-
 test_expect_success 'git diff between submodule commits [.gitmodules]' '
 	git diff HEAD^..HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index e9c3335b7..6f8337ffb 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -46,16 +46,6 @@ test_expect_success 'submodule update aborts on missing gitmodules url' '
 	test_must_fail git submodule init
 '
 
-test_expect_success 'configuration parsing' '
-	test_when_finished "rm -f .gitmodules" &&
-	cat >.gitmodules <<-\EOF &&
-	[submodule "s"]
-		path
-		ignore
-	EOF
-	test_must_fail git status
-'
-
 test_expect_success 'setup - repository in init subdirectory' '
 	mkdir init &&
 	(
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index eea36f1db..46c09c776 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -31,6 +31,21 @@ test_expect_success 'submodule config cache setup' '
 	)
 '
 
+test_expect_success 'configuration parsing with error' '
+	test_when_finished "rm -rf repo" &&
+	test_create_repo repo &&
+	cat >repo/.gitmodules <<-\EOF &&
+	[submodule "s"]
+		path
+		ignore
+	EOF
+	(
+		cd repo &&
+		test_must_fail test-submodule-config "" s 2>actual &&
+		test_i18ngrep "bad config" actual
+	)
+'
+
 cat >super/expect <<EOF
 Submodule name: 'a' for path 'a'
 Submodule name: 'a' for path 'b'
@@ -107,78 +122,6 @@ test_expect_success 'using different treeishs works' '
 	)
 '
 
-cat >super/expect_url <<EOF
-Submodule url: 'git@somewhere.else.net:a.git' for path 'b'
-Submodule url: 'git@somewhere.else.net:submodule.git' for path 'submodule'
-EOF
-
-cat >super/expect_local_path <<EOF
-Submodule name: 'a' for path 'c'
-Submodule name: 'submodule' for path 'submodule'
-EOF
-
-test_expect_success 'reading of local configuration' '
-	(cd super &&
-		old_a=$(git config submodule.a.url) &&
-		old_submodule=$(git config submodule.submodule.url) &&
-		git config submodule.a.url git@somewhere.else.net:a.git &&
-		git config submodule.submodule.url git@somewhere.else.net:submodule.git &&
-		test-submodule-config --url \
-			"" b \
-			"" submodule \
-				>actual &&
-		test_cmp expect_url actual &&
-		git config submodule.a.path c &&
-		test-submodule-config \
-			"" c \
-			"" submodule \
-				>actual &&
-		test_cmp expect_local_path actual &&
-		git config submodule.a.url "$old_a" &&
-		git config submodule.submodule.url "$old_submodule" &&
-		git config --unset submodule.a.path c
-	)
-'
-
-cat >super/expect_url <<EOF
-Submodule url: '../submodule' for path 'b'
-Submodule url: 'git@somewhere.else.net:submodule.git' for path 'submodule'
-EOF
-
-test_expect_success 'reading of local configuration for uninitialized submodules' '
-	(
-		cd super &&
-		git submodule deinit -f b &&
-		old_submodule=$(git config submodule.submodule.url) &&
-		git config submodule.submodule.url git@somewhere.else.net:submodule.git &&
-		test-submodule-config --url \
-			"" b \
-			"" submodule \
-				>actual &&
-		test_cmp expect_url actual &&
-		git config submodule.submodule.url "$old_submodule" &&
-		git submodule init b
-	)
-'
-
-cat >super/expect_fetchrecurse_die.err <<EOF
-fatal: bad submodule.submodule.fetchrecursesubmodules argument: blabla
-EOF
-
-test_expect_success 'local error in fetchrecursesubmodule dies early' '
-	(cd super &&
-		git config submodule.submodule.fetchrecursesubmodules blabla &&
-		test_must_fail test-submodule-config \
-			"" b \
-			"" submodule \
-				>actual.out 2>actual.err &&
-		touch expect_fetchrecurse_die.out &&
-		test_cmp expect_fetchrecurse_die.out actual.out  &&
-		test_cmp expect_fetchrecurse_die.err actual.err  &&
-		git config --unset submodule.submodule.fetchrecursesubmodules
-	)
-'
-
 test_expect_success 'error in history in fetchrecursesubmodule lets continue' '
 	(cd super &&
 		git config -f .gitmodules \
diff --git a/unpack-trees.c b/unpack-trees.c
index 78590f1bf..68d34259c 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1,5 +1,6 @@
 #define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
+#include "repository.h"
 #include "config.h"
 #include "dir.h"
 #include "tree.h"
@@ -255,47 +256,41 @@ static int check_submodule_move_head(const struct cache_entry *ce,
 {
 	unsigned flags = SUBMODULE_MOVE_HEAD_DRY_RUN;
 	const struct submodule *sub = submodule_from_ce(ce);
+
 	if (!sub)
 		return 0;
 
 	if (o->reset)
 		flags |= SUBMODULE_MOVE_HEAD_FORCE;
 
-	switch (sub->update_strategy.type) {
-	case SM_UPDATE_UNSPECIFIED:
-	case SM_UPDATE_CHECKOUT:
-		if (submodule_move_head(ce->name, old_id, new_id, flags))
-			return o->gently ? -1 :
-				add_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name);
-		return 0;
-	case SM_UPDATE_NONE:
-		return 0;
-	case SM_UPDATE_REBASE:
-	case SM_UPDATE_MERGE:
-	case SM_UPDATE_COMMAND:
-	default:
-		warning(_("submodule update strategy not supported for submodule '%s'"), ce->name);
-		return -1;
-	}
+	if (submodule_move_head(ce->name, old_id, new_id, flags))
+		return o->gently ? -1 :
+				   add_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name);
+	return 0;
 }
 
-static void reload_gitmodules_file(struct index_state *index,
-				   struct checkout *state)
+/*
+ * Preform the loading of the repository's gitmodules file.  This function is
+ * used by 'check_update()' to perform loading of the gitmodules file in two
+ * differnt situations:
+ * (1) before removing entries from the working tree if the gitmodules file has
+ *     been marked for removal.  This situation is specified by 'state' == NULL.
+ * (2) before checking out entries to the working tree if the gitmodules file
+ *     has been marked for update.  This situation is specified by 'state' != NULL.
+ */
+static void load_gitmodules_file(struct index_state *index,
+				 struct checkout *state)
 {
-	int i;
-	for (i = 0; i < index->cache_nr; i++) {
-		struct cache_entry *ce = index->cache[i];
-		if (ce->ce_flags & CE_UPDATE) {
-			int r = strcmp(ce->name, GITMODULES_FILE);
-			if (r < 0)
-				continue;
-			else if (r == 0) {
-				submodule_free();
-				checkout_entry(ce, state, NULL);
-				gitmodules_config();
-				git_config(submodule_config, NULL);
-			} else
-				break;
+	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
+
+	if (pos >= 0) {
+		struct cache_entry *ce = index->cache[pos];
+		if (!state && ce->ce_flags & CE_WT_REMOVE) {
+			repo_read_gitmodules(the_repository);
+		} else if (state && (ce->ce_flags & CE_UPDATE)) {
+			submodule_free();
+			checkout_entry(ce, state, NULL);
+			repo_read_gitmodules(the_repository);
 		}
 	}
 }
@@ -308,19 +303,9 @@ static void unlink_entry(const struct cache_entry *ce)
 {
 	const struct submodule *sub = submodule_from_ce(ce);
 	if (sub) {
-		switch (sub->update_strategy.type) {
-		case SM_UPDATE_UNSPECIFIED:
-		case SM_UPDATE_CHECKOUT:
-		case SM_UPDATE_REBASE:
-		case SM_UPDATE_MERGE:
-			/* state.force is set at the caller. */
-			submodule_move_head(ce->name, "HEAD", NULL,
-					    SUBMODULE_MOVE_HEAD_FORCE);
-			break;
-		case SM_UPDATE_NONE:
-		case SM_UPDATE_COMMAND:
-			return; /* Do not touch the submodule. */
-		}
+		/* state.force is set at the caller. */
+		submodule_move_head(ce->name, "HEAD", NULL,
+				    SUBMODULE_MOVE_HEAD_FORCE);
 	}
 	if (!check_leading_path(ce->name, ce_namelen(ce)))
 		return;
@@ -364,6 +349,10 @@ static int check_updates(struct unpack_trees_options *o)
 
 	if (o->update)
 		git_attr_set_direction(GIT_ATTR_CHECKOUT, index);
+
+	if (should_update_submodules() && o->update && !o->dry_run)
+		load_gitmodules_file(index, NULL);
+
 	for (i = 0; i < index->cache_nr; i++) {
 		const struct cache_entry *ce = index->cache[i];
 
@@ -377,7 +366,7 @@ static int check_updates(struct unpack_trees_options *o)
 	remove_scheduled_dirs();
 
 	if (should_update_submodules() && o->update && !o->dry_run)
-		reload_gitmodules_file(index, &state);
+		load_gitmodules_file(index, &state);
 
 	enable_delayed_checkout(&state);
 	for (i = 0; i < index->cache_nr; i++) {