ci(multi-arch): selective per-PR builds via TheRock build_stages - #10920
Open
ammallya wants to merge 1 commit into
Open
ci(multi-arch): selective per-PR builds via TheRock build_stages#10920ammallya wants to merge 1 commit into
ammallya wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
github actions
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR speeds up rocm-systems multi-arch presubmit CI by computing a per-PR allowlist of TheRock build stages to build (and skipping all others) based on the changed projects and TheRock’s build topology.
Changes:
- Add
.github/scripts/compute_build_stages.pyto map changed projects → minimal required TheRock build stages (fail-safe to “build all” when narrowing is unsafe). - Wire
build_stagesthrough.github/workflows/therock-multi-arch-ci.ymlfrom the configure job into TheRock’ssetup_multi_arch.yml. - Add unit tests for the rocm-systems-owned selection/fail-safe logic.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/therock-multi-arch-ci.yml | Computes and forwards a per-PR build_stages allowlist into TheRock multi-arch setup. |
| .github/scripts/compute_build_stages.py | New script that computes the stage allowlist from changed projects using TheRock topology. |
| .github/scripts/tests/compute_build_stages_test.py | Unit tests covering parsing and fail-safe behaviors without requiring a TheRock checkout. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+103
to
106
| sparse-checkout: | | ||
| build_tools/github_actions | ||
| build_tools/_therock_utils | ||
| sparse-checkout-cone-mode: true |
Comment on lines
+102
to
+104
| therock_build_tools = Path(therock_path) / "build_tools" | ||
| sys.path.insert(0, os.fspath(therock_build_tools)) | ||
| try: |
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.
Motivation
The multi-arch CI (
therock-multi-arch-ci.yml) currently rebuilds every TheRockbuild stage on every rocm-systems PR, regardless of how small the change is. This is
because rocm-systems is represented as a single
source_setin TheRock'sBUILD_TOPOLOGY.toml, so any change maps to the full stage graph and nothing can benarrowed. The result is slow per-PR turnaround and a scaling bottleneck as more projects
land in the monorepo.
The pre-multi-arch CI only built the components a PR actually touched. This PR restores
that behavior for the multi-arch pipeline: it builds only the stages a PR needs and
skips the rest outright (no build, no artifact copy). This is a pure per-PR speed-up; it
does not change what a full/nightly build produces.
Technical Details
This builds on ROCm/TheRock#7600 ("feat(ci): Add
build_stagesinput to filter CIbuild graph"), now merged, which adds a
build_stagesallowlist input to TheRock'ssetup_multi_arch.yml. When set, TheRock builds only the listed stages and skips everyother stage outright; it also validates unknown stage names and auto-scopes test labels to
the allowlisted stages. rocm-systems just needs to compute and pass the allowlist.
Changes in this PR (rocm-systems only):
New
.github/scripts/compute_build_stages.py. Maps the changed projects (alreadyproduced by TheRock's
configure_external_repo_ci.py) to the minimal allowlist of buildstages required to build them, using TheRock's build topology as the single source of
truth (
BuildTopology.get_stages_for_projects, which walks the upstream build-dependencyclosure). The topology module is imported read-only from the TheRock checkout, so this
script owns no dependency math and cannot drift from the actual build graph.
therock-multi-arch-ci.ymlwiring.configurejob's repo sparse-checkout now also fetches the new script; the TheRocksparse-checkout is broadened to include
build_tools/_therock_utils(cone mode alsopulls the root
BUILD_TOPOLOGY.tomlandbuild_tools/project_mappings.json).build_stages.setupjob passesbuild_stages: ${{ needs.configure.outputs.build_stages }}tosetup_multi_arch.yml.Dependency / rollout. This PR does not change the pinned TheRock ref; the
build_stagesinput is consumed only once the pinned ref includes #7600, which is beingbumped in a separate PR. Until that lands,
build_stagesis an inert no-op (unknown inputsare ignored), so this change is safe to merge independently and activates automatically when
the ref is updated. Keep the configure-checkout ref and the
setup/multi_arch_ci_*refsin sync so the topology used for selection matches what is built.
Fail-safe (empty allowlist ⇒ build all). The script conservatively builds everything
whenever the change set cannot be safely narrowed:
run_all_teststrue — workflow/scripts/repos-config.json);clr,hip,hipother,hip-tests,rocr-runtime,amdsmi);get_stages_for_projectssilently ignores names it cannot resolve);Safety.
get_stages_for_projectsalways returns the full upstream closure, so nobuilt stage ever depends on a skipped stage. The trade-off while active: skipped stages
produce no artifacts, so downstream jobs see only the built subset. This is intentional and
temporary; see the tracking issue below.
Relevant links:
build_tools/_therock_utils/build_topology.py(
get_stages_for_projects),build_tools/project_mappings.jsonIssue Tracking
ISSUE ID: ROCm/rocm-libraries#11195
Test Plan
.github/scripts/tests/compute_build_stages_test.py) covering: project-pathnormalization, the
run_all_testsshort-circuit, fan-out policy, mixed known/unknownprojects, the "all stages required" case, and topology-load failure. The topology is
stubbed so the tests need no TheRock checkout.
compute_build_stages.pyagainst a real TheRock checkout pinned to the Bump tornado from 6.5.4 to 6.5.7 in /projects/rocminfo/docs/sphinx #7600 mergecommit for representative changed-project sets.
develop.Test Result
blackclean; workflow YAML valid.projects/rdc→compiler-runtime,dctools-coreprojects/rocshmem→comm-libs,compiler-runtimeprojects/rocprofiler-systems→compiler-runtime,profiler-appsprojects/hip(fan-out), unknown project, empty, andrun_all_tests=true→ empty(build all).
Submission Checklist