Repository navigation
feat(ci): build wsl-rocdxg when a PR has the ci:run-wsl label - #14472
amd-justchen wants to merge 1 commit into
Conversation
TheRock (ROCm/TheRock#8457) can also test the Linux build on WSL GPU runners when a PR has ci:run-wsl. Those tests need TheRock's wsl-rocdxg stage, and TheRock rejects ci:run-wsl when the build_stages allowlist omits it, so narrow PRs with the label would fail setup. compute_build_stages.py now takes the PR labels and, with ci:run-wsl, adds the wsl-rocdxg artifact before mapping artifacts to stages, so its upstream stages come along too. Full builds are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the opt-in WSL testing changes.
No open issues 🎉
What changed in this PR
Enables opt-in WSL testing for narrowly scoped PRs by including the required wsl-rocdxg build stages when ci:run-wsl is present. Full builds and PRs without the label remain unchanged.
Changes:
- Passes PR labels through an environment variable to the build-stage script.
- Adds the WSL artifact before resolving required stages and their dependencies.
- Adds tests for WSL stage inclusion and label handling.
| File | Description |
|---|---|
.github/workflows/therock-multi-arch-ci.yml |
Passes PR labels to build-stage computation. |
.github/scripts/tests/compute_build_stages_test.py |
Tests WSL stage inclusion and CLI label propagation. |
.github/scripts/compute_build_stages.py |
Parses labels and includes WSL stages for opted-in PRs. |
Review effort: Balanced
| # (ROCm/TheRock#8457). Those tests need the wsl-rocdxg artifact (the WSL | ||
| # libhsakmt), and TheRock rejects ci:run-wsl when the allowlist omits its stage. | ||
| WSL_RUN_LABEL = "ci:run-wsl" | ||
| WSL_ARTIFACT = "wsl-rocdxg" |
There was a problem hiding this comment.
This one-off hardcode handling I think should be fine? I can't think of another case where we'd want to do something like this again
If/once we have mature WSL support and enough runner capacity, would the eventual goal be to always build+test WSL for every PR? Or to keep having this opt in system? rocdxg builds fairly quickly (for now) so having it always build may not be so bad.
There was a problem hiding this comment.
Opt-in (xfail) for now; default-on once WSL results are green.
| - name: Compute build-stage allowlist | ||
| id: build_stages | ||
| env: | ||
| PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} |
There was a problem hiding this comment.
this will be removed soon (TheRock CI will handle this entirely). can we fix the label issue inside TheRock instead?
if the label is added to a rocm-systems PR, does WSL not run as expected?
There was a problem hiding this comment.
It runs today on full-build PRs (clr, hip, hip-tests, rocr-runtime, CI changes), e.g. this PR's run. On narrow PRs, TheRock's setup rejects ci:run-wsl because build_stages lacks wsl-rocdxg; that's the case this change covers.
There was a problem hiding this comment.
Makes sense, moved it into TheRock in ROCm/TheRock#9628: ci:run-wsl now adds the wsl-rocdxg stage instead of failing. I'll close this once the pin picks it up.
|
CI on this PR: run 40733409077 (the first run was cancelled by a bot-triggered rerun). With |
Note
Drafted with Claude (AI agent), run and reviewed by @amd-justchen.
Motivation
ROCm/TheRock#8457 added opt-in WSL GPU tests: with the
ci:run-wsllabel, hip-tests and rocrtst also run, non-blocking (xfail), on WSL2 V710 runners next to the native Linux jobs. rocm-systems picked this up with the TheRock bump in #14272.This makes the label usable on every rocm-systems PR, so HIP and ROCr developers can see and triage WSL results before WSL becomes blocking.
Technical Details
The WSL tests need TheRock's
wsl-rocdxgstage (the WSL libhsakmt), and TheRock rejectsci:run-wslwhen thebuild_stagesallowlist leaves it out. Full builds (clr,hip,hip-tests,rocr-runtime, CI changes, ...) already include it, but a narrow PR with the label would fail the setup job.compute_build_stages.pytakes--pr-labels. Withci:run-wslit adds thewsl-rocdxgartifact before mapping artifacts to stages, so the stage and its upstream stages are built. Full builds and PRs without the label are unchanged.therock-multi-arch-ci.ymlpasses the PR labels to it (throughenv, since label names come from the PR).ci:run-wsllabel now exists in this repo.To run WSL tests on a PR, add
ci:run-wslplusci:gfx110X-all(the WSL pool is gfx1101, and gfx110X is not in this repo's default Linux families).Issue Tracking
GitHub issue: https://github.com/ROCm/TheRock-Infra/issues/317
Test Plan
python -m pytest .github/scripts/tests/compute_build_stages_test.pycompute_build_stages.pyagainst the real topology at the pinned TheRockb063323.configure_multi_arch_ci.pyatb063323, fed this repo's PR inputs (familiesgfx94X,gfx950,gfx125X, labelsci:run-wsl,ci:gfx110X-all).ci:run-wsl,ci:gfx110X-all,test:hip-testsandtest:rocrtst, so its own CI shows the WSL jobs.Test Result
projects/rdc):compiler-runtime,dctools-corewithout the label;compiler-runtime,dctools-core,wsl-rocdxgwith it.projects/clrstays a full build.compiler-runtime,dctools-coreandci:run-wslit raisesci:run-wsl needs the wsl-rocdxg stage; with this change's allowlist it passes and gives gfx110X-all thewsl-gfx1101-gpu-rocmrunner.Test gfx110X-all (WSL)jobs.Submission Checklist