fix(amdsmi): correctness and coverage fixes for the Python packaging rework by marifamd · Pull Request #9033 · ROCm/rocm-systems

marifamd · 2026-07-22T17:57:16Z

Description

Correctness and coverage follow-ups to the AMD SMI Python packaging rework (relanded in #9340). This branch is the packaging-hardening layer on top of the SONAME conflict test + build harness (#7912).

Rebased onto develop after #7912 and #9340 merged; the now-upstream shebang and self-locate-loader commits were dropped, and the surviving relocatable-loader change was reconciled with develop's version (keeps develop's len(parents) > 3 guard and adds a try/except OSError fall-through so a present-but-unloadable relocatable library does not shadow the system SONAME).

Loader / wrapper

Fall through to the system library when the relocatable TheRock library exists but fails to load (missing deps).
Dedupe the wrapper _libraries init (the ctypesgen preamble already initializes it).
Read and write the staged wrapper as UTF-8 in disable_system_fallback.py so the wheel flag flip is deterministic regardless of build locale; make the staging idempotent so a rebuild that reuses the flipped copy succeeds.

Packaging correctness

Stop the RPM from co-owning system Python directories.
Restrict package preun/prerm cleanup to a full erase (not upgrades).
Quote the install prefix when writing ld.so.conf.d on RPM.
Pin permissions on the installed Python module; make the operational log group-writable (not world-writable) and stop the logger from restoring world-writable permissions.
Only strip release libraries when a strip tool is available; fail the build when the system python3 cannot be detected.

Wheel build

Build one wheel per platform (not per interpreter), tag it manylinux_2_28, and verify the bundled .so.
Use a clean PyPI version and expose the commit as metadata.
Reject regenerating the wrapper against the wheel library; skip the system site-packages install for wheel builds.

Tests

Loader-contract tests in test_abi_compat.py: preferred bundled .so, wheel refuses system fallback when the bundle is missing, the relocatable OSError fall-through, and an ambiguous-wrapper hard failure. Assert the idempotent no-op in the fallback double-run guard.
Guard against drift between the two installed module copies; install the dual-copy runner so its guard tests run in CI; run the wheel install-location check without a GPU.

Verification

cmake -DBUILD_TESTS=ON + make: 0 errors, libamd_smi.so.27 + amdsmitst built.
test_abi_compat.py (10) and test_dual_copy_guard.py (4): all pass.

JIRA ID

ROCM-3941

therock-pr-bot · 2026-07-22T21:27:34Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Title/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.

geomin12

lgtm! but this is majority of amd-smi code changes, i would appreciate an amd-smi team member(s) to review and approve

…sions - The postinst sets the operational log to 0664, but the logger reset it to 0666 (S_IWOTH) on every open, so the first root run made the root-owned log world-writable again and negated the CWE-732 fix. - Drop S_IWOTH so the runtime mode matches the 0664 the package installs: owner and group may write, others read-only. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- The module installs under an absolute system site-packages path (e.g. /usr/lib64/python3.9/site-packages) that the OS python package owns. - CPack's RPM auto-filelist declared %dir ownership of that directory and its /usr parents with the default group-writable 0775 mode, co-owning system directories the package should not touch. - Walk the sitelib's ancestors and add them to CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION so the RPM owns only its own <sitelib>/amdsmi subtree; verified the /usr parents no longer appear in the built RPM's file list. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- The wheel-loader tests copy amdsmi_wrapper.py expecting the committed system variant (_AMDSMI_ALLOW_SYSTEM_FALLBACK = True), then flip it to False. - _find_py_interface() fell back to `import amdsmi`, which in the supported wheel-plus-system coexistence resolves to the wheel's wrapper (already False), making the flip test fail on a valid configuration. - Prefer the co-installed share/amd_smi/amdsmi copy (always the system variant, immune to a wheel shadowing sys.path) before the imported package. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- The install(DIRECTORY) to AMDSMI_SYSTEM_PYTHON_SITELIB ran even with BUILD_PYTHON_WHEEL=ON. On a manylinux build host the detected sitelib can be a container-internal path (e.g. /opt/python/cp312-cp312/...), so a cpack run in that configuration would ship the module at a path no target system has. - The wheel packages its own module tree, so guard the system install with if(NOT BUILD_PYTHON_WHEEL). Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- With BUILD_WRAPPER=ON and BUILD_PYTHON_WHEEL=ON the generator is fed -l libamd_smi_python.so, so ctypeslib emits bindings keyed on 'libamd_smi_python.so' while the loader still populates 'libamd_smi.so' -- every API call would then raise KeyError. - The wheel reuses the committed wrapper (only flipping the fallback flag) and never needs regeneration, so fail configure with a clear message instead of emitting a broken wrapper. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

…ld dir - The manylinux smoke test only checked that `import amdsmi` works, so a build-step ordering regression that shipped a wheel with the system-library fallback still enabled would pass silently. Add an assertion that the built wheel's _AMDSMI_ALLOW_SYSTEM_FALLBACK is False. - The SONAME check read the build tree from a hardcoded /tmp/amdsmi-build that only matched build_wheel.py's default by coincidence. Set the build dir once in a job-level env var and pass it to both the wheel build and the SONAME check so the coupling is explicit. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- the wheel bundles a prebuilt .so loaded via ctypes and carries no python c-extension, so it is identical (py3-none) for every cpython; building it once per platform is sufficient - add select_interpreters() to pick a single build interpreter and reduce the remaining oldest/newest ones to build-smoke checks - replace the per-interpreter build_wheels() with build_wheel() plus build_smoke() so the manylinux image no longer emits ~8 identical wheels Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

…dled .so - the wheel ships a prebuilt libamd_smi_python.so, so a py3-none-any tag lets pip install it on incompatible platforms and fail at load time - add a setup.py that marks the distribution binary and overrides bdist_wheel to emit a non-pure py3-none platform wheel, which auditwheel can then stamp with the concrete manylinux tag - stage setup.py into the wheel package tree from cmake - fail the build when --repair is set but auditwheel is missing or the repair fails, instead of silently shipping an un-audited wheel Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

…fails to load - the loader returned early once the relative TheRock library path existed, so a present-but-unloadable file (missing dependencies) shadowed the dynamic-linker SONAME lookup and aborted the load - catch OSError from the relative load and continue to the system SONAME - apply the same fix to generator.py so a regenerated wrapper keeps it Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- the leftovers, log folder, test dir and logrotate cleanups ran on upgrade as well, deleting the incoming package's state - gate them on a full erase (RPM $1 -eq 0, DEB the remove action) so only an uninstall triggers them - keep the old gpuv-smi symlink removal on both erase and upgrade

- the comment implied the group-write bit lets same-group processes write, but the log is root-owned so no non-root user is in its group - restate that 0664 only matches the postinst mode and that withholding world-write is the security-relevant part Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

…data - the wheel version carried a +hash local suffix, which PyPI rejects on upload - add AMDSMI_WHEEL_RELEASE (and build_wheel.py --release) to emit a clean MAJOR.MINOR.PATCH version for the published wheel - keep +hash by default so per-commit CI wheels stay uniquely named - expose the commit as amdsmi.__commit__ so it is recoverable either way - pass --release on the published manylinux leg Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- a rebuild reuses the already-flipped staged wrapper, so the script saw zero occurrences of the True anchor and aborted the second build - treat an already-disabled flag as a no-op, while still failing when neither the anchor nor the replacement is present Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- state that AMDSMI_WHEEL_BUILD_DIR mirrors build_wheel.py's default build path, so its origin is clear at the point of use Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- sparse-checkout only projects/amdsmi and the workflows dir instead of the whole monorepo - assert the imported amdsmi resolves under pip's install location so the smoke test proves the built wheel is what loads, not another copy - collapse the two identical wheel uploads into one SHA-named artifact kept for manual download Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- the coexistence table describes default sys.path ordering between install locations, but a global PYTHONPATH precedes all of them - document that some ROCm container images set PYTHONPATH=/opt/rocm/share/amd_smi, which makes the share copy win over a pip wheel, and how to restore install-location precedence Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

Replace the separate Set Project Directory step with a job-level env entry, since the path is a fixed constant. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

- Dedupe the wrapper _libraries init: the ctypesgen preamble already initializes it, so the injected loader block no longer re-declares it (wrapper regenerated from generator.py). - Read and write the staged wrapper as UTF-8 in disable_system_fallback.py so the wheel flag flip is deterministic regardless of the build locale. - Assert the idempotent no-op in the fallback double-run guard, and add a case that the tool refuses (non-zero exit) an ambiguous wrapper. - Cover the relocatable OSError fall-through: a present-but-unloadable <root>/lib/<SONAME> must fall through to the system SONAME. - Reword a comment to drop non-standard phrasing. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

…kspace The build/test jobs run in a container where the repo is mounted at a different path than the host runner. Job-level env PROJECT_DIR resolved ${{ github.workspace }} to the HOST path, so python3 could not open run_amdsmi_build.py inside the container (No such file or directory). Set PROJECT_DIR in a step from $GITHUB_WORKSPACE, which is the in-container mount, matching the working pattern on develop. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

The standalone PyPI wheel ships only the amdsmi Python bindings; it declares no console_scripts entry point, so it does not provide the amd-smi CLI. Mark that matrix cell unsupported. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

…rework (#9033) ## Description Correctness and coverage follow-ups to the AMD SMI Python packaging rework (relanded in #9340). This branch is the packaging-hardening layer on top of the SONAME conflict test + build harness (#7912). Rebased onto `develop` after #7912 and #9340 merged; the now-upstream shebang and self-locate-loader commits were dropped, and the surviving relocatable-loader change was reconciled with develop's version (keeps develop's `len(parents) > 3` guard and adds a `try/except OSError` fall-through so a present-but-unloadable relocatable library does not shadow the system SONAME). ### Loader / wrapper - Fall through to the system library when the relocatable TheRock library exists but fails to load (missing deps). - Dedupe the wrapper `_libraries` init (the ctypesgen preamble already initializes it). - Read and write the staged wrapper as UTF-8 in `disable_system_fallback.py` so the wheel flag flip is deterministic regardless of build locale; make the staging idempotent so a rebuild that reuses the flipped copy succeeds. ### Packaging correctness - Stop the RPM from co-owning system Python directories. - Restrict package `preun`/`prerm` cleanup to a full erase (not upgrades). - Quote the install prefix when writing `ld.so.conf.d` on RPM. - Pin permissions on the installed Python module; make the operational log group-writable (not world-writable) and stop the logger from restoring world-writable permissions. - Only strip release libraries when a strip tool is available; fail the build when the system `python3` cannot be detected. ### Wheel build - Build one wheel per platform (not per interpreter), tag it `manylinux_2_28`, and verify the bundled `.so`. - Use a clean PyPI version and expose the commit as metadata. - Reject regenerating the wrapper against the wheel library; skip the system site-packages install for wheel builds. ### Tests - Loader-contract tests in `test_abi_compat.py`: preferred bundled `.so`, wheel refuses system fallback when the bundle is missing, the relocatable OSError fall-through, and an ambiguous-wrapper hard failure. Assert the idempotent no-op in the fallback double-run guard. - Guard against drift between the two installed module copies; install the dual-copy runner so its guard tests run in CI; run the wheel install-location check without a GPU. ### Verification - `cmake -DBUILD_TESTS=ON` + `make`: 0 errors, `libamd_smi.so.27` + `amdsmitst` built. - `test_abi_compat.py` (10) and `test_dual_copy_guard.py` (4): all pass. ## JIRA ID ROCM-3941 --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

#9034) ## Description Packaging install-path and multi-Python-version test coverage for the AMD SMI Python packaging rework. Stacks on #9033 (packaging correctness follow-ups). - **Loader across Python versions** — verify loader behavior across supported interpreters; run the 3.6/3.7 loader tests in python container images. - **Install-path assertion** — assert the system package ships the module on an importable path (`site-packages`/`dist-packages`). - **Upgrade/downgrade round trip** — exercise the deb/rpm upgrade and downgrade path in CI; select upgrade-test packages by build order (not version string); fail the upgrade test when the `ld.so.conf.d` linker registration is missing; actually run the CLI check in the upgrade test. - **Python 3.6 compatibility** — make the version and package tests run on Python 3.6. - **CI hygiene** — wire the packaging tests into the harness and CI, sparse-checkout the python-version and upgrade jobs, pin `actions/setup-python` to a commit SHA. ## JIRA ID ROCM-3941 Co-authored-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>

marifamd requested review from a team as code owners July 22, 2026 17:57

marifamd mentioned this pull request Jul 22, 2026

test(amdsmi): packaging install-path and multi-Python-version coverage #9034

Merged

github-actions Bot added documentation Improvements or additions to documentation project: amdsmi github actions Pull requests that update GitHub Actions code labels Jul 22, 2026

systems-assistant Bot added the organization: ROCm label Jul 22, 2026

geomin12 reviewed Jul 22, 2026

View reviewed changes

Comment thread .github/workflows/amdsmi-manylinux-build.yml Resolved

dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from fd22606 to e95d2a4 Compare July 23, 2026 00:40

marifamd force-pushed the users/marifamd/amdsmi-packaging-followup branch from 4fce49e to 3645f9e Compare July 23, 2026 01:15

marifamd force-pushed the users/juwillia/py-wheel-tests branch from e95d2a4 to da3a3e3 Compare July 23, 2026 03:47

marifamd force-pushed the users/marifamd/amdsmi-packaging-followup branch from 3645f9e to 4eaeeeb Compare July 23, 2026 03:47

github-actions Bot added the MINOR ABI BREAKAGE label Jul 23, 2026

marifamd force-pushed the users/juwillia/py-wheel-tests branch from da3a3e3 to 067bbc8 Compare July 24, 2026 14:04

marifamd force-pushed the users/marifamd/amdsmi-packaging-followup branch from f12f477 to 8b8852f Compare July 24, 2026 14:04

github-actions Bot removed the MINOR ABI BREAKAGE label Jul 24, 2026

marifamd force-pushed the users/juwillia/py-wheel-tests branch from 067bbc8 to 5521e21 Compare July 24, 2026 14:31

marifamd force-pushed the users/marifamd/amdsmi-packaging-followup branch from 8b8852f to b580ad8 Compare July 24, 2026 14:31

marifamd mentioned this pull request Jul 27, 2026

ci(amdsmi): [ROCM-3941] Python packaging rework #7914

Merged

1 task

marifamd force-pushed the users/juwillia/py-wheel-tests branch 2 times, most recently from b422d2c to a65f95a Compare July 27, 2026 07:36

marifamd force-pushed the users/marifamd/amdsmi-packaging-followup branch 4 times, most recently from d5ff51f to 7dc2998 Compare July 27, 2026 15:54

marifamd force-pushed the users/juwillia/py-wheel-tests branch from 6258090 to af25984 Compare July 27, 2026 19:52

marifamd force-pushed the users/marifamd/amdsmi-packaging-followup branch from 7dc2998 to 7e2e902 Compare July 27, 2026 19:58

dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from af25984 to 6434cec Compare July 27, 2026 20:48

dmitrii-galantsev force-pushed the users/marifamd/amdsmi-packaging-followup branch from 7e2e902 to 799bf48 Compare July 27, 2026 20:48

marifamd added 18 commits July 28, 2026 22:12

docs(amdsmi): clarify the manylinux wheel build directory

6d6ad51

- state that AMDSMI_WHEEL_BUILD_DIR mirrors build_wheel.py's default build path, so its origin is clear at the point of use Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

ci(amdsmi): set PROJECT_DIR as a job-level env var

862bd84

Replace the separate Set Project Directory step with a job-level env entry, since the path is a fixed constant. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

marifamd force-pushed the users/marifamd/amdsmi-packaging-followup branch from 799bf48 to 10d1d26 Compare July 29, 2026 03:13

marifamd requested a review from geomin12 July 29, 2026 06:10

marifamd mentioned this pull request Jul 29, 2026

fix(amdsmi): [ROCM-3941] packaging correctness and coverage follow-ups #9427

Closed

marifamd added 2 commits July 29, 2026 02:52

chore(amdsmi): merge develop into packaging follow-up

705a041

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

dmitrii-galantsev approved these changes Jul 29, 2026

View reviewed changes

gabrpham approved these changes Jul 29, 2026

View reviewed changes

JeniferC99 merged commit e7df082 into develop Jul 29, 2026
38 of 41 checks passed

JeniferC99 deleted the users/marifamd/amdsmi-packaging-followup branch July 29, 2026 18:33

Navigation Menu

Repository navigation

fix(amdsmi): correctness and coverage fixes for the Python packaging rework - #9033

fix(amdsmi): correctness and coverage fixes for the Python packaging rework#9033
JeniferC99 merged 36 commits into
developfrom
users/marifamd/amdsmi-packaging-followup

marifamd commented Jul 22, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Jul 22, 2026 •

edited

Loading

Uh oh!

geomin12 left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

marifamd commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Loader / wrapper

Packaging correctness

Wheel build

Tests

Verification

JIRA ID

Uh oh!

therock-pr-bot Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Policy Checks Passed

Uh oh!

geomin12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

marifamd commented Jul 22, 2026 •

edited

Loading

therock-pr-bot Bot commented Jul 22, 2026 •

edited

Loading