Repository navigation
Add git_submodules flag to control dependency submodule cloning - #314
Conversation
Co-Authored-By: Claude Opus 4.8 <[email protected]>
micprog
left a comment
micprog
left a comment
There was a problem hiding this comment.
I understand the frustration!
From separate conversations, we ideally add a configuration in the Bender.yml indicating which submodules are required, and which are not, on a per-dependency basis, possibly with additional target specification. In this instance, I like the approach to get at least something going. It would be great if we could have a command or option to force the checkout/update of all submodules in all dependencies, just in case we checked out without submodules and now need to get some/all. I think this is a good flag/subcommand for the checkout command, let me know what you think. To avoid breaking setups, I see an option to cleanly get all submodules as a prerequisite to this feature. Other than that, LGTM 👍
Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
I agree with the approach of specifying this on the side of the dependency, since the maintainer is more aware of which submodules are actually required. As discussed, the format would be something like: git_submodules:
- sw/deps/printf
- target: test
submodule: sw/deps/cva6-sdk
recursive: false # default: true
shallow: false # default: true?
# not updated if no specified explicitely
# - pd/deps/ihp-130-pdk With this functionality in place, it would make sense to reverse the default to not clone i.e. I would tackle this in a separate PR. In that sense, this PR is ready as it is and it also does not include any breaking changes since the default stays the same and can be released as a patch version. Then, the following modifications would be released as a breaking release. Let me know what you think! |
Cloning dependency submodules with `git submodule update --init --recursive` runs unconditionally on every checkout and is frequently the slowest part of fetching dependencies. The submodules often hold software or tooling that is irrelevant to the hardware build. Add a `git_submodules` config field (default true), mirroring the existing `git_lfs` option, plus a value-taking `--git-submodules <true|false>` CLI flag (env `BENDER_GIT_SUBMODULES`) modelled on `--git-throttle`. The flag overrides the configured value in either direction. When disabled, the submodule update is skipped during checkout. The flag takes a value rather than being a presence-only switch so the accepted values can later widen (e.g. to select specific dependencies) without a breaking change to the CLI surface. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Co-Authored-By: Claude Opus 4.8 <[email protected]>
micprog
left a comment
micprog
left a comment
There was a problem hiding this comment.
I added a warning to help out in getting the submodule back in case the flag was inadvertently added, but now LGTM 👍
LGTM! |
2 participants
What
Adds a
git_submodulesoption to control whether Bender clones the Git submodules of dependencies.git_submodules(defaulttrue), mirroring the existinggit_lfsoption — OR-merged across config layers, validated withunwrap_or(true).--git-submodules <true|false>(envBENDER_GIT_SUBMODULES), modelled on--git-throttle. It overrides the configured value in either direction.git submodule update --init --recursivestep is skipped during checkout (along with its per-submodule progress bar).Why
git submodule update --init --recursivecurrently runs unconditionally on every checkout that has a.gitmodulesfile, and it is frequently the slowest part of the initial dependency fetch. In top-level hardware projects, dependency submodules often hold software or tooling that is irrelevant to the build, so cloning them is wasted work. This gives users an escape hatch — persistently via config, or per-invocation via the flag (e.g. in CI).The default stays
true, so existing behavior is unchanged unless explicitly opted out.Notes
--no-progress). This keeps the door open to later widening the accepted values — e.g. selecting specific dependencies — without a breaking change to the CLI surface. The config field stays a plainboolfor now; no enum is introduced yet.book/src/configuration.md(config field reference) andbook/src/dependencies.md(Submodules section).🤖 Generated with Claude Code