Repository navigation
Bump rocm-systems from d22b864 to e4c06dd - #7005
Closed
systems-assistant[bot] wants to merge 1 commit into
Closed
systems-assistant[bot] wants to merge 1 commit into
systems-assistant[bot] wants to merge 1 commit into
Conversation
systems-assistant
Bot
added
the
ci:run-all-archs
|
🤖 Bump PR detected (author |
ScottTodd
reviewed
Jul 30, 2026
Member
There was a problem hiding this comment.
The "validate artifact structure" test is failing: https://github.com/ROCm/TheRock/actions/runs/30577017670/job/91006212708?pr=7005
=========================== short test summary info ============================
FAILED tests/test_artifact_structure.py::TestArtifactStructure::test_no_cross_artifact_overlaps - Failed: Found 3 cross-artifact overlap(s) across 357 archives (see https://github.com/ROCm/TheRock/issues/3796):
include/hsakmt/hsakmt.h
- core-runtime (core-runtime_run_generic.tar.zst)
- wsl-rocdxg (wsl-rocdxg_dev_generic.tar.zst)
include/hsakmt/hsakmttypes.h
- core-runtime (core-runtime_run_generic.tar.zst)
- wsl-rocdxg (wsl-rocdxg_dev_generic.tar.zst)
include/hsakmt/rocdxg_smi.h
- core-runtime (core-runtime_run_generic.tar.zst)
- wsl-rocdxg (wsl-rocdxg_dev_generic.tar.zst)
========================= 1 failed, 2 passed in 10.43s =========================
projects/rocr-runtime/libhsakmt/include/hsakmt/rocdxg_smi.h was added by ROCm/rocm-systems#8745
(more analysis pending)
Member
There was a problem hiding this comment.
Agent analysis:
• Root cause identified: rocm-systems PR 8745 added an install rule that makes wsl-rocdxg claim three headers already
owned by core-runtime.
- Commit 59a33b9 (https://github.com/ROCm/rocm-systems/commit/59a33b9e0ec69871d0c2ed9f3c0f66ea98a26307) added hsakmt.h,
hsakmttypes.h, and rocdxg_smi.h to CMakeLists_wsl.txt.
- The normal libhsakmt build already installs the entire include/hsakmt directory, including the new header.
- The artifact test requires single ownership of each flattened path, even when duplicate files are byte-identical.
- Current rocm-systems/develop still contains the duplicate install rule, so simply rerunning PR 7005 will fail again.
Recommended resolution:
1. Remove the three-header install(FILES ...) block from CMakeLists_wsl.txt in rocm-systems.
2. If amdrocm-wsl-devel must expose those headers independently, make it depend on amdrocm-runtime-devel instead of
duplicating them. It currently only depends on amdrocm-wsl.
3. Land that upstream and refresh the submodule bump.
For an urgent TheRock-only workaround, exclude include/hsakmt/** from the wsl-rocdxg dev component in D:/projects/
TheRock/core/artifact-wsl-rocdxg.toml:1. I’d favor the upstream fix because the WSL CMake file itself already describes
the headers as shared with libhsakmt.
One secondary issue: core-runtime_run_generic.tar.zst owns these headers because D:/projects/TheRock/core/artifact-
core-runtime.toml:1 has a bare run entry, which acts as a catch-all. That classification could be cleaned up
separately, but moving the headers to core-runtime_dev would not eliminate the cross-artifact overlap.
Follow-up:
› Could/should this be fixed via artifact descriptors in TheRock or does it need to be fixed in rocm-systems?
• It can be fixed in TheRock’s descriptor, but the primary fix should be in rocm-systems.
The duplicate originates from CMakeLists_wsl.txt explicitly installing headers already installed by the normal
libhsakmt build. That file even says librocdxg should share libhsakmt’s headers “without duplicating header
installation,” so the new block contradicts the existing ownership model.
Recommended split:
- rocm-systems: remove the duplicate header install block. This fixes all packaging consumers, not just TheRock.
- TheRock: make amdrocm-wsl-devel depend on amdrocm-runtime-devel if the WSL development package needs those headers.
- Artifact descriptor exclusion: reasonable only as a temporary unblock or if upstream deliberately decides librocdxg
must install standalone copies.
I would not make the descriptor exclusion the permanent fix unless the rocm-systems maintainers confirm the duplication
is intentional. Otherwise it hides an upstream packaging collision that could also affect DEB/RPM installations outside
TheRock.
1 task done
Contributor
|
revert up: ROCm/rocm-systems#9527 |
Member
|
Since this bump stalled, we landed some patches in #7007 that will need to be undone as part of the next bump. |
Contributor
|
As patches are landed are we going to go ahead and merge this bump? |
Member
|
No, this bump has test failures. We'll need a new one (or to reuse this and advance the submodule pointer) |
Member
|
Closing in favor of #7011 |
ihhethan
added a commit
to ROCm/rocm-systems
that referenced
this pull request
Aug 26, 2026
Flora Cui noted on review that the hsakmt public headers are already installed by rocr-runtime, and asked whether a second copy shipped from the WSL build could put rocprofiler-sdk and rocr-runtime on mismatched headers now that both live in one repo. Both points land on the same defect, and develop already said so: the comment this rule displaced explains that no separate header installation belongs here and that duplicating it breaks TheRock's artifact uniqueness check (ROCm/TheRock#7005). include/hsakmt is installed by the else() branch of if (WIN_SDK) in libhsakmt/CMakeLists.txt, which the native package runs, and in a full ROCm install both copies land in the same prefix. Restore develop's comment verbatim so the rule is not re-added, and drop the installed_header_smoke targets with it. They only checked that the installed header set compiles standalone, so with no install rule they have no subject. dxg-snapshot-refcount and dxg-dynamic-exports still build and pass. Keep the hsakmt.h forward declarations. They are not test scaffolding: src/extsem.c includes only <hsakmt/hsakmt.h>, and without them GCC scopes struct kfd_runtime_info and struct kfd_dbg_device_info_entry to the prototypes that name them. linux/kfd_ioctl.h defines both and is excluded from the dev package, so these declarations are what make those prototypes well formed for any C consumer.
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.
Bumps ROCm/rocm-systems from ROCm/rocm-systems@d22b864 to ROCm/rocm-systems@e4c06dd.
See full comparison here: ROCm/rocm-systems@d22b864...e4c06dd