[PATCHv3 0/2] fix recurse.submodule config for git pull

[PATCHv3 0/2] fix recurse.submodule config for git pull

[Nicolas Morey-Chaisemartin]

Changes since v2:
- Add a patch that fixes the option parsing order (parse config before cli, not the other way around)
- Enhance the tests to check --recurse-submodule and submodule.recurse combinations

Nicolas Morey-Chaisemartin (2):
  pull: fix cli and config option parsing order
  pull: honor submodule.recurse config option

 builtin/pull.c            |  8 ++++++--
 t/t5572-pull-submodule.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

-- 
2.14.1.461.g503560879

[PATCHv3 1/2] pull: fix cli and config option parsing order

[Nicolas Morey-Chaisemartin]

pull parses first the cli options and then the config option.
The expected behavior is the other way around, so that config
options can not override the cli ones.

This patch changes the parsing order so config options are
parsed first.

Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com>
---
 builtin/pull.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 7fe281414..9ef1ab501 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -815,6 +815,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (!getenv("GIT_REFLOG_ACTION"))
 		set_reflog_message(argc, argv);
 
+	git_config(git_pull_config, NULL);
+
 	argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
 	parse_repo_refspecs(argc, argv, &repo, &refspecs);
@@ -825,8 +827,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (opt_rebase < 0)
 		opt_rebase = config_get_rebase();
 
-	git_config(git_pull_config, NULL);
-
 	if (read_cache_unmerged())
 		die_resolve_conflict("pull");
 
-- 
2.14.1.461.g503560879

[PATCHv3 2/2] pull: honor submodule.recurse config option

[Nicolas Morey-Chaisemartin]

"git pull" supports a --recurse-submodules option but does not parse the
submodule.recurse configuration item to set the default for that option.
Meanwhile "git fetch" does support submodule.recurse, producing
confusing behavior: when submodule.recurse is enabled, "git pull"
recursively fetches submodules but does not update them after fetch.

Handle submodule.recurse in "git pull" to fix this.

Reported-by: Magnus Homann <magnus@homann.se>
Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com>
---
 builtin/pull.c            |  4 ++++
 t/t5572-pull-submodule.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 9ef1ab501..6f772e8a2 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -325,6 +325,10 @@ static int git_pull_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "rebase.autostash")) {
 		config_autostash = git_config_bool(var, value);
 		return 0;
+	} else if (!strcmp(var, "submodule.recurse")) {
+		recurse_submodules = git_config_bool(var, value) ?
+			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+		return 0;
 	}
 	return git_default_config(var, value, cb);
 }
diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
index 077eb07e1..321bd37de 100755
--- a/t/t5572-pull-submodule.sh
+++ b/t/t5572-pull-submodule.sh
@@ -65,6 +65,38 @@ test_expect_success 'recursive pull updates working tree' '
 	test_path_is_file super/sub/merge_strategy.t
 '
 
+test_expect_success "submodule.recurse option triggers recursive pull" '
+	test_commit -C child merge_strategy_2 &&
+	git -C parent submodule update --remote &&
+	git -C parent add sub &&
+	git -C parent commit -m "update submodule" &&
+
+	git -C super -c submodule.recurse pull --no-rebase &&
+	test_path_is_file super/sub/merge_strategy_2.t
+'
+
+test_expect_success " --[no-]recurse-submodule and submodule.recurse" '
+	test_commit -C child merge_strategy_3 &&
+	git -C parent submodule update --remote &&
+	git -C parent add sub &&
+	git -C parent commit -m "update submodule" &&
+
+	git -C super -c submodule.recurse pull --no-recurse-submodules --no-rebase &&
+	test_path_is_missing super/sub/merge_strategy_3.t &&
+	git -C super -c submodule.recurse=false pull --recurse-submodules --no-rebase &&
+	test_path_is_file super/sub/merge_strategy_3.t &&
+
+	test_commit -C child merge_strategy_4 &&
+	git -C parent submodule update --remote &&
+	git -C parent add sub &&
+	git -C parent commit -m "update submodule" &&
+
+	git -C super -c submodule.recurse=false pull --no-recurse-submodules --no-rebase &&
+	test_path_is_missing super/sub/merge_strategy_4.t &&
+	git -C super -c submodule.recurse=true pull --recurse-submodules --no-rebase &&
+	test_path_is_file super/sub/merge_strategy_4.t
+'
+
 test_expect_success 'recursive rebasing pull' '
 	# change upstream
 	test_commit -C child rebase_strategy &&
-- 
2.14.1.461.g503560879