Skip to content

ci: re-add Windows MSI publishing to release buckets (RFC0012 path) - #8704

Closed
idass1990 wants to merge 1 commit into
mainfrom
users/idass1990/readd_msi_publish_windows_installers
Closed

idass1990 wants to merge 1 commit into
mainfrom
users/idass1990/readd_msi_publish_windows_installers

Conversation

@idass1990

Copy link
Copy Markdown
Contributor

Summary

Re-adds publish_native_windows_packages() to publish_rocm_to_release_buckets.py, previously reverted in #8629 to fix the #8617 regression, with the destination path corrected per RFC0012.

⚠️ Do not merge yet

The MSI build workflow (multi_arch_build_native_windows_packages.yml) is still wired into nothing (0 callers). publish_native_windows_packages() hard-fails on zero MSI files — so merging this alone would reintroduce #8617 on the next nightly Windows release.

This PR must land together with (or after) the Milestone 4 wiring that calls the MSI build workflow from the release pipeline (#1987). Held as a draft until then.

Test plan

  • pytest build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py — 14 passed; test_windows_copies_native_msi_packages asserts the v5/rocm/core/windows-installers/{date}-{run_id} destination.
  • Post-merge (ROCm-only, OIDC): validate the publish hop via workflow_dispatch once the build workflow is wired in and producing MSIs.

🤖 Generated with Claude Code

Re-adds publish_native_windows_packages() to the release publish script,
previously reverted in #8629 to fix the #8617 regression. The original
#8097 implementation published to core/packages/msi/, which does not
match RFC0012: per the repo-structure RFC, Windows installer artifacts
(.exe/.msi) belong under core/windows-installers/, not core/packages/.

Addresses marbre's post-merge review comment on #8097
(discussion_r4145328177). The current MSI is a single OS-agnostic
installer, so it is published to a flat windows-installers/ prefix
(dated subdir for dev/nightly, stable path for prerelease) rather than
the per-OS subfolders (win11/, server2022/, server2025/) RFC0012
reserves for OS-specific installers — our MSI payload is identical
across Windows versions.

NOTE: do not merge until the MSI build workflow
(multi_arch_build_native_windows_packages.yml) is wired into the release
pipeline (Milestone 4, #1987). Publishing hard-fails on zero MSI files;
merging this alone, while the build has no callers, reintroduces #8617.

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
@therock-pr-bot

therock-pr-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 2, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@ScottTodd ScottTodd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Do not merge yet

The MSI build workflow (multi_arch_build_native_windows_packages.yml) is still wired into nothing (0 callers). publish_native_windows_packages() hard-fails on zero MSI files — so merging this alone would reintroduce #8617 on the next nightly Windows release.

This PR must land together with (or after) the Milestone 4 wiring that calls the MSI build workflow from the release pipeline (#1987). Held as a draft until then.

This PR is not a draft. What sort of review are you looking for here?

idass1990 added a commit that referenced this pull request Oct 15, 2026
…stone 4)

The MSI build workflow (multi_arch_build_native_windows_packages.yml) existed but
had no callers — the gap behind the #8617 regression. Wire it into both Windows
workflows per Milestone 4 (#1987), with the build/publish distinction each needs:

- multi_arch_ci_windows.yml (nightly): add build_native_msi_packages, which
  builds the runtime MSI from this run's artifacts and uploads it to the run's
  artifacts bucket (a build artifact). No release-bucket publish in CI.

- multi_arch_release_windows.yml (release cut): add build_native_msi_packages,
  make publish_to_release_buckets depend on it (so the MSIs exist before publish
  copies them to the release bucket — the dependency edge that prevents #8617),
  and add trigger_test_native_windows_install, which dispatches the MSI install
  test against this run's artifacts prefix. Both jobs added to
  notify_quartz_completed's needs.

Only the runtime package is wired (packages: runtime); core's Windows artifacts
are not yet published in nightly runs.

DRAFT: depends on #8704 (publish_native_windows_packages) and #8669
(test_native_windows_packages_install.yml), which are not yet on main. The
workflow_dispatch_inputs test correctly flags the install-test dispatch until
#8669 merges. Rebase and validate once both land.

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
@idass1990

Copy link
Copy Markdown
Contributor Author

⚠️ Do not merge yet

The MSI build workflow (multi_arch_build_native_windows_packages.yml) is still wired into nothing (0 callers). publish_native_windows_packages() hard-fails on zero MSI files — so merging this alone would reintroduce #8617 on the next nightly Windows release.
This PR must land together with (or after) the Milestone 4 wiring that calls the MSI build workflow from the release pipeline (#1987). Held as a draft until then.

This PR is not a draft. What sort of review are you looking for here?

was looking for code correctness and logic review.. #9025 should merge first though and is now ready for review

@ScottTodd

Copy link
Copy Markdown
Member

Don't you need the code here for publish_to_release_buckets to work on #9025? Seems like the PRs should be batched together?

@idass1990

Copy link
Copy Markdown
Contributor Author

Don't you need the code here for publish_to_release_buckets to work on #9025? Seems like the PRs should be batched together?

yeah to keep things simple and clean ill batch them

@idass1990

Copy link
Copy Markdown
Contributor Author

Don't you need the code here for publish_to_release_buckets to work on #9025? Seems like the PRs should be batched together?

this PR is now picked into #9025, i will close this one

@idass1990

Copy link
Copy Markdown
Contributor Author

Superseded by #9025, which batches the publish function together with the build + install-test wiring. Merging them atomically removes any window where publish could run without the build (the #8617 concern). The publish_native_windows_packages() change here was cherry-picked into #9025 unchanged.

@idass1990 idass1990 closed this Oct 15, 2026
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants