Re: [PATCH] pull: only pass '--recurse-submodules' to subcommands
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 10, 2022, 00:44 UTC
- Message-ID
- <xmqq7d6u9px0.fsf@gitster.g>
- In-Reply-To
- <pull.1262.git.git.1652138854255.gitgitgadget@gmail.com>
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 9 quoted lines
> From: Glen Choo <chooglen@google.com> > > 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. > ...
Do we know if this ever worked correctly or it once used to work but we broke it?
> builtin/pull.c | 10 +++++++--- > t/t5572-pull-submodule.sh | 14 ++++++++++++++ > 2 files changed, 21 insertions(+), 3 deletions(-)
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?
Show 27 quoted lines
> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh > index fa6b4cca65c..65aaa7927fb 100755 > --- a/t/t5572-pull-submodule.sh > +++ b/t/t5572-pull-submodule.sh > @@ -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