Repository navigation
Conversation
SDK build and installed metadata need the same ownership rules as kpack and bootstrap. Load the shared ownership module from rocm-systems and embed its source into generated SDK metadata so installed packages remain self-contained without a bootstrap or build-tools dependency. The rocm-systems change in ROCm/rocm-systems#11801 must land first, followed by a rocm-systems submodule bump containing that change, before this commit lands. The submodule bump is not included here. Direct bootstrap imports remain a follow-up under #4865. Progress on #8278. 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 Test results: - 100 passed with the companion rocm-systems checkout at ada5b1dc5ef. - Generated SDK metadata runs under python -I -S without importing bootstrap or TheRock build tools. - Ownership parity and invalid embedding-marker checks pass.
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
ScottTodd
left a comment
ScottTodd
left a comment
There was a problem hiding this comment.
This source patching from the bootstrap module is a little difficult to follow at first glance but I understand after a more careful reading. I guess we'll need to keep the metadata format aligned across projects, so if we change the structure of python/rocm-bootstrap/python/rocm_bootstrap/package_metadata.py in rocm-systems again we'll also need to update build_tools/_therock_utils/sdk_targets.py and build_tools/packaging/python/templates/rocm/src/rocm_sdk/_dist_info.py in this repository.
Yes, otherwise we would need to duplicate but it is coupled anyway. It will need a submodule bump before this can land, let me think a bit more about how to make this more robust. |
Individual gfx1250 and gfx1250-strict selections share the gfx1250 archive name while retaining only the requested payloads. Combined selections collect both targets before compression, avoiding separate strict archives or overwritten outputs. Family archive names remain unchanged, and test archives follow the same ownership grouping. The tarball workflow checks out rocm-systems so the ownership adapter can use shared metadata when #8325 lands. Refs #8278 Test plan: - python -m pytest -q -p no:cacheprovider build_tools/packaging/archives/tests build_tools/tests/sdk_targets_test.py Test results: - 15 tests passed, covering individual and combined target selections, both input orders, and test and multiarch archive naming. - Real artifact tarball assembly remains pending CI validation.
|
Filed #8354 to make this more robust in future and to be addressed as a follow up. |
## Motivation Individual gfx1250 and gfx1250-strict selections share the gfx1250 archive name while retaining only the requested payloads. Combined selections collect both targets before compression, avoiding separate strict archives or overwritten outputs. Progress on #8278. ## Technical Details Family archive names remain unchanged, and test archives follow the same ownership grouping. The tarball workflow checks out rocm-systems so the ownership adapter can use shared metadata when #8325 lands. ## Test Plan - python -m pytest -q -p no:cacheprovider build_tools/packaging/archives/tests build_tools/tests/sdk_targets_test.py ## Test Result - 15 tests passed, covering individual and combined target selections, both input orders, and test and multiarch archive naming. - Real artifact tarball assembly remains pending CI validation. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md. Generated with Codex
|
Rebasing on latest main (after submodule bumps landed), it got clear that centralizes target ownership metadata in bootstrap, but introduces a dependency on the rocm-systems source checkout for more packaging and further more CI tools than the PR addressed. Several workflows lack that checkout or initialize it too late, causing imports to fail before the tools can run. Our local validation missed this because the submodule was available. Instead for introducing this dependency, I will move forward andl keep a small, centralized metadata module in TheRock that matches bootstrap’s counterpart exactly. TheRock’s tools will import this local copy, and SDK packaging will embed it into generated packages so they remain self-contained. For now we will maintain the two copies in sync and remove the unnecessary tarball workflow checkout, avoiding a bootstrap installation or submodule requirement for these tools. Will send a separate PR for this. |
|
Superseded by #8377. Please review and land the other one instead @ScottTodd. |
## 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
Motivation
SDK build and installed metadata need the same ownership rules as kpack and bootstrap. Load the shared ownership module from rocm-systems and embed its source into generated SDK metadata so installed packages remain self-contained without a bootstrap or build-tools dependency.
Progress on #8278.
Technical Details
The rocm-systems change in ROCm/rocm-systems#11801 must land first, followed by a rocm-systems submodule bump containing that change, before this commit lands. The submodule bump is not included here.
Direct bootstrap imports remain a follow-up under #4865.
Test Plan
Test Result
Submission Checklist
Generated with Codex