Skip to content

Centralize target ownership metadata within TheRock - #8377

Merged
ScottTodd merged 1 commit into
mainfrom
users/marbre/gfx1250-strict-local-ownership
Sep 22, 2026
Merged

ScottTodd merged 1 commit into
mainfrom
users/marbre/gfx1250-strict-local-ownership

Conversation

@marbre

@marbre marbre commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Motivation

Packaging and CI tools need consistent ownership rules without requiring a rocm-systems checkout or an installed bootstrap package. A local metadata module matches bootstrap's counterpart exactly; the two copies must stay synchronized. Build tools import its helpers, and SDK packages embed its source so installed packages remain self-contained.

Progress on #8278.

Technical Details

The tarball workflow does not need a metadata submodule checkout, Git installation, or the associated safe-directory configuration.

Installing a pinned rocm-bootstrap version is tracked in #8354 but would require to install it in several workflows which didn't require this dependency previously. We may want to revisit that decision as there is a drift risk but this seems acceptable for now as it is not expected that package_metadata.py changes frequently.

Test Plan

  • python -m pytest -q -p no:cacheprovider \ build_tools/tests/sdk_targets_test.py \ build_tools/tests/py_packaging_test.py \ build_tools/github_actions/tests/expand_amdgpu_families_test.py \ build_tools/packaging/archives/tests/build_tarballs_test.py \ build_tools/packaging/linux/tests/build_package_test.py
  • cmp build_tools/_therock_utils/package_metadata.py \ rocm-systems/python/rocm-bootstrap/python/rocm_bootstrap/package_metadata.py

Dev build: https://github.com/ROCm/TheRock/actions/runs/35440643880

Test Result

  • 152 tests passed, including embedding and invalid-marker checks.
  • The local metadata copy matches bootstrap byte for byte.
  • Isolated imports and SDK embedding passed without submodules or site packages.

Submission Checklist

Generated with Codex

Packaging and CI tools need consistent ownership rules without requiring
a rocm-systems checkout or an installed bootstrap package. A local
metadata module matches bootstrap's counterpart exactly; the two copies
must stay synchronized. Build tools import its helpers, and SDK packages
embed its source so installed packages remain self-contained.

The tarball workflow does not need a metadata submodule checkout, Git
installation, or the associated safe-directory configuration.

Test plan:
- python -m pytest -q -p no:cacheprovider \
    build_tools/tests/sdk_targets_test.py \
    build_tools/tests/py_packaging_test.py \
    build_tools/github_actions/tests/expand_amdgpu_families_test.py \
    build_tools/packaging/archives/tests/build_tarballs_test.py \
    build_tools/packaging/linux/tests/build_package_test.py
- cmp build_tools/_therock_utils/package_metadata.py \
    rocm-systems/python/rocm-bootstrap/python/rocm_bootstrap/package_metadata.py

Test results:
- 152 tests passed, including embedding and invalid-marker checks.
- The local metadata copy matches bootstrap byte for byte.
- Isolated imports and SDK embedding passed without submodules or site
  packages.
@therock-pr-bot

therock-pr-bot Bot commented Sep 19, 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

Copy link
Copy Markdown

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

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.

I'm not really following the back and forth code moves across recent PRs here. What end state do you want? I'm fine with querying the rocm-bootstrap package for metadata or treating bootstrap and the python packaging as independent code.

How much actually needs to be synchronized? Just the "gfx1250-strict goes in gfx1250" code? That's not worth 100s of lines of cross-repository code/tests IMO.

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.

Ah, you want this instead of #8325. Okay, makes enough sense I guess.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, that was the idea :)

@ScottTodd
ScottTodd merged commit 655185e into main Sep 22, 2026
308 of 312 checks passed
@ScottTodd
ScottTodd deleted the users/marbre/gfx1250-strict-local-ownership branch September 22, 2026 22:10
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Sep 22, 2026
marbre added a commit that referenced this pull request Oct 30, 2026
## Motivation

Family selection must include both gfx1250 targets for builds,
distributions, and artifact fetching. Default test compilation includes
strict along with the other registered targets. Explicit individual
target selections retain their own payload identity.

Closes #8278

## Technical Details

Prerequisites before landing:
- ~~#8325: shared SDK ownership.~~ (superseded)
- #8377: centralized target ownership.
- #8327: native package ownership.
- #8328: tarball ownership.
- ~~A rocm-systems submodule bump containing ROCm/rocm-systems#11801.~~
**(landed)**
- Established broader component readiness for family enablement.

## Test Plan

Fresh compiler/HIP/RAND builds and SDK, DEB/RPM, and tarball validation
passed on the integration branch with temporary family membership. The
exact family enablement was then integrated for a combined build with
RAND tests and target-neutral hipthreads. Test target overrides were
left empty to exercise the default registered-target list. These checks
do not establish full component or GPU execution support.

- python -m pytest -q -p no:cacheprovider
build_tools/tests/cmake_amdgpu_targets_test.py
build_tools/tests/artifact_manager_tool_test.py
build_tools/github_actions/tests/configure_multi_arch_ci_test.py
- cmake --build ../therock-build-rand-tests-gfx125X-dcgpu --target
artifact-rand artifact-hipthreads
-
../therock-build-rand-tests-gfx125X-dcgpu/compiler/amd-llvm/dist/lib/llvm/bin/llvm-objdump
--offloading
../therock-build-rand-tests-gfx125X-dcgpu/math-libs/hipthreads/stage/lib/hipthreads/libhipthreads.a
- python -m rocm_kpack.tools.verify_artifacts --artifacts-dir
../therock-build-rand-tests-gfx125X-dcgpu/artifacts --verbose
--clang-offload-bundler
../therock-build-rand-tests-gfx125X-dcgpu/compiler/amd-llvm/dist/lib/llvm/bin/clang-offload-bundler

## Test Result

- 171 passed and 1 existing test skipped with umask 022.
- rocRAND and hipRAND tests compiled for both gfx1250 targets. Each
target's test kpack contains 34 entries with only its target identity.
- Target-neutral hipthreads compiled with the default test target list.
Its library contains exactly 29 distinct device images, including
separate gfx1250 and gfx1250-strict images.
- All four artifact verifier checks passed.
- No GPU tests were executed.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.

Generated with Codex
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