Update TheRock reference to (45bcd63) - #12067
Open
assistant-librarian[bot] wants to merge 1 commit into
Open
assistant-librarian[bot] wants to merge 1 commit into
assistant-librarian[bot] wants to merge 1 commit into
Conversation
|
🤖 Bump PR detected (author |
This was referenced Sep 14, 2026
Merged
tony-davis
added a commit
to ROCm/TheRock
that referenced
this pull request
Sep 15, 2026
…ilds (wkmi) (#8204) ## Summary Fixes #8203. `REPO_CONFIGS["rocm-libraries"]` in `detect_external_repo_config.py` only lists `dvc_projects: ["external-rocm-libraries"]`, but `fetch_sources.py --dvc-projects` *replaces* its own default project list rather than extending it. That silently dropped `rocm-systems` from the DVC pull for rocm-libraries-triggered external-repo builds, so the DVC-tracked `wkmi` prebuilt libs under TheRock's own `rocm-systems` submodule (`shared/amdgpu-windows-interop/wkmi/{win,lnx}/lib/`) never get pulled — breaking CLR linking on both Windows (`compiler-runtime`, missing `wkmi.lib`) and Linux (`wsl-rocdxg`, missing `libwkmi.a`). ## Risk Assessment Low risk (2/5). Single-entry addition to an existing list, isolated to the `rocm-libraries` external-repo path, mirrors the already-shipped `rocm-systems` entry exactly, and does not change behavior for any other external repo (`rocm-systems`, `rocgdb`). ## Related - Fixes: #8203 - Root-caused by: #8132 (fixed #8131's hipdnn golden-data gap, introduced this side effect) - Evidence: [ROCm/rocm-libraries#12060](ROCm/rocm-libraries#12060) (Windows), [ROCm/rocm-libraries#12067](ROCm/rocm-libraries#12067) (Linux) ## Device / Architecture Coverage Build/CI-tooling change, not device/arch-scoped. Affects Windows and Linux external-repo builds of rocm-libraries equally (both are currently broken the same way; this fix addresses both). ## Testing Summary - Unit tests covering `REPO_CONFIGS`, and generated `fetch_sources_args` for both `rocm-libraries` and `rocm-systems`. - New regression test (`test_rocm_libraries_also_pulls_rocm_systems_dvc`) that fails against the pre-fix config and passes with the fix. - New integration-level regression test (`test_rocm_libraries_scenario_pulls_rocm_systems_wkmi`), mirroring the existing `test_wsl_rocdxg_scenario` (written for the rocm-systems-triggered path), but for the rocm-libraries-triggered path — the one that was actually broken. - Manually confirmed the new integration test fails against the pre-fix `dvc_projects=["external-rocm-libraries"]` (rocm-systems/wkmi never pulled) and passes with the fix. ## Testing Checklist - [x] Config/detection unit tests - `python -m unittest build_tools/github_actions/tests/dvc_external_repo_test.py` - Status: Passed - [x] fetch_sources DVC unit tests (incl. new regression tests) - `python -m unittest build_tools/tests/fetch_sources_dvc_test.py` - Status: Passed - [x] detect_external_repo_config unit tests - `python -m unittest build_tools/github_actions/tests/detect_external_repo_config_test.py` - Status: Passed - [ ] PR CI - GitHub PR checks - Status: Pending - [ ] Downstream validation - rerun of ROCm/rocm-libraries Multi-Arch CI once this ref is picked up - Status: Pending ## Flags / Guardrails None. ## Adjacent Tests Considered Checked the `rocm-systems` and `rocgdb` `REPO_CONFIGS` entries — unaffected by this change. Considered making `fetch_sources.py --dvc-projects` additive to its default instead of replacing it, which would prevent this whole class of regression at the source; leaving that as a follow-up idea rather than folding it into this fix, to keep this PR minimal and low-risk. ## Technical Changes - Add `"rocm-systems"` to `REPO_CONFIGS["rocm-libraries"]["dvc_projects"]` in `detect_external_repo_config.py`, with a comment explaining why both entries are required. - Update `dvc_external_repo_test.py`: relax the `rocm-libraries` `dvc_projects` equality assertions to `assertIn` (list now has two entries) and add a dedicated regression test. - Add an integration-level regression test to `fetch_sources_dvc_test.py` for the rocm-libraries-triggered path. --------- Co-authored-by: Cursor <cursoragent@cursor.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 all pinned TheRock workflow and source refs to
45bcd63due to submodule bump