Repository navigation
Update TheRock reference to (e44b42b) - #7626
Merged
Merged
Conversation
github-actions
Bot
added
github actions
project: none
assistant-librarian
Bot
added
the
external contribution
jayhawk-commits
approved these changes
May 22, 2026
jayhawk-commits
left a comment
jayhawk-commits
left a comment
Collaborator
There was a problem hiding this comment.
Windows rocprim failures are known and expected
ScottTodd
reviewed
May 27, 2026
ScottTodd
left a comment
ScottTodd
left a comment
Member
There was a problem hiding this comment.
Please review these PRs more carefully.
| repository: "ROCm/TheRock" | ||
| path: TheRock | ||
| ref: ${{ inputs.therock_ref }} | ||
| ref: e44b42bbcc7452328ccf12bcac319bda5ede4a6a # 2026-05-20 commit |
Member
Member
|
Also, failures like https://github.com/ROCm/rocm-libraries/actions/runs/26145148955/job/76909291934 Are not "known and expected" - those tests must be kept passing, or they should be disabled. I saw a few PRs fly by here and in TheRock and they should be sequenced to not cause failures. |
This was referenced May 27, 2026
amd-justchen
added a commit
that referenced
this pull request
May 28, 2026
This restores the single-source-of-truth model from #7262 that was partially reverted by #7626. It also closes the remaining gap from #7262 by routing the reusable test workflows through the centralized `therock_ref` path, while preserving the intended TheRock ref at `e44b42bbcc7452328ccf12bcac319bda5ede4a6a` (2026-05-20). - **Centralize the current TheRock ref** - Update `.github/actions/ci-env/action.yml` to hold the current TheRock commit. - Make `ci-env` the only literal TheRock SHA under `.github/`. - **Remove hardcoded refs from top-level TheRock workflows** - Restore `therock-ci.yml` and `therock-ci-nightly.yml` to read the ref from `steps.ci-env.outputs.therock-ref`. - Restore `therock-ci-linux.yml` and `therock-ci-windows.yml` to use their existing `inputs.therock_ref` instead of hardcoded SHAs. - **Close the missed reusable-workflow gap** - Add required `therock_ref` inputs to: - `.github/workflows/therock-test-packages.yml` - `.github/workflows/therock-test-component.yml` - Replace their hardcoded TheRock checkout refs with `${{ inputs.therock_ref }}`. - **Thread `therock_ref` through the reusable workflow chain** - Pass `therock_ref` from `therock-ci-linux.yml` / `therock-ci-windows.yml` into `therock-test-packages.yml`. - Pass `therock_ref` from `therock-test-packages.yml` into `therock-test-component.yml`. Example of the restored pattern: ```yaml - name: Load CI environment id: ci-env uses: ./.github/actions/ci-env - name: Checkout TheRock repository uses: actions/checkout@... with: repository: "ROCm/TheRock" ref: ${{ steps.ci-env.outputs.therock-ref }} ``` <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> ## Background PR [#7262](#7262) introduced `.github/actions/ci-env/action.yml` as the single source of truth for shared CI constants (TheRock commit ref, Docker image, runner labels). All TheRock workflows were supposed to read the TheRock ref from `ci-env` instead of hardcoding it. Two follow-up bumps highlighted gaps: - PR [#7582](#7582) correctly bumped the ref by editing only `.github/actions/ci-env/action.yml` (one file). This is the intended workflow. - PR [#7626](#7626) ("Update TheRock reference to e44b42b") regressed this. It hardcoded `e44b42bbcc7452328ccf12bcac319bda5ede4a6a # 2026-05-20 commit` directly into six workflow files, effectively reverting the centralization for those files. Scott Todd called this out on #7626: > "Why was this merged? This reverted the changes from #7262 by @KKyang (cc @geomin12)" ([review comment](#7626 (comment))) Additionally, two workflow files were **never** centralized as part of #7262 — they still contained a hardcoded TheRock ref even before #7626: - `.github/workflows/therock-test-component.yml` - `.github/workflows/therock-test-packages.yml` These are reusable workflows invoked transitively from `therock-ci-linux.yml` / `therock-ci-windows.yml`, so the `therock_ref` input needs to be threaded through them. ## Goal Fully implement the intent of #7262 by: 1. Updating `.github/actions/ci-env/action.yml` so it holds the current ref (`e44b42bbcc7452328ccf12bcac319bda5ede4a6a` from 2026-05-20). This preserves the actual ref that #7626 wanted to land. 2. Removing all the hardcoded TheRock refs reintroduced by #7626 and routing them back through `ci-env`. 3. Closing the gaps left over from #7262 by also routing `therock-test-component.yml` and `therock-test-packages.yml` through `ci-env`. After this PR, there should be **exactly one** literal occurrence of a TheRock commit SHA under `.github/`, and it should live in `.github/actions/ci-env/action.yml`. ## Required changes Base this PR on the current `develop` branch. ### 1. `.github/actions/ci-env/action.yml` Set the `therock-ref` output value to the 2026-05-20 commit: ```yaml outputs: therock-ref: description: "TheRock commit hash to use" value: "e44b42bbcc7452328ccf12bcac319bda5ede4a6a" # 2026-05-20 ``` (Leave `docker-image`, `linux-runner`, `windows-runner` untouched.) ### 2. `.github/workflows/therock-ci.yml` The `setup` job's "Checkout TheRock repository" step currently has: ```yaml ref: e44b42bbcc7452328ccf12bcac319bda5ede4a6a # 2026-05-20 commit ``` Restore it to: ```yaml ref: ${{ steps.ci-env.outputs.therock-ref }} ``` (This was the form merged in #7262 and reverted by #7626.) ### 3. `.github/workflows/therock-ci-nightly.yml` Same fix as `therock-ci.yml` — the `setup` job's "Checkout TheRock repository" step must use `${{ steps.ci-env.outputs.therock-ref }}` instead of the hardcoded `e44b42b...` SHA. ### 4. `.github/workflows/therock-ci-linux.yml` The build job's "Checkout TheRock repository" step currently has: ```yaml ref: e44b42bbcc7452328ccf12bcac319bda5ede4a6a # 2026-05-20 commit ``` Restore it to: ```yaml ref: ${{ inputs.therock_ref }} ``` The `therock_ref` workflow input is already declared at the top of this file — just use it. ### 5. `.github/workflows/therock-ci-windows.yml` Same fix as `therock-ci-linux.yml`: change the hardcoded `ref:` in the "Checkout TheRock repository" step to `${{ inputs.therock_ref }}`. The input is already declared. ### 6. `.github/workflows/therock-test-packages.yml` (gap from #7262) This reusable workflow has a hardcoded ref on the "Checking out repository" step: ```yaml ref: e44b42bbcc7452328ccf12bcac319bda5ede4a6a # 2026-05-20 commit ``` Changes: - Add a new required `therock_ref` string input to `on.workflow_call.inputs` (alongside the existing `projects_to_test`, `amdgpu_families`, `test_runs_on`, `platform`, `test_type`). - Replace the hardcoded `ref:` with `${{ inputs.therock_ref }}`. - When this workflow calls `./.github/workflows/therock-test-component.yml`, pass `therock_ref: ${{ inputs.therock_ref }}` through in the `with:` block. ### 7. `.github/workflows/therock-test-component.yml` (gap from #7262) This reusable workflow has a hardcoded ref on its "Checkout Repository" step: ```yaml ref: e44b42bbcc7452328ccf12bcac319bda5ede4a6a # 2026-05-20 commit ``` Changes: - Add a new required `therock_ref` string input to `on.workflow_call.inputs`. - Replace the hardcoded `ref:` with `${{ inputs.therock_ref }}`. ### 8. Wire `therock_ref` through the call chain `therock-ci-linux.yml` and `therock-ci-windows.yml` both call `./.github/workflows/therock-test-packages.yml` via `uses:` in their `therock-test-linux` / `therock-test-windows` jobs. Update those `with:` blocks to also pass: ```yaml therock_ref: ${{... </details> <!-- START COPILOT CODING AGENT SUFFIX --> *This pull request was created from Copilot chat.* > --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JC <justchen@amd.com>
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.
Updated TheRock ref to
e44b42bdue to submodule bump