Repository navigation
Update TheRock reference to (4be597f) - #8014
assistant-librarian[bot] wants to merge 7 commits into
Conversation
github-actions
Bot
added
github actions
project: none
assistant-librarian
Bot
added
the
external contribution
|
This bump was generated automatically, but the bump automation is currently broken: after rocm-libraries#7262 centralized the TheRock pin into In the meantime, instead of the 6-file hardcode here, recommend making this a manual one-line change to the centralized source of truth, - value: "685f8c3b89006309b34a7c16de17e841400f0d76" # 2026-05-29 commit
+ value: "84b3ab155f3faf64c5f7f783a7a35f315e0969c7" # 2026-06-03 commitThis unblocks #7872 identically — One caveat regardless of approach: |
The bump automation bot incorrectly hardcoded the TheRock SHA in 6
workflow files instead of only updating ci-env/action.yml. This restores
the original ${{ inputs.therock_ref }} and ${{ steps.ci-env.outputs.therock-ref }}
expressions and updates ci-env/action.yml to the new ref.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Yes, this PR bump should unblock #7872. |
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
need to add here:
to trigger CI
a208165 to
a695160
Compare
The path filter in therock_configure_ci.py only checked .github/workflows/ and .github/scripts/ — changes to .github/actions/ci-env/ (the single source of truth for the TheRock ref) were not detected, causing CI to skip builds on TheRock ref bump PRs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tony-davis
left a comment
tony-davis
left a comment
There was a problem hiding this comment.
Approving. The bump is now done the right way: the TheRock ref is updated in the centralized single source of truth (.github/actions/ci-env/action.yml), the workflow ${{ steps.ci-env.outputs.therock-ref }} expressions are restored (so nothing needs reverting once the automation fix ROCm/TheRock#5620 lands), and therock_configure_ci.py is correctly taught that .github/actions/ci-env/* changes should trigger the full project matrix — which is what makes a ref bump actually exercise CI.
This approval is of course contingent on CI completing green, with one known exception: the pre-existing rocPRIM-isolated build failures (Linux (rocprim,rocthrust,hipcub) / Build on gfx94X/gfx950). Those are the amdsmi configure error fixed by ROCm/TheRock#5612 (still open) and are unrelated to this change — 84b3ab1 simply predates that fix.
348857e to
f580fdb
Compare
tony-davis
removed
external contribution
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
from here: ROCm/TheRock@685f8c3...84b3ab1, it seems this PR will be fine to land
|
Hey @geomin12 — thanks for flagging the failing job. I dug into the log for The configure step fails in which ends in Since this is the pre-existing failure we'd already called out as the known exception (and it's unrelated to what this PR changes), would you be okay re-reviewing / dismissing the changes-requested so it's not blocking? Or, if you'd prefer to wait for a fully-green run, we can hold this until #5612 merges and bump to a SHA with both fixes. Either way works for me — just let me know your preference. |
|
Following up on my comment above — I dug into the Root cause: The failure is the
Why it matters: This isn't a build we rarely run. In Recommendation: Hold this bump. The guard is ROCm/TheRock#5612 (still open) — once it merges we should bump to a SHA that contains both #5529 (tensilelite, unblocks #7872) and #5612 (amdsmi guard), rather than landing |
tony-davis
left a comment
tony-davis
left a comment
There was a problem hiding this comment.
Based on the rocprim build issue, this bump can't proceed until the SHA can point to a version of TheRock where that build issue is corrected.
tony-davis
left a comment
tony-davis
left a comment
There was a problem hiding this comment.
If CI passes with new SHA, approved.
tony-davis
changed the title
|
Closing this as overcome by events. The TheRock ref bump this PR was carrying has already landed on One piece of this PR is still worth landing and is not yet on |
Revert ci-env TheRock ref bump and therock_configure_ci.py changes that belong to PR #8014, not this PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert ci-env TheRock ref bump and therock_configure_ci.py changes that belong to PR #8014, not this PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…es (#5620) ## Summary Fix the bump automation bot to update the centralized CI environment action instead of modifying individual workflow files for rocm-libraries. ## Technical Details ROCm/rocm-libraries#7262 centralized TheRock ref pin into `.github/actions/ci-env/action.yml` The bump automation bot was still hardcoded to update 6 workflow files that no longer contain direct `ref:` fields, causing PRs like ROCm/rocm-libraries#8014 The ci-env action file uses a different YAML structure (`therock-ref:` → `value: "<sha>"`) than workflow files (`repository: "ROCm/TheRock"` → `ref: <sha>`), so a new updater function is needed. Changes: - `ROCM_LIBRARIES_FILES`: replaced 6 workflow paths with single `".github/actions/ci-env/action.yml"` - `SUBMODULE_CONFIG`: added `"updater"` key (`"ref"` for rocm-systems, `"ci-env"` for rocm-libraries) - `update_ci_env_file()`: new function that finds `therock-ref:` block and updates the `value:` line - `handle_push()`: selects updater function based on config instead of always calling `update_ref_in_file()` - Fix blank-line edge case in `update_ci_env_file()` where empty lines between `description:` and `value:` caused a silent skip - Replace deprecated `datetime.utcnow()` with `datetime.now(timezone.utc)` (both updater functions) ## Test Plan - [ ] Merge and verify the next bot-generated rocm-libraries bump PR touches only `ci-env/action.yml` - [ ] Verify the next rocm-systems bump PR still works (unchanged code path) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary The TheRock commit pin is centralized in `.github/actions/ci-env/action.yml` (the single source of truth introduced by #7262). But `therock_configure_ci.py` only treated `.github/workflows/` and `.github/scripts/` changes as CI-relevant, so a ref bump that edits **only** `ci-env/action.yml` wasn't detected as CI-relevant — meaning it didn't trigger the full build/test matrix that validates the new ref (only the basic checks ran). This teaches `is_path_workflow_file_related_to_ci` to also treat `.github/actions/ci-env/action.yml` as CI-relevant, so centralized TheRock ref bumps exercise the full project matrix again. ## Changes - `.github/scripts/therock_configure_ci.py`: add `GITHUB_ACTIONS_CI_PATTERNS = ["ci-env/action.yml"]` and a `.github/actions/` clause to `is_path_workflow_file_related_to_ci`. (The `.github/workflows/` + `.github/scripts/` checks are refactored into a single prefix loop — behaviorally identical.) - `.github/scripts/tests/therock_configure_ci_test.py`: extend `test_is_path_workflow_file_related_to_ci` to assert `ci-env/action.yml` is CI-relevant and that an unrelated action path (`setup-rocm-linux/action.yml`) is not. ## Background This is the one still-needed piece carved out of the now-closed #8014 (a TheRock ref bump that was overcome by events once #8196 bumped `develop` directly). The bump portion of #8014 is obsolete, but this trigger fix is not yet on `develop`. ## Test plan - [x] `is_path_workflow_file_related_to_ci` unit test updated (syntax-checked locally; full suite runs in CI). - [ ] Confirm a subsequent ci-env-only ref bump triggers the full matrix. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Summary
Bump the TheRock reference to
4be597ff149ea67c11b6036480f96d90f64365e1(2026-06-04). The bump is applied to the centralized single source of truth,.github/actions/ci-env/action.yml, rather than hardcoded into individual workflow files, and includes a companion fix so that ci-env changes correctly trigger the full CI matrix.What this PR does
therock-refoutput value in.github/actions/ci-env/action.ymlfrom685f8c3(2026-05-29) to4be597f(2026-06-04). All workflows consume this via${{ steps.ci-env.outputs.therock-ref }}/inputs.therock_ref, so the new ref propagates everywhere.${{ steps.ci-env.outputs.therock-ref }}expressions in thetherock-*workflow files (an earlier automated revision had hardcoded the SHA into them). This keeps the single source of truth intact, so nothing needs reverting once the bump-automation fix ([ci] Fix bump automation to update ci-env/action.yml for rocm-libraries TheRock#5620) lands..github/scripts/therock_configure_ci.pyto treat.github/actions/ci-env/action.ymlas CI-relevant, so a ref bump that only touchesci-env/action.ymlstill triggers the full project matrix. Without this, a centralized ref bump would no longer trigger the build/test jobs that validate it.Why
4be597fincludes:amdsmibuild guard), which keeps the prim-isolatedLinux (rocprim,rocthrust,hipcub) / Buildgreen.Work tracking
Tests
Full TheRock CI matrix runs on this PR (triggered by the configure_ci change above).
History / note
This PR originally targeted
84b3ab1(2026-06-03), which predated the rocPRIMamdsmibuild guard and caused the prim-isolated builds (gfx94X, gfx950) to fail with theget_target_property() ... non-existent target "amdsmi"configure error — a regression introduced by ROCm/TheRock#4334. It has been re-targeted to4be597f, a descendant of84b3ab1that additionally contains the #5612 guard fix, so that failure no longer occurs.