Patrick Steinhardt <ps@xxxxxx> 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.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> builtin/fetch.c | 2 ++
> t/t5526-fetch-submodules.sh | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index c310d89878..08d7fc7233 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1876,6 +1876,8 @@ static void add_options_to_argv(struct strvec *argv)
> strvec_push(argv, "--keep");
> if (recurse_submodules == RECURSE_SUBMODULES_ON)
> strvec_push(argv, "--recurse-submodules");
> + else if (recurse_submodules == RECURSE_SUBMODULES_OFF)
> + strvec_push(argv, "--no-recurse-submodules");
> else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
> strvec_push(argv, "--recurse-submodules=on-demand");
> if (tags == TAGS_SET)
Makes sense.
I wondered for a bit whether this should have been checking
recurse_submodules_cli (the actual CLI flag) back in 386c076a86 (fetch
--negotiate-only: do not update submodules, 2022-01-18). I think this
current state is correct, though. After we have told the superproject
what submodule recursion mode to use, we want to continue using that
mode when recursing through submodules regardless of whether that mode
originally came from the CLI or config.
> +test_expect_success "fetch --all with --no-recurse-submodules only fetches superproject" '
> + test_when_finished "git -C downstream remote remove second" &&
> +
> + # We need to add a second remote, otherwise --all falls back to the
> + # normal fetch-one case.
> + git -C downstream remote add second .. &&
> + git -C downstream fetch --all &&
> +
> + add_submodule_commits &&
> + add_superproject_commits &&
> + old_commit=$(git rev-parse --short HEAD~) &&
> + new_commit=$(git rev-parse --short HEAD) &&
> +
> + git -C downstream fetch --all --no-recurse-submodules >actual.out 2>actual.err &&
> +
> + cat >expect.out <<-EOF &&
> + Fetching origin
> + Fetching second
> + EOF
> +
> + cat >expect.err <<-EOF &&
> + From $(test-tool path-utils real_path .)/.
> + $old_commit..$new_commit super -> origin/super
> + From ..
> + $old_commit..$new_commit super -> second/super
> + EOF
> +
> + test_cmp expect.out actual.out &&
> + test_cmp expect.err actual.err
> +'
The test looks okay, though is there a reason you didn't copy the style
of the previous test? It is nearly exactly what you want, I think, like
(untested)
test_expect_success "fetch --all with --no-recurse-submodules only fetches superproject" '
test_when_finished "rm -fr src_clone" &&
git clone --recurse-submodules src src_clone &&
(
cd src_clone &&
git remote add secondary ../src &&
git config submodule.recurse true &&
git config fetch.parallel 0 &&
git fetch --all 2>../fetch-log
) &&
grep "Fetching submodule" fetch-log >fetch-subs &&
test_must_be_empty fetch-subs
'
which has the handy benefit of not needing the test-tools invocation.
