Subject: Re: [PATCH v4 1/8] fetch: fix `--no-recurse-submodules` with multi-remote fetches

From: Junio C Hamano <[email-masked]>
Date: Tue, 09 May 2023 10:49:47 -0700

Patrick Steinhardt <[email-masked]> writes:

> When running `git fetch --no-recurse-submodules`, the exectation is that
> we don't fetch any submodules. And while this works for fetches of a
> single remote, it doesn't when fetching multiple remotes at once. The
> result is that we do recurse into submodules even though the user has
> explicitly asked us not to.
>
> This is because while we pass on `--recurse-submodules={yes,on-demand}`
> if specified by the user, we don't pass on `--no-recurse-submodules` to
> the subprocess spawned to perform the submodule fetch.
>
> Fix this by also forwarding this flag as expected.

Makes sense.

> +test_expect_success "fetch --all with --no-recurse-submodules only fetches superproject" '
> +	test_when_finished "rm -rf src_clone" &&
> +
> +	git clone --recurse-submodules src src_clone &&
> +	(
> +		cd src_clone &&
> +		git remote add secondary ../src &&
> +		git config submodule.recurse true &&

The above two is essential to this test; we are interested in making
sure that --no-recurse-submodules is propagated down even when the
"--all" option is used, and we want another remote for that.  We set
the default to recurse, so that passing "--no-recurse-submodules"
would defeat it, but just refraining to pass "--recurse-submodules"
would cause us to recurse.

> +		git config fetch.parallel 0 &&

Is this necessary for the purpose of the test, though?  It should
not hurt, but we do not require the end-users to set it in real life
for the parallel fetching to work, either, right?

Just being curious.

> +		git fetch --all --no-recurse-submodules 2>../actual
> +	) &&
> +
> +	cat >expect <<-EOF &&
> +	From ../src
> +	 * [new branch]      master     -> secondary/master
> +	EOF
> +	test_cmp expect actual
> +'

In the context of a series that attempts to introduce a new stable
output format for machine consumption, which implies the traditional
output can change to match human users' preference, this test feels
a bit brittle, but let's wait until the end of the series to judge
that.

Looking good.  Thanks.

>  test_done