ci: rename Checkout third-party submodules step -> Checkout mrbind submodules#5991
Closed
ci: rename Checkout third-party submodules step -> Checkout mrbind submodules#5991
Conversation
…odules"
The step exists in two workflows -- build-test-macos.yml and
build-test-windows.yml -- which already use `submodules: true` on the
parent `actions/checkout`, then run a targeted recursive submodule
update for `thirdparty/mrbind` only. The previous step name suggested
a broader scope; the new name reflects what the step actually does
(of the 29 top-level submodules under thirdparty/, only mrbind has
nested sub-submodules -- specifically thirdparty/mrbind/deps/cppdecl).
Also expand the inline comment from a one-liner to four lines that
spell out:
- what the parent checkout already did,
- which submodule has nested sub-submodules and why we recurse only
there,
- the perf reason we don't `submodules: recursive` on the parent.
The other six workflows that have a `Checkout third-party submodules`
step (build-test-emscripten, build-test-linux-vcpkg,
build-test-ubuntu-{arm64,x64}, pip-build, update-docs-manual) do NOT
set `submodules: true` on their parent checkout. Their step's longer
submodule list is doing the actual top-level init for selected
submodules and cannot be simplified without breaking those builds.
Left untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Earlier commit on this branch renamed and simplified the second
checkout step on macos and windows workflows where the parent
`actions/checkout` already had `submodules: true`. The other six
workflows -- build-test-emscripten, build-test-linux-vcpkg,
build-test-ubuntu-{arm64,x64}, pip-build, update-docs-manual --
deliberately fetched only a selective list of submodules without
setting `submodules: true` on the parent.
This commit aligns those six workflows with the macos/windows pattern:
- Add `submodules: true` to the parent `actions/checkout`. With
`actions/checkout@v6`'s default `fetch-depth: 1`, the action
shallow-clones every top-level submodule (29 in MeshLib's
.gitmodules), at depth 1.
- Simplify the next step to a single `git submodule update --init
--recursive --depth 1 thirdparty/mrbind` -- only thirdparty/mrbind
has its own .gitmodules (deps/cppdecl), so that's the only path
that needs targeted recursion.
- Rename the step to "Checkout mrbind submodules".
- Drop the per-submodule `safe.directory` lines on pip-build that
only existed for the previous selective-list approach.
Trade-off: the parent checkout now shallow-clones ~24 extra submodules
on each of these legs (the ones the selective list deliberately
skipped, mostly heavy ones like openvdb, GDCM, libE57Format, mbedtls
that the docker image provides pre-built). At depth 1 over the parent
repo's fast remote, this adds an estimated 20-50 s per leg. Worth it
for a single uniform pattern across every workflow that uses this
step.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Direction reversed — replaced by a fresh PR that takes the opposite approach (selective list across all 8 workflows, no |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make every workflow that runs the
Checkout … submodulestwo-step pattern look the same:Checkoutdoessubmodules: true(shallow-cloning all 29 top-level submodules at the parent'sfetch-depth: 1),thirdparty/mrbind's nested sub-submodule (deps/cppdecl) — the only path underthirdparty/that has its own.gitmodules.Why
Of the 29 top-level submodules in MeshLib, only
thirdparty/mrbindhas nested sub-submodules. So the second step is genuinely only about mrbind on every workflow — but the previous step name ("Checkout third-party submodules") suggested a broader scope, and most workflows had a long selective list of submodules in thegit submodule updateline that hid the actual intent and caused drift between workflows.Files changed (8 workflows)
build-test-macos.ymlsubmodules: true; submodule list already onlythirdparty/mrbind)build-test-windows.ymlbuild-test-emscripten.ymlsubmodules: trueon parent; submodule list trimmed to justthirdparty/mrbind; step renamedbuild-test-linux-vcpkg.ymlbuild-test-ubuntu-arm64.ymlbuild-test-ubuntu-x64.ymlpip-build.ymlsafe.directorylines that only existed for the previous selective-list approachupdate-docs-manual.ymlTrade-off
Six of the eight workflows previously fetched only a 4-6 submodule selective list (no
submodules: trueon parent). Addingsubmodules: truemakes the parent shallow-clone all 29 top-level submodules at depth 1 — the heavy ones (openvdb, GDCM, libE57Format, mbedtls, onetbb, opencascade-related) are now fetched even though the docker image already provides them pre-built. With depth 1 over GitHub's fast remote this adds an estimated 20–50 s per matrix leg, and is paid in parallel with other parts of the checkout — small relative to multi-minute build times.In exchange, the workflow code becomes much easier to reason about: every "Checkout / Checkout mrbind submodules" pair across the repo now does the same thing.
Net diff
+69 / −32 across 8 files, two commits:
b7e8069c— macos + windows rename only161b3cfa— extend pattern to the other six workflows🤖 Generated with Claude Code