Skip to content

feat(amdsmi): [ROCM-3941] Add SONAME conflict test and build harness - #7912

Merged
marifamd merged 1 commit into
developfrom
users/juwillia/py-wheel-tests
Jul 29, 2026
Merged

marifamd merged 1 commit into
developfrom
users/juwillia/py-wheel-tests

Conversation

@amd-juwillia

@amd-juwillia amd-juwillia commented Jun 28, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Guard the SONAME-distinctness invariant that lets the system and wheel libraries coexist without double-loading. (The loader/ABI-parity test ships with the loader change in #7914.)

Technical Details

Test (tests/run_amdsmi_pkg_conflict_test.py):

  • Assert the system libamd_smi.so and the wheel libamd_smi_python.so keep distinct SONAMEs so they never double-load in one process.

Harness (tests/amdsmi_build/run_amdsmi_build.py):

  • Extend the local build/install/verify harness mirroring the CI matrix.

Split 3 of 3 from #5726. Depends on #7914 and #7913 and is stacked on them; all target develop. Collapses to 2 files once the parents merge. Merge order: #7914 → #7913 → this.

JIRA ID

Resolves ROCM-3941

Test Plan

  • python3 projects/amdsmi/tests/run_amdsmi_pkg_conflict_test.py

Test Result

  • SONAME-distinctness check passes against a -DBUILD_PYTHON_WHEEL=ON build.

Submission Checklist

@amd-juwillia
amd-juwillia requested a review from a team June 28, 2026 01:29
@amd-juwillia
amd-juwillia requested a review from a team as a code owner June 28, 2026 01:29
@github-actions github-actions Bot added documentation Improvements or additions to documentation project: amdsmi github actions Pull requests that update GitHub Actions code labels Jun 28, 2026
@therock-pr-bot

therock-pr-bot Bot commented Jun 28, 2026 •

Copy link
Copy Markdown

✅ 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.

@amd-juwillia
amd-juwillia force-pushed the users/juwillia/py-wheel-tests branch 2 times, most recently from 3c25834 to eaa8ad4 Compare June 29, 2026 17:37
@amd-juwillia amd-juwillia changed the title feat(amdsmi): [ROCM-3941] Add wheel/system ABI and SONAME conflict tests feat(amdsmi): [ROCM-3941] Add SONAME conflict test and build harness Jun 29, 2026
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-tests branch 2 times, most recently from 7f80227 to f2a2de4 Compare July 6, 2026 16:35
@marifamd
marifamd requested a review from geomin12 July 7, 2026 19:54

@geomin12 geomin12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

comments regards to yml. for a workflow github action step, it should be very clear on what the failure is based on what was run. we shouldn't have to parse through a lot of logs to figure out "oh it was an install error"

i would prefer amdsmi folks to review the amdsmi related code changes

Comment thread .github/workflows/amdsmi-build.yml Resolved
Comment thread .github/workflows/amdsmi-build.yml Resolved
Comment thread .github/workflows/amdsmi-build.yml Resolved
Comment thread .github/workflows/amdsmi-build.yml Resolved
@dmitrii-galantsev
dmitrii-galantsev changed the base branch from develop to users/juwillia/py-wheel-build July 20, 2026 20:53
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-build branch from eac9e6b to dfa7fc8 Compare July 22, 2026 00:11
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from f2a2de4 to ba6e095 Compare July 22, 2026 00:11
@dmitrii-galantsev

dmitrii-galantsev commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Generated by Claude (AI assistant), reviewed by @dmitrii-galantsev.

Top of the ROCM-3941 chain: #7914 -> #7913 -> #7912 (this). Rebased onto the updated #7913; history rewritten and force-pushed, so please re-pull.

Fixes applied to run_amdsmi_pkg_conflict_test.py:

  • Replaced the lexicographic sorted(...)[0] library selection with a numeric version key. Lexically libamd_smi.so.24 sorts before .so.9, so the old code could pick the wrong major when several coexist; it now picks the highest numeric version.
  • Robust readelf handling: guard with shutil.which and catch CalledProcessError instead of an opaque traceback with stderr discarded.
  • Honor ROCM_PATH / ROCM_HOME for the system lib directory instead of hardcoding /opt/rocm.
  • Dropped from __future__ import annotations (not needed; the style guide avoids it).

_version_key verified: with a decoy .so.9 present it correctly selects .so.26.


Update: rebased on the follow-up #7914. The install-verify interpreter fix now lives in #7914 (the base layer) so every stacked PR's CI picks it up; this commit retains only its original run_amdsmi_build.py change (adding abi_compat_output.txt to the scanned results) plus the SONAME conflict-test fixes. CI green: Manylinux Gate, RHEL8, SLES and the RPM/Debian wheel jobs all pass.

@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-build branch from dfa7fc8 to 4718bd1 Compare July 22, 2026 01:01
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from ba6e095 to 02ec224 Compare July 22, 2026 01:01
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-build branch from 4718bd1 to 2122443 Compare July 22, 2026 01:08
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from 02ec224 to 3102982 Compare July 22, 2026 01:08
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-build branch from 2122443 to cccc84f Compare July 22, 2026 03:06
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from 3102982 to 2d1b4b0 Compare July 22, 2026 03:07
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-build branch from cccc84f to 3eafae4 Compare July 22, 2026 04:49
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from 2d1b4b0 to fd22606 Compare July 22, 2026 04:49
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-build branch from 07a1bd3 to 6491aa1 Compare July 23, 2026 03:47
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-tests branch from e95d2a4 to da3a3e3 Compare July 23, 2026 03:47
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-build branch from 6491aa1 to 9222b97 Compare July 24, 2026 14:04
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-tests branch from da3a3e3 to 067bbc8 Compare July 24, 2026 14:04
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-build branch from 9222b97 to fa32a56 Compare July 24, 2026 14:31
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-tests branch from 067bbc8 to 5521e21 Compare July 24, 2026 14:31
@marifamd
marifamd requested review from geomin12 and marifamd July 27, 2026 00:10
Base automatically changed from users/juwillia/py-wheel-build to develop July 27, 2026 00:38
@marifamd
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
marifamd force-pushed the users/juwillia/py-wheel-tests branch from 6258090 to af25984 Compare July 27, 2026 19:52
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/juwillia/py-wheel-tests branch from af25984 to 6434cec Compare July 27, 2026 20:48
@dmitrii-galantsev

dmitrii-galantsev commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

ci failing, latest rebase should have fixed it: #9061
see the same problem pop up in #7914

@dmitrii-galantsev
dmitrii-galantsev marked this pull request as draft July 27, 2026 22:22
@dmitrii-galantsev

dmitrii-galantsev commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

converted to draft because we're reverting #7914 and #7913

@marifamd
marifamd marked this pull request as ready for review July 28, 2026 22:58
- Add run_amdsmi_pkg_conflict_test.py asserting the system libamd_smi.so
  and the wheel libamd_smi_python.so keep distinct SONAMEs so they never
  double-load in one process.
- Extend run_amdsmi_build.py for the build/install/verify harness.

Signed-off-by: Justin Williams <Justin.Williams@amd.com>
@marifamd
marifamd force-pushed the users/juwillia/py-wheel-tests branch from 6434cec to b859b76 Compare July 28, 2026 22:59
@marifamd
marifamd merged commit 548e257 into develop Jul 29, 2026
40 of 41 checks passed
@marifamd
marifamd deleted the users/juwillia/py-wheel-tests branch July 29, 2026 03:00
JeniferC99 pushed a commit that referenced this pull request Jul 29, 2026
…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>
marifamd pushed a commit that referenced this pull request Aug 8, 2026
…7912)

Add tests/run_amdsmi_pkg_conflict_test.py, which asserts that two AMD SMI
packages built with different SOVERSIONs do not co-own the same
libamd_smi.so.<major>, so installing one over the other cannot silently
break the other's loader.

Wire the ABI-compat output into the build harness: summarize_results in
run_amdsmi_build.py now scans abi_compat_output.txt alongside the existing
integration/unit/perf test outputs so an ABI regression surfaces in the CI
summary.

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
marifamd added a commit that referenced this pull request Aug 8, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation github actions Pull requests that update GitHub Actions code organization: ROCm project: amdsmi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants