Re: [PATCH] pull: only pass '--recurse-submodules' to subcommands
- From
Glen Choo <chooglen@google.com>
- Date
- May 10, 2022, 18:27 UTC
- Message-ID
- <kl6l35hh8cqe.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <3234941c-5190-819f-fe3a-f528942c6b44@gmail.com>
Thanks Philippe and Junio, both!
Philippe Blain <levraiphilippeblain@gmail.com> writes:
Show 40 quoted lines
>> if (cleanup_arg) >> /* >> * this only checks the validity of cleanup_arg; we don't need >> 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 >> +' >> + > > OK so here we test the following situation: > > fetch.recurseSubmodules submodule.recurse --recurse-submodules fetch result pull result > ----------------------- ----------------- -------------------- ------------ ------------ > true unset absent recursive not recursive > > I think this is a good start, but since the default behaviour > of Git is the following: > > fetch.recurseSubmodules submodule.recurse --recurse-submodules fetch result pull result > ----------------------- ----------------- -------------------- ------------ ------------ > default: on-demand default: false unset on-demand not recursive > > we end up testing something not very far from the default behaviour.
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 :)