Re: The argument "--recurse-submodules=no" when doing "fetch --all" is not passed to the following commands which can ends up fetching submodules

From: Jeff King <peff@peff.net>
Date: Feb 9, 2021, 17:00 UTC
Message-ID: <YCK/yfrDQ3LXP3ec@coredump.intra.peff.net>
In-Reply-To: <DM5PR1701MB1724CCBB1AC5CF342BA9ADD5898E9@DM5PR1701MB1724.namprd17.prod.outlook.com>

On Tue, Feb 09, 2021 at 04:35:58PM +0000, Dominick Latreille wrote:

Show 14 quoted lines

> Here is the command I ran to trigger this issue
> ```
> GIT_TRACE2=1 git fetch --all --recurse-submodules=no
> ```
> 
> I expected the submodules to not be fetched and to see this command with traces activated
> ```
> git fetch --append --no-auto-gc --recurse-submodules=no --no-write-commit-graph origin
> ```
> 
> But, here is the command line I got with traces activated and the submodules were fetched later on.
> ```
> git fetch --append --no-auto-gc --no-write-commit-graph origin
> ```

The issue is that "fetch --all" runs fetch sub-processes (one for each remote), and tries to copy its arguments to them. But it does so by looking at what it parsed and trying to recreate the relevant set of original arguments.

And it looks like when this was added way back in 7dce19d374 (fetch/pull: Add the --recurse-submodules option, 2010-11-12), it was not anticipated that somebody would need to turn recursion _off_. :) Of course it makes perfect sense if somebody later switches the default to on-demand. :)

So the solution is probably:

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 91f3d20696..cb59f79157 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1660,6 +1660,8 @@ static void add_options_to_argv(struct strvec *argv)
 		strvec_push(argv, "--recurse-submodules");
 	else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
 		strvec_push(argv, "--recurse-submodules=on-demand");
+	else if (recurse_submodules == RECURSE_SUBMODULES_OFF)
+		strvec_push(argv, "--recurse-submodules=no");
 	if (tags == TAGS_SET)
 		strvec_push(argv, "--tags");
 	else if (tags == TAGS_UNSET)

but there are a bunch of other RECURSE_SUBMODULES_* values. I have no
idea if any of those should be covered, too.

-Peff