Hi brian,
Le 2021-06-19 à 17:44, brian m. carlson a écrit :
When the user recursively clones a repository with submodules and one or
more of those submodules is marked with the submodule.<name>.update=none
configuration, the submodule will end up being active. This is a
problem because we will have skipped cloning or checking out the
submodule, and as a result, other commands, such as git reset or git
checkout, will fail if they are invoked with --recurse-submodules (or
when submodule.recurse is true).
This is obviously not the behavior the user wanted, so let's fix this by
specifically setting the submodule as inactive in this case when we're
cloning the repository.
I'm not sure we want to fix it only at (recursive) clone time. The original bug report
was with 'git clone --recurse-submodules', but a non-recursive clone
followed by the usual 'git submodule init && git submodule update' (or
in one step 'git submodule update --init') would also lead to the same
bad experience (I'm not sure I want to call it a bug per se... it's certainly
a UX bug :)
That will make us properly ignore the submodule
when performing recursive operations.
Note that we only do this when the --require-init option is passed,
which is only passed during clone. That's because the update-clone
submodule helper is also invoked during a user-invoked git submodule
update, where we explicitly indicate that we override the configuration
setting, so we don't want to set such a configuration option then.
I'm not sure what you mean here by 'where we explicitely indicate that we
override the configuration setting'. For me, as I wrote above,
'git clone --recurse-submodules' and 'git clone' followed by
'git submodule update --init' should lead to mostly [*] the same end result.
If you mean 'git submodule update --checkout', that indeed seems to sometimes override the 'update=none'
configuration (a fact which is absent from the documentation), then it's true that we
would not want to write 'active=false' at that invocation. As an aside, in my limited testing
I could not always get 'git submodule update --checkout' to clone and checkout 'update=none' submdules;
it would fail with "fatal: could not get a repository handle for submodule 'sub1'" because
'git checkout/reset --recurse-submodules' leaves a bad .git/modules/sub1/config file
with the core.worktree setting when the command fails (this should not happen)...
In any case, that leads me to think that maybe the right place to write the 'active' setting
would be during 'git submodule init', thus builtin/submodule--helper.c::init_submodule ?
This way it would lead to the same behaviour if the clone was recursive or not,
and it would not interfere with 'git submodule update --checkout'.
[*] I say 'mostly' because in the first case you end up with a 'submodule.active=.'
configuration, and no 'submodule.$name.active' configuration for each submodule,
whereas in the second case, there is no 'submodule.active' setting and
'submodule.$name.active' is true for each submodule.
Reported-by: Rose Kunkel <rose@xxxxxxxxxxxxx>
Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
---
builtin/submodule--helper.c | 5 +++++
t/t5601-clone.sh | 24 ++++++++++++++++++++++++
2 files changed, 29 insertions(+)
I think that such a change would warrant a mention in the doc, in
gitsubmodules [1], git-config [2], and probably git-submodule [3] if we agree that
'git submodule init' is the right place to set the 'active=false' flag.
Thanks for proposing a patch!
Philippe.
[1] https://git-scm.com/docs/gitmodules#Documentation/gitmodules.txt-submoduleltnamegtupdate
[2] https://git-scm.com/docs/git-config#Documentation/git-config.txt-submoduleltnamegtupdate
[3] https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-init--ltpathgt82308203
