pull: only pass '--recurse-submodules' to subcommands

From: Glen Choo <[email protected]>

Fix a bug in "git pull" where submodule.recurse is preferred over fetch.recurseSubmodules (Documentation/config/fetch.txt says that fetch.recurseSubmodules should be preferred.). Do this by passing the value of the "--recurse-submodules" CLI option to the underlying fetch, instead of passing a value that combines the CLI option and config variables.

In other words, this bug occurred because builtin/pull.c is conflating two similar-sounding, but different concepts:

- Whether "git pull" itself should care about submodules e.g. whether it should update the submodule worktrees after performing a merge.
- The value of "--recurse-submodules" to pass to the underlying "git fetch".

Thus, when submodule.recurse is set, the underlying "git fetch" gets invoked with "--recurse-submodules", overriding the value of fetch.recurseSubmodules.

An alternative (and more obvious) approach to fix the bug would be to teach "git pull" to understand fetch.recurseSubmodules, but the proposed solution works better because:

- We don't maintain two identical config-parsing implementions in "git pull" and "git fetch".
- It works better with other commands invoked by "git pull" e.g. "git merge" won't accidentally respect fetch.recurseSubmodules.

---
pull: only pass '--recurse-submodules' to subcommands

Thanks Huang Zou for the report [1], and Philippe Blain for the initial investigation.

This patch fixes the original bug, but not in the 'obvious' way of teaching "git pull" to parse fetch.recurseSubmodules. Instead, "git pull" now propagates its value of "--recurse-submodules" to "git fetch" (ignoring any config values), and leaves the config parsing to "git fetch".

I think this works better because we get a nice separation of "config that git pull cares about" and "config that its subprocess care about", and as a result:

* We don't maintain two identical config-parsing implementations in "git pull" and "git fetch".
* It works better with other commands invoked by "git pull" e.g. "git merge" won't accidentally respect fetch.recurseSubmodules.

PS I'm having a hard time writing today, let me know how the commit message/cover letter can be improved :)

[1]
https://lore.kernel.org/git/CAFnZ=JNE_Sa3TsKghBPj1d0cz3kc6o91Ogj-op8o6qK8t9hPgg@mail.gmail.com

In-Reply-To:
CAFnZ=JNE_Sa3TsKghBPj1d0cz3kc6o91Ogj-op8o6qK8t9hPgg@mail.gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1262%2Fchooglen%2Fpull%2Ffetch-recurse-submodules-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1262/chooglen/pull/fetch-recurse-submodules-v1
Pull-Request: https://github.com/git/git/pull/1262

 builtin/pull.c            | 10 +++++++---
 t/t5572-pull-submodule.sh | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

