Repository navigation
ci: publish Windows MSIs to release buckets + dispatch install test (2/2) - #9371
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
7db7436 to
4847342
Compare
ScottTodd
left a comment
There was a problem hiding this comment.
Code changes generally LGTM
- Please prepare (and ideally land before this) the corresponding changes to https://github.com/ROCm/rockrel/tree/main/.github/workflows
- The PR description is very difficult to read. Please completely rewrite it. Yourself. No AI please, this is out of control. See https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md#ai-tool-use-policy
- There are dozens of repeated links to the same pull requests
- The code is self-explanatory. Statements like
and thread quartz_tracking_id to both new edgesare not necessary. The PR description should highlight what a reviewer should be aware of, not exhaustively restate what is already in the code Part 2 of 2 of the Milestone 4 MSI wiring (#1987), stacked on #9370 (review this after #9370).is also self-evident from the stacked PR github feature. Not useful to include in a commit message (which the PR description will be)(verified: identical trees)not usefulr4289106318 and r4289108399 — "Omit comment. The code is self-explanatory."useless text for a PR description
| uses: benc-uk/workflow-dispatch@d004979be141727c7a17c28f603e2facc40d0855 # v1.3.3 | ||
| with: | ||
| workflow: test_native_windows_packages_install.yml |
There was a problem hiding this comment.
All use of benc-uk/workflow-dispatch in release workflows need stub workflows in the rockrel repository like https://github.com/ROCm/rockrel/blob/main/.github/workflows/test_native_linux_packages_install.yml. See ROCm/rockrel#49
Please land that before this change so we don't break nightly releases.
There was a problem hiding this comment.
stub workflow added in ROCm/rockrel#130, following the same pattern as test_native_linux_packages_install.yml.
Thin wrapper forwarding to TheRock's test_native_windows_packages_install.yml, matching the existing pattern for test_native_linux_packages_install.yml. Required before ROCm/TheRock#9371 merges: the release workflow dispatches this workflow from rockrel's context, so the file must exist here. Co-Authored-By: Claude <noreply@anthropic.com>
…pt-in (1/2) (#9370) ## Summary Part **1 of 2** splitting the Milestone 4 MSI wiring (#1987), per review feedback on #9025 to separate CI wiring from release wiring. This PR wires the native Windows MSI build into the **CI (nightly)** path and lands the shared pieces both CI and release need. The release-bucket publish + install-test dispatch follow in stacked PR #9371. ### CI wiring - `multi_arch_ci_windows.yml`: new `build_native_msi_packages` job, gated behind **`build_native_windows`** so CI in other repositories (not building all of ROCm) skips it. Builds the runtime MSI from this run's artifacts and uploads it to the run's artifacts bucket. No release-bucket publish here. - `build_native_windows` flag plumbed exactly like `build_native_linux`: input on `multi_arch_ci.yml`, exported by `setup_multi_arch.yml`, read into `CIInputs`/`BuildConfig` by `configure_multi_arch_ci.py` (windows-scoped, auto-disabled on partial builds), with PR-label overrides `ci:build-native-windows` / `ci:skip-native-windows` (`configure_ci_labels.py` + docs). ### Shared core (also needed by the release follow-up) - `multi_arch_build_native_windows_packages.yml`: derive a WiX-legal numeric MSI `ProductVersion` (`major.minor.<days-since-2016-04-01>.0`) from the PEP440 wheel version, which WiX rejects (WIX0108); keep the full version as the display version. `compute_rocm_package_version.py` gains `package_type="msi"`; generator gains `--display-version` and per-`major.minor` UpgradeCode coexistence. - `skip-ci-config.toml`: register the build workflow (now transitively reached from `multi_arch_ci.yml`). Supersedes the CI half of #9025 (kept open until these land). ### Addressed #9025 review comments (CI-side) - [`r4296724405`](#9025 (comment)) — "make this job optional, CI in other repos should skip packaging": the `build_native_msi_packages` job is now gated behind `build_native_windows` (mirroring `build_native_linux`), with `ci:build/skip-native-windows` label overrides. - [`r4289101570`](#9025 (comment)) — dev-version build failure (WIX0108, PEP440 version not valid for `Package/@Version`): fixed by deriving a numeric MSI ProductVersion in `compute_rocm_package_version.py` (`package_type="msi"`), keeping the full version as the display version. Added version-parse tests. The two remaining `multi_arch_release_windows.yml` "omit comment" threads ([`r4289106318`](#9025 (comment)), [`r4289108399`](#9025 (comment))) touch the release workflow and are addressed in #9371. ## Test plan - [x] `configure_multi_arch_ci_test.py` (build_native_windows field, platform gating, label overrides) — pass - [x] `configure_ci_path_filters_test.py`, `quartz_tracking_id_propagation_test.py`, `workflow_dispatch_inputs_test.py` — pass - [x] `compute_rocm_package_version_test.py`, `generate_msi_wxs_test.py` — pass - [x] pre-commit (black + actionlint + mdformat) — green 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4 (1M context) <noreply@anthropic.com>
4847342 to
f0cc775
Compare
ScottTodd
left a comment
There was a problem hiding this comment.
Thanks for rewriting the PR description. Much easier to read now :)
| - [x] native linux packages | ||
| - [ ] native windows packages | ||
| - [x] native windows packages |
There was a problem hiding this comment.
FYI we also have some ooooold docs that predate the native linux packaging which we could update to mention native linux and native windows now: https://github.com/ROCm/TheRock/blob/main/docs/development/development_guide.md#overall-build-architecture
See the files in https://github.com/ROCm/TheRock/tree/main/docs/development/assets which can be loaded into https://excalidraw.com/. I think agents are also pretty good at editing the excalidraw svgs now too...
| if artifacts_root.platform == "windows" and not args.skip_native_packages: | ||
| publish_native_windows_packages(artifacts_root, args.release_type, backend) |
There was a problem hiding this comment.
Once we've done enough initial validation on the nightly windows packages we can update the release docs too:
Lines 38 to 42 in 4236235
Part 2 of 2 of the Milestone 4 MSI wiring (#1987), stacked on the CI-wiring PR. With the MSI build already wired into the release workflow's build graph, this adds the release-only pieces: - publish_rocm_to_release_buckets.py: re-add publish_native_windows_packages() (reverted in #8629 because the build wasn't wired in — the #8617 regression). MSIs now copy to v5/rocm/core/windows-installers/{date}-{run_id} per RFC0012. - multi_arch_release_windows.yml: add build_native_msi_packages, make publish_to_release_buckets depend on it (the needs-edge that guarantees the MSI prefix is non-empty before publish — the fix for #8617), add trigger_test_native_windows_install (async install-test dispatch), and thread quartz_tracking_id to both new edges. - test_native_windows_packages_install.yml: declare quartz_tracking_id so the release graph's propagation check stays green. Together with the CI-wiring PR this reconstructs the original combined change exactly (verified: identical trees). Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Follows the build-workflow contract change in the CI PR (#9370), which made msi_version a required input computed upstream instead of inside the build workflow. Surface rocm_msi_package_version from multi_arch_release.yml into multi_arch_release_windows.yml and pass it as msi_version to the native MSI build job, mirroring how the deb/rpm versions are threaded on the Linux path. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Co-authored-by: Scott Todd <Scott.Todd@amd.com>
ad4e48a to
a963872
Compare
|
will wait on #9650 resolution to merge |
Summary
Adds the release-side plumbing:
Test plan
quartz_tracking_id_propagation_test.py,configure_ci_path_filters_test.py,workflow_dispatch_inputs_test.py,workflow_step_timeouts_test.py— passpublish_rocm_to_release_buckets_test.py(windows MSI copy) — pass