index 4d667abc19d..01155ba67b2 100644
@@ -72,6 +72,7 @@ static const char * const pull_usage[] = {
 static int opt_verbosity;
 static char *opt_progress;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
 
 /* Options passed to git-merge or git-rebase */
 static enum rebase_type opt_rebase = -1;

@@ -120,7 +121,7 @@ static struct option pull_options[] = {
 		N_("force progress reporting"),
 		PARSE_OPT_NOARG),
 	OPT_CALLBACK_F(0, "recurse-submodules",
-		   &recurse_submodules, N_("on-demand"),
+		   &recurse_submodules_cli, N_("on-demand"),
 		   N_("control for recursive fetching of submodules"),
 		   PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
 

@@ -536,8 +537,8 @@ static int run_fetch(const char *repo, const char **refspecs)
 		strvec_push(&args, opt_tags);
 	if (opt_prune)
 		strvec_push(&args, opt_prune);
-	if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
-		switch (recurse_submodules) {
+	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
+		switch (recurse_submodules_cli) {
 		case RECURSE_SUBMODULES_ON:
 			strvec_push(&args, "--recurse-submodules=on");
 			break;

@@ -1001,6 +1002,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
+	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
+		recurse_submodules = recurse_submodules_cli;
+
 	if (cleanup_arg)
 		/*
 		 * this only checks the validity of cleanup_arg; we don't need

index fa6b4cca65c..65aaa7927fb 100755
@@ -107,6 +107,20 @@ test_expect_success " --[no-]recurse-submodule and submodule.recurse" '
 	test_path_is_file super/sub/merge_strategy_4.t
 '
 
+test_expect_success "fetch.recurseSubmodules option triggers recursive fetch (but not recursive update)" '
+	test_commit -C child merge_strategy_5 &&
+	git -C parent submodule update --remote &&
+	git -C parent add sub &&
+	git -C parent commit -m "update submodule" &&
+
+	git -C super -c fetch.recursesubmodules=true pull --no-rebase &&
+	# Check that the submodule commit was fetched
+	sub_oid=$(git -C super rev-parse FETCH_HEAD:sub) &&
+	git -C super/sub cat-file -e $sub_oid &&
+	# Check that the submodule worktree did not update
+	! test_path_is_file super/sub/merge_strategy_5.t
+'
+
 test_expect_success 'pull --rebase --recurse-submodules (remote superproject submodule changes, local submodule changes)' '
 	# This tests the following scenario :
 	# - local submodule has new commits

base-commit: e8005e4871f130c4e402ddca2032c111252f070a

gitgitgadget

OK. So the sub-git that is run in submodules will always see a command line option, and what its configuration file says does not matter?

OK.

Thanks, will queue.

Do we know if this ever worked correctly or it once used to work but we broke it?

The reason I ask is because applying this patch to maint-2.35 and then reverting the builtin/pull.c part still allow the new test in t5572 this patch introduces to pass. So either it used to work without the change to builtin/pull.c back in v2.35.x days (but in a newer codebase we need the change to builtin/pull.c), or the new test is not testing the alleged breakage at all.

It seems that applying only the test part to 'master' does not detect any test failure, so perhaps the test is faulty?

Hi Glen,

Le 2022-05-09 à 19:27, Glen Choo via GitGitGadget a écrit :

First, regarding the commit message title (I never know where to comment on that since I can't quote it :P)

I understand the intent, i.e. "only pass the CLI flag, not any config, to subcommands" but only because I already know what the patch is about. It could be read to mean "pass --recurse-submodules only to subcommands, and not to something else". Since this is really a bug that affects the underlying 'git fetch', maybe something like this ?

pull: pass '--recurse-submodules' to 'fetch' from CLI, not config

here I would add "for the underlying 'git fetch'"

(Documentation/config/fetch.txt says that

nit: "or rebase".

the wording is a litlle bit misleading here, as submodule.recurse could
be set to 'false', and then 'git fetch' will be invoked with '--recurse-submodules=false'.

I'm not sure of the meaning of the second bullet, since "git merge" should never perform a fetch ?...

fetch.recurseSubmodules  submodule.recurse  --recurse-submodules  fetch result  pull result
-----------------------  -----------------  --------------------  ------------  ------------
true                     unset              absent                recursive     not recursive

fetch.recurseSubmodules  submodule.recurse  --recurse-submodules  fetch result  pull result
-----------------------  -----------------  --------------------  ------------  ------------
default: on-demand       default: false     unset                 on-demand     not recursive

fetch.recurseSubmodules  submodule.recurse  --recurse-submodules  fetch result   pull result
-----------------------  -----------------  --------------------  -------------  ------------
false                    true                absent               not recursive*  recursive

fetch.recurseSubmodules  submodule.recurse  --recurse-submodules  fetch result   pull result
-----------------------  -----------------  --------------------  -------------  ------------
on-demand                false               absent               on-demand      recursive

fetch.recurseSubmodules  submodule.recurse  --recurse-submodules  fetch result   pull result
-----------------------  -----------------  --------------------  -------------  ------------
on-demand                true               absent                on-demand      recursive

Thanks Philippe and Junio, both!

Yes, I made a silly mistake with the test.. (I could've sworn it made sense at the time).

- fetch.recurseSubmodules=true always worked correctly when submodule.recurse is not set because "git pull" realizes that the behavior is unspecified, so "git fetch" doesn't receive a "--recurse-submodules" and does the right thing.

For completeness, we probably still want this test (even though it has always passed).

- I didn't test the actual buggy behavior, which is the precedence of the two config values.

This would be a new test that tests something like fetch.recurseSubmodules=foo and submodule.recurse=bar. This should be enough to test precedence; I don't think we need to test more combinations of values.

I hope you both didn't waste too much time on this :)

Thanks for the wording suggestions :) I'll see what I can incorporate.

Ah, I'm describing a hypothetical issue with the 'obvious' (aka a literal reading of the docs) approach of teaching "git pull" to handle fetch.recurseSubmodules.

You are correct, "git merge" should never perform a fetch, so it shouldn't care about fetch.recurseSubmodules. But a careless 'fix' might be to copy what "git fetch" does with its config, e.g.

	if (!strcmp(k, "submodule.recurse")) {
		int r = git_config_bool(k, v) ?
			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
		recurse_submodules = r;
	} else if (!strcmp(k, "fetch.recursesubmodules")) {
		recurse_submodules = parse_fetch_recurse_submodules_arg(k, v);
		return 0;
	}

which might make the internal "git merge" suddenly recurse into submodules. Of course, this can be fixed by using a fetch-specific variable, like:

	if (!strcmp(k, "submodule.recurse")) {
		int r = git_config_bool(k, v) ?
			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
		recurse_submodules = r;
	}
  if (!strcmp(k, "fetch.recursesubmodules")) {
		fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(k, v);
		return 0;
	}

  static int run_fetch(const char *repo, const char **refspecs)
  {
    /* ... */
    if (fetch_recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
       /* This is actually wrong wrt the docs, but assume that we could
       combine the two values here correctly. */
       recurse_submodules = fetch_recurse_submodules;
    if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
      switch (recurse_submodules) {
  }

but then we'd have to add more variables if we have merge.recurseSubmodules, or rebase.recurseSubmodules etc.

So the easiest way is to just tell "git pull" to stop assuming that it knows what the subcommands want :)

From: Glen Choo <[email protected]>

Fix a bug in "git pull" where submodule.recurse is preferred over fetch.recurseSubmodules when performing a fetch (Documentation/config/fetch.txt says that fetch.recurseSubmodules should be preferred.). Do this by passing the value of the "--recurse-submodules" CLI option to the underlying fetch, instead of passing a value that combines the CLI option and config variables.

In other words, this bug occurred because builtin/pull.c is conflating two similar-sounding, but different concepts:

- Whether "git pull" itself should care about submodules e.g. whether it should update the submodule worktrees after performing a merge.
- The value of "--recurse-submodules" to pass to the underlying "git fetch".

Thus, when submodule.recurse is set, the underlying "git fetch" gets invoked with "--recurse-submodules[=value]", overriding the value of fetch.recurseSubmodules.

An alternative (and more obvious) approach to fix the bug would be to teach "git pull" to understand fetch.recurseSubmodules, but the proposed solution works better because:

- We don't maintain two identical config-parsing implementions in "git pull" and "git fetch".
- It works better with other commands invoked by "git pull" e.g. "git merge" won't accidentally respect fetch.recurseSubmodules.

---
pull: only pass '--recurse-submodules' to subcommands

Thanks for the debugging help :)

Changes since v1:

* add a test that actually tests the precedence of the config values
* I've kept the previous test; it has always worked, but it still seems like a useful smoke test
* reworded the commit message slightly

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1262%2Fchooglen%2Fpull%2Ffetch-recurse-submodules-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1262/chooglen/pull/fetch-recurse-submodules-v2
Pull-Request: https://github.com/git/git/pull/1262

Range-diff vs v1:

1: caad7092826 ! 1: ba08e10b759 pull: only pass '--recurse-submodules' to subcommands
@@ Commit message
pull: only pass '--recurse-submodules' to subcommands

Fix a bug in "git pull" where submodule.recurse is preferred over
- fetch.recurseSubmodules (Documentation/config/fetch.txt says that
- fetch.recurseSubmodules should be preferred.). Do this by passing the
- value of the "--recurse-submodules" CLI option to the underlying fetch,
- instead of passing a value that combines the CLI option and config
- variables.
+ fetch.recurseSubmodules when performing a fetch
+ (Documentation/config/fetch.txt says that fetch.recurseSubmodules
+ should be preferred.). Do this by passing the value of the
+ "--recurse-submodules" CLI option to the underlying fetch, instead of
+ passing a value that combines the CLI option and config variables.

In other words, this bug occurred because builtin/pull.c is conflating two similar-sounding, but different concepts:
@@ Commit message
fetch".

Thus, when submodule.recurse is set, the underlying "git fetch" gets
- invoked with "--recurse-submodules", overriding the value of
+ invoked with "--recurse-submodules[=value]", overriding the value of
fetch.recurseSubmodules.

An alternative (and more obvious) approach to fix the bug would be to
@@ t/t5572-pull-submodule.sh: test_expect_success " --[no-]recurse-submodule and su

+test_expect_success "fetch.recurseSubmodules option triggers recursive fetch (but not recursive update)" '
+ test_commit -C child merge_strategy_5 &&
-+ git -C parent submodule update --remote &&
-+ git -C parent add sub &&
-+ git -C parent commit -m "update submodule" &&
++ # Omit the parent commit, otherwise this passes with the
++ # default "pull" behavior.
+
+ git -C super -c fetch.recursesubmodules=true pull --no-rebase &&
+ # Check that the submodule commit was fetched
-+ sub_oid=$(git -C super rev-parse FETCH_HEAD:sub) &&
++ sub_oid=$(git -C child rev-parse HEAD) &&
+ git -C super/sub cat-file -e $sub_oid &&
+ # Check that the submodule worktree did not update
+ ! test_path_is_file super/sub/merge_strategy_5.t
+'
++
++test_expect_success "fetch.recurseSubmodules takes precedence over submodule.recurse" '
++ test_commit -C child merge_strategy_6 &&
++ # Omit the parent commit, otherwise this passes with the
++ # default "pull" behavior.
++
++ git -C super -c submodule.recurse=false -c fetch.recursesubmodules=true pull --no-rebase &&
++ # Check that the submodule commit was fetched
++ sub_oid=$(git -C child rev-parse HEAD) &&
++ git -C super/sub cat-file -e $sub_oid &&
++ # Check that the submodule worktree did not update
++ ! test_path_is_file super/sub/merge_strategy_6.t
++'
+
test_expect_success 'pull --rebase --recurse-submodules (remote superproject submodule changes, local submodule changes)' '
# This tests the following scenario :

 builtin/pull.c            | 10 +++++++---
 t/t5572-pull-submodule.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

index 4d667abc19d..01155ba67b2 100644
@@ -72,6 +72,7 @@ static const char * const pull_usage[] = {
 static int opt_verbosity;
 static char *opt_progress;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
 
 /* Options passed to git-merge or git-rebase */
 static enum rebase_type opt_rebase = -1;

@@ -120,7 +121,7 @@ static struct option pull_options[] = {
 		N_("force progress reporting"),
 		PARSE_OPT_NOARG),
 	OPT_CALLBACK_F(0, "recurse-submodules",
-		   &recurse_submodules, N_("on-demand"),
+		   &recurse_submodules_cli, N_("on-demand"),
 		   N_("control for recursive fetching of submodules"),
 		   PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
 

@@ -536,8 +537,8 @@ static int run_fetch(const char *repo, const char **refspecs)
 		strvec_push(&args, opt_tags);
 	if (opt_prune)
 		strvec_push(&args, opt_prune);
-	if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
-		switch (recurse_submodules) {
+	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
+		switch (recurse_submodules_cli) {
 		case RECURSE_SUBMODULES_ON:
 			strvec_push(&args, "--recurse-submodules=on");
 			break;

@@ -1001,6 +1002,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
+	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
+		recurse_submodules = recurse_submodules_cli;
+
 	if (cleanup_arg)
 		/*
 		 * this only checks the validity of cleanup_arg; we don't need

index fa6b4cca65c..a35396fadf5 100755
@@ -107,6 +107,32 @@ test_expect_success " --[no-]recurse-submodule and submodule.recurse" '
 	test_path_is_file super/sub/merge_strategy_4.t
 '
 
+test_expect_success "fetch.recurseSubmodules option triggers recursive fetch (but not recursive update)" '
+	test_commit -C child merge_strategy_5 &&
+	# Omit the parent commit, otherwise this passes with the
+	# default "pull" behavior.
+
+	git -C super -c fetch.recursesubmodules=true pull --no-rebase &&
+	# Check that the submodule commit was fetched
+	sub_oid=$(git -C child rev-parse HEAD) &&
+	git -C super/sub cat-file -e $sub_oid &&
+	# Check that the submodule worktree did not update
+	! test_path_is_file super/sub/merge_strategy_5.t
+'
+
+test_expect_success "fetch.recurseSubmodules takes precedence over submodule.recurse" '
+	test_commit -C child merge_strategy_6 &&
+	# Omit the parent commit, otherwise this passes with the
+	# default "pull" behavior.
+
+	git -C super -c submodule.recurse=false -c fetch.recursesubmodules=true pull --no-rebase &&
+	# Check that the submodule commit was fetched
+	sub_oid=$(git -C child rev-parse HEAD) &&
+	git -C super/sub cat-file -e $sub_oid &&
+	# Check that the submodule worktree did not update
+	! test_path_is_file super/sub/merge_strategy_6.t
+'
+
 test_expect_success 'pull --rebase --recurse-submodules (remote superproject submodule changes, local submodule changes)' '
 	# This tests the following scenario :
 	# - local submodule has new commits

base-commit: e8005e4871f130c4e402ddca2032c111252f070a

gitgitgadget

Hi Glen,

Le 2022-05-10 à 15:25, Glen Choo via GitGitGadget a écrit :

Thanks, this version looks good to me. I don't feel to strongly about the title either, so as you wish :)

Philippe.

Perhaps

pull: do not let submodule.recurse override fetch.recurseSubmodules

more clearly expresses what it wants to do (not passing the command line option is "how" we achieve that goal)?

Le 2022-05-11 à 18:34, Junio C Hamano a écrit :

I like that title, yes.

Hm, yes that makes more sense. The commit message leads with the bug, so it's more consistent to mention the bug in the title too.

It's arguably more _correct_ to say that passing only the CLI option is the desired result, and we just so happen to be fixing a bug along the way. But meh, the bug is roughly 90% [1] of the motivation for making this patch, so leading with the bug is ok.

Will reroll.

[1] The other 10% or so is some upcoming work with git rebase --recurse-submodules and supporting that in git pull --rebase --recurse-submodules :)

Correct.

If it were clear that the title meant "teach 'git pull' to try affecting the recursive behaviour of subcommands only when it got '--recurse-submodules' on its command line", I wouldn't have waited for retitling.

It however is easy to misread the original title in such a way that the mention of '--recurse-submodules' refers to the fact that 'pull' invokes 'fetch' and passes the "--recurse-submodules" command line option, and "only" incorrectly hints that 'pull' passes only that option and no other option from the command line when it does so.

That will be puzzling because (1) that is clearly not what we do in today's code, suggesting this is a totally different change from what we have in the patch, and (2) it is not clear what we want to achieve by changing the code in such a drastic way to stop passing "--all" and other options to "fetch".

So, it is arguable, but the original title does not say what we wanted it to say, exactly.

From: Glen Choo <[email protected]>

Fix a bug in "git pull" where submodule.recurse is preferred over fetch.recurseSubmodules when performing a fetch (Documentation/config/fetch.txt says that fetch.recurseSubmodules should be preferred.). Do this by passing the value of the "--recurse-submodules" CLI option to the underlying fetch, instead of passing a value that combines the CLI option and config variables.

In other words, this bug occurred because builtin/pull.c is conflating two similar-sounding, but different concepts:

- Whether "git pull" itself should care about submodules e.g. whether it should update the submodule worktrees after performing a merge.
- The value of "--recurse-submodules" to pass to the underlying "git fetch".

Thus, when submodule.recurse is set, the underlying "git fetch" gets invoked with "--recurse-submodules[=value]", overriding the value of fetch.recurseSubmodules.

An alternative (and more obvious) approach to fix the bug would be to teach "git pull" to understand fetch.recurseSubmodules, but the proposed solution works better because:

- We don't maintain two identical config-parsing implementions in "git pull" and "git fetch".
- It works better with other commands invoked by "git pull" e.g. "git merge" won't accidentally respect fetch.recurseSubmodules.

---
pull: do not let submodule.recurse override fetch.recurseSubmodules

Thanks for the debugging help :)

Changes since v1:

* add a test that actually tests the precedence of the config values
* I've kept the previous test; it has always worked, but it still seems like a useful smoke test
* reworded the commit message slightly

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1262%2Fchooglen%2Fpull%2Ffetch-recurse-submodules-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1262/chooglen/pull/fetch-recurse-submodules-v3
Pull-Request: https://github.com/git/git/pull/1262

Range-diff vs v2:

1: ba08e10b759 ! 1: 07f052c377c pull: only pass '--recurse-submodules' to subcommands
@@ Metadata
Author: Glen Choo <[email protected]>

## Commit message ##
- pull: only pass '--recurse-submodules' to subcommands
+ pull: do not let submodule.recurse override fetch.recurseSubmodules

Fix a bug in "git pull" where submodule.recurse is preferred over fetch.recurseSubmodules when performing a fetch

 builtin/pull.c            | 10 +++++++---
 t/t5572-pull-submodule.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

index 4d667abc19d..01155ba67b2 100644
@@ -72,6 +72,7 @@ static const char * const pull_usage[] = {
 static int opt_verbosity;
 static char *opt_progress;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
 
 /* Options passed to git-merge or git-rebase */
 static enum rebase_type opt_rebase = -1;

@@ -120,7 +121,7 @@ static struct option pull_options[] = {
 		N_("force progress reporting"),
 		PARSE_OPT_NOARG),
 	OPT_CALLBACK_F(0, "recurse-submodules",
-		   &recurse_submodules, N_("on-demand"),
+		   &recurse_submodules_cli, N_("on-demand"),
 		   N_("control for recursive fetching of submodules"),
 		   PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
 

@@ -536,8 +537,8 @@ static int run_fetch(const char *repo, const char **refspecs)
 		strvec_push(&args, opt_tags);
 	if (opt_prune)
 		strvec_push(&args, opt_prune);
-	if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
-		switch (recurse_submodules) {
+	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
+		switch (recurse_submodules_cli) {
 		case RECURSE_SUBMODULES_ON:
 			strvec_push(&args, "--recurse-submodules=on");
 			break;

@@ -1001,6 +1002,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
+	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
+		recurse_submodules = recurse_submodules_cli;
+
 	if (cleanup_arg)
 		/*
 		 * this only checks the validity of cleanup_arg; we don't need

index fa6b4cca65c..a35396fadf5 100755
@@ -107,6 +107,32 @@ test_expect_success " --[no-]recurse-submodule and submodule.recurse" '
 	test_path_is_file super/sub/merge_strategy_4.t
 '
 
+test_expect_success "fetch.recurseSubmodules option triggers recursive fetch (but not recursive update)" '
+	test_commit -C child merge_strategy_5 &&
+	# Omit the parent commit, otherwise this passes with the
+	# default "pull" behavior.
+
+	git -C super -c fetch.recursesubmodules=true pull --no-rebase &&
+	# Check that the submodule commit was fetched
+	sub_oid=$(git -C child rev-parse HEAD) &&
+	git -C super/sub cat-file -e $sub_oid &&
+	# Check that the submodule worktree did not update
+	! test_path_is_file super/sub/merge_strategy_5.t
+'
+
+test_expect_success "fetch.recurseSubmodules takes precedence over submodule.recurse" '
+	test_commit -C child merge_strategy_6 &&
+	# Omit the parent commit, otherwise this passes with the
+	# default "pull" behavior.
+
+	git -C super -c submodule.recurse=false -c fetch.recursesubmodules=true pull --no-rebase &&
+	# Check that the submodule commit was fetched
+	sub_oid=$(git -C child rev-parse HEAD) &&
+	git -C super/sub cat-file -e $sub_oid &&
+	# Check that the submodule worktree did not update
+	! test_path_is_file super/sub/merge_strategy_6.t
+'
+
 test_expect_success 'pull --rebase --recurse-submodules (remote superproject submodule changes, local submodule changes)' '
 	# This tests the following scenario :
 	# - local submodule has new commits

base-commit: e8005e4871f130c4e402ddca2032c111252f070a

gitgitgadget

Let's see if there are any more inputs and start merging down to 'next'.

Thanks.

Hi Glen and Junio,

Le 2022-05-12 à 16:38, Junio C Hamano a écrit :

Well I just thought that since you just merged pb/submodule-recurse-mode-enum into 'next', maybe Glen could make use of the new enum in his new code, though that could also be a separate cleanup for later.

Philippe.