Repository navigation
ci(amdsmi): [ROCM-3941] Python packaging rework - #7914
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
64baf4b to
9507a30
Compare
a2685e3 to
986c7ac
Compare
geomin12
left a comment
There was a problem hiding this comment.
yml file changes lgtm, but i would get some AMDSMI folks to review as majority of the changes are amdsmi related
amd-nicknick
left a comment
There was a problem hiding this comment.
Native package building needs some fixes.
.so fallback logic & pip wheel handling LGTM. Thanks!
986c7ac to
8def682
Compare
Restructured the ROCM-3941 work into a proper PR chain so each PR shows a single commit: #7914 (this) -> #7913 -> #7912. History rewritten and force-pushed; please re-pull your local branches. Fixes applied to this commit on top of the original: RPM/CLI interpreter mismatch (main fix). The module installed into a version-specific RPM dependency. Interpreter dependency is now distro-aware and matches the detected default interpreter: Minor. Type hints on Verified in Docker against real consumers (not CI): built on el8 with the harness switching build-python to 3.12, installed on stock el8 (default 3.6), confirmed Update (follow-up round, CI-verified): the initial Fixes added to this commit:
Verified in Docker across el8/el9/SLES/Azure/Debian and confirmed green in this PR's CI: AlmaLinux8, RHEL8/9/10, SLES, AzureLinux3, Debian10, Ubuntu20/22 all pass (AlmaLinux8 and RHEL8 were previously failing). Approach cross-checked against the Fedora/RHEL Python packaging guidelines. Update 2 (cross-consumer regression fix): the previous round moved the Investigated the full consumer set (PyTorch, vLLM, JAX, Triton, DeepSpeed, omnistat, distro packagers, and every in-repo consumer). Conclusion: site-packages is the correct primary (PyTorch/vLLM and all major ML consumers use plain Fix (additive, keeps site-packages primary):
Verified in Docker: (a) TheRock artifact layout — module present at |
6481cf1 to
71e8dde
Compare
71e8dde to
99c5988
Compare
|
#9061 resolved, ci should work |
geomin12
left a comment
There was a problem hiding this comment.
some cleanup comments and also some AI generated comments about order priority
…ll, wheel build - Replace the ROCm-path-walking resolver in amdsmi_wrapper.py (and tools/generator.py, regenerated in lockstep) with a flat 3-case loader: AMDSMI_LIB_OVERRIDE, a bundled libamd_smi_python.so, then system libamd_smi.so; a _MissingLibrary sentinel keeps import tolerant. - Install the amdsmi module directly into the system python's site-packages via install(DIRECTORY) instead of pip and a .pth file; drop pip plumbing from the DEBIAN/RPM post/pre scripts and remove the python3-pip/setuptools/wheel package dependencies and setup.py.in. - Add BUILD_PYTHON_WHEEL (default OFF, requires BUILD_SHARED_LIBS): build a SONAME-isolated libamd_smi_python.so via -Bsymbolic-functions and a python_wheel target that flips the loader's system fallback off with tools/disable_system_fallback.py so the wheel never loads a system lib. - Gate the test runner on a staged in-tree copy (common.py) so the tests resolve the dist-packages-installed module instead of rejecting it. - Add test_abi_compat.py covering the loader contract and run it in the build-and-test matrix. - Pin the amdsmi_cli shebang to /usr/bin/python3, drop the dead cpu-handles guard, and strip the system library in Release builds. Signed-off-by: Justin Williams <Justin.Williams@amd.com>
- The RPM %postun removed /etc/ld.so.conf.d/x86_64-libamd_smi_lib.conf on both final erase ($1=0) and upgrade ($1=1). - RPM runs the new package's %post (which creates the entry) before the old package's %postun, so removing it on upgrade deleted the new package's own linker registration, leaving libamd_smi.so unresolvable until the next ldconfig run. - Only remove it on final erase ($1=0); the DEB prerm is unaffected since dpkg runs the new postinst after the old prerm. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
…-3.8 - main() was annotated with list[str] (PEP 585), which is only valid at runtime on Python 3.9+. On the 3.6-3.8 interpreters AMD SMI still supports it raised TypeError: 'type' object is not subscriptable at import. - The wheel build runs this tool under the system python, so an older default interpreter broke the build. - Use typing.List[str], which works on every supported version. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- seven near-identical steps each cat one result log, which is noisy in the workflow and in the Actions log - collapse them into a single step that prints each log inside a collapsible ::group:: block keyed by a readable label Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- the packaging rework made the CLI import site-packages first and only fall back to share/amd_smi, reversing the priority from #3082 - on a multi-version ROCm host the CLI must load the amdsmi library it shipped with, not another version's copy earlier on sys.path - resolve share/amd_smi (ROCM_PATH or CLI-relative) before importing, and fall back to site-packages only when no share copy exists Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- sparse-checkout only projects/amdsmi and the workflows dir instead of the whole monorepo, so the checkout stays cheap - keep each test-result dump as its own step so a single failure is directly attributable instead of buried in one grouped step - resolve the project and runner script by their fixed sparse-checkout paths rather than a find plus existence guard - drop the Uninstall step; the job runs in a --rm container that is discarded after the run - note why the artifact glob matches the 99999 local version Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- the upload feeds no downstream job; document that it exists for manual reviewer download so the step is not mistaken for dead weight Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- the wheel-location check accepted any of site-packages, dist-packages or /opt/rocm, so it passed even when a system copy shadowed the pip install and never proved the pip module wins - assert import amdsmi resolves under pip's reported install location and not the /opt/rocm system copy, so a coexisting system module fails the check instead of masking it - document the CLI's share -> pip -> system resolution order, distinct from a user script where the pip install must win Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- continue-on-error let every distro leg report success even when the package failed to build, install or import - drop it so any failure on the self-hosted runner CI is visible; these runners are expected to be stable Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
964ecfa to
3252cec
Compare
- the check asserted the pip install wins, but the CI container and ROCm installs set PYTHONPATH=/opt/rocm/share/amd_smi, which always precedes site-packages, so import amdsmi resolved to the system copy and the assertion could never pass on RHEL/SLES/Ubuntu22 - run the check with PYTHONPATH cleared so it tests the real scripting scenario (a bare interpreter where pip must win), not the environment's path config Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
geomin12
left a comment
There was a problem hiding this comment.
lgtm! thanks for addressing comments!
Reverts the manylinux wheel CI job (#7913) and the Python packaging rework (#7914): flat loader, dist-packages install, and wheel build. Restores the pre-rework loader in amdsmi_wrapper.py / tools/generator.py, the pip-based DEBIAN/RPM install scriptlets, and setup.py.in; removes pin_shebang.cmake, tools/disable_system_fallback.py, and tests/python/test_abi_compat.py. Accounts for changes that landed on develop after the rework merged: - Keeps the libamd_smi.so.27 SONAME and its CHANGELOG entry from the 27.0.0 version bump. The restored loader hardcoded the SONAME, so the literal is bumped from .so.26 to .so.27 in both generator.py and the regenerated wrapper to match AMDSMI_LIB_VERSION_MAJOR. - Keeps the ras --cper --json no-entries fix (#8819) and its CHANGELOG entry; only the rework's own CHANGELOG entry is removed. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Reintroduces the manylinux wheel CI job (#7913) and the Python packaging rework (#7914): flat loader, dist-packages install, and wheel build. Combined here with the amd-smi CLI shebang fixes so the rework and its follow-up corrections land as one reviewable change. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
feat(amdsmi): [ROCM-3941] Python packaging rework: flat loader, dist-packages install, wheel build - Replace the ROCm-path-walking resolver in amdsmi_wrapper.py (and tools/generator.py, regenerated in lockstep) with a flat loader: AMDSMI_LIB_OVERRIDE, a bundled libamd_smi_python.so, the SONAME relative to the wrapper (TheRock share/amd_smi), then the system libamd_smi.so; a _MissingLibrary sentinel keeps import tolerant, and a present-but-unloadable relative library falls through to the system lookup. - Install the amdsmi module directly into the system python's site-packages via install(DIRECTORY) instead of pip and a .pth file; drop pip plumbing from the DEBIAN/RPM scripts and the python3-pip/setuptools/wheel deps. - Add BUILD_PYTHON_WHEEL (default OFF, requires BUILD_SHARED_LIBS): build a SONAME-isolated libamd_smi_python.so via -Bsymbolic-functions and a python_wheel target that disables the loader's system fallback so the wheel never loads a system lib. - Resolve the CLI import from the installed share/amd_smi copy first, so amd-smi uses the modules it shipped with on a multi-version ROCm host. - Add test_abi_compat.py covering the loader contract and run it in the build-and-test matrix; keep disable_system_fallback.py importable on Python 3.6-3.8. - Keep the ld.so.conf.d linker registration across RPM upgrades (only remove it on a full erase). - CI: sparse-checkout projects/amdsmi, individual per-result steps, drop the redundant find/uninstall steps, remove continue-on-error so any distro failure is visible, and assert the pip wheel (with PYTHONPATH cleared) takes priority for scripting. --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>
Reverts the manylinux wheel CI job (#7913) and the Python packaging rework (#7914): flat loader, dist-packages install, and wheel build. Restores the pre-rework loader in amdsmi_wrapper.py / tools/generator.py, the pip-based DEBIAN/RPM install scriptlets, and setup.py.in; removes pin_shebang.cmake, tools/disable_system_fallback.py, and tests/python/test_abi_compat.py. Accounts for changes that landed on develop after the rework merged: - Keeps the libamd_smi.so.27 SONAME and its CHANGELOG entry from the 27.0.0 version bump. The restored loader hardcoded the SONAME, so the literal is bumped from .so.26 to .so.27 in both generator.py and the regenerated wrapper to match AMDSMI_LIB_VERSION_MAJOR. - Keeps the ras --cper --json no-entries fix (#8819) and its CHANGELOG entry; only the rework's own CHANGELOG entry is removed. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Relands the manylinux wheel CI job (#7913) and the Python packaging rework (#7914) after they were reverted in #9339, together with the follow-up amd-smi CLI shebang fixes so the rework and its corrections land as one change. Packaging rework (reintroduced): - Install the amdsmi module into the system python's site-packages via install(DIRECTORY) instead of pip + a .pth file; drop the pip plumbing and python3-pip/setuptools/wheel deps from the DEBIAN/RPM scriptlets. - Replace the ROCm-path-walking resolver in amdsmi_wrapper.py (and tools/generator.py, regenerated in lockstep) with a flat loader: AMDSMI_LIB_OVERRIDE, a bundled libamd_smi_python.so, then the system libamd_smi.so; a _MissingLibrary sentinel keeps import tolerant. - Add BUILD_PYTHON_WHEEL (default OFF): build a SONAME-isolated libamd_smi_python.so and a wheel that disables the loader's system fallback. - Stage the module under share/amd_smi for ROCM_PATH consumers and TheRock. - Add test_abi_compat.py and the manylinux wheel CI job. CLI shebang fixes: - Use a portable "#!/usr/bin/env python3" shebang for the amd-smi CLI so it runs on relocatable installs with no /usr/bin/python3 (TheRock containers) and on wheel runners where /usr/bin/python3 lacks the amdsmi module. - Stop pinning the CLI shebang to a fixed interpreter: remove pin_shebang.cmake and the AMDSMI_CLI_SHEBANG_INTERP derivation. The pinned absolute path became a hard RPM "Requires:" (e.g. /usr/libexec/platform-python3.6) that RHEL10 and SLES16 could not satisfy; the RPM now depends only on python(abi). - Resolve the env-form shebang in the install-verify harness instead of running "/usr/bin/env -c". Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Relands the manylinux wheel CI job (#7913) and the Python packaging rework (#7914) after they were reverted in #9339, together with the follow-up amd-smi CLI shebang fixes so the rework and its corrections land as one change. Packaging rework (reintroduced): - Install the amdsmi module into the system python's site-packages via install(DIRECTORY) instead of pip + a .pth file; drop the pip plumbing and python3-pip/setuptools/wheel deps from the DEBIAN/RPM scriptlets. - Replace the ROCm-path-walking resolver in amdsmi_wrapper.py (and tools/generator.py, regenerated in lockstep) with a flat loader: AMDSMI_LIB_OVERRIDE, a bundled libamd_smi_python.so, then the system libamd_smi.so; a _MissingLibrary sentinel keeps import tolerant. - Add BUILD_PYTHON_WHEEL (default OFF): build a SONAME-isolated libamd_smi_python.so and a wheel that disables the loader's system fallback. - Stage the module under share/amd_smi for ROCM_PATH consumers and TheRock. - Add test_abi_compat.py and the manylinux wheel CI job. CLI shebang fixes: - Use a portable "#!/usr/bin/env python3" shebang for the amd-smi CLI so it runs on relocatable installs with no /usr/bin/python3 (TheRock containers) and on wheel runners where /usr/bin/python3 lacks the amdsmi module. - Stop pinning the CLI shebang to a fixed interpreter: remove pin_shebang.cmake and the AMDSMI_CLI_SHEBANG_INTERP derivation. The pinned absolute path became a hard RPM "Requires:" (e.g. /usr/libexec/platform-python3.6) that RHEL10 and SLES16 could not satisfy; the RPM now depends only on python(abi). - Resolve the env-form shebang in the install-verify harness instead of running "/usr/bin/env -c". Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Motivation
amd-smishipped its Python module by having the system.deb/.rpmpostinst callpip installand drop anamdsmi.pth, with a ~200-line ROCm-path-walking loader. This drops allpip/.pthplumbing, installs the module directly into the system interpreter'ssite-packages, and adds an opt-in SONAME-isolated wheel build so a PyPI wheel can coexist in-process with the system library.Technical Details
Loader (
py-interface/amdsmi_wrapper.py,tools/generator.py— regenerated in lockstep):AMDSMI_LIB_OVERRIDE, a bundledlibamd_smi_python.so, then systemlibamd_smi.so; a_MissingLibrarysentinel keeps import tolerant.Install layout (
py-interface/CMakeLists.txt):/usr/bin/python3for its real on-sys.pathdir andinstall(DIRECTORY ...)the module there, excludinglibamd_smi*.so.Packaging (
DEBIAN/postinst.in,prerm.in,RPM/post.in,preun.in,CMakeLists.txt):.pth; drop thepython3-pip/setuptools/wheeldeps andsetup.py.in.Wheel build (
src/CMakeLists.txt,py-interface/CMakeLists.txt,tools/disable_system_fallback.py,CMakeLists.txt):BUILD_PYTHON_WHEEL(default OFF, requiresBUILD_SHARED_LIBS): a SONAME-isolatedlibamd_smi_python.solinked-Bsymbolic-functions, and apython_wheeltarget that flips_AMDSMI_ALLOW_SYSTEM_FALLBACKtoFalsein the staged wrapper so the wheel never loads a system lib.CLI / build (
amdsmi_cli/amdsmi_cli.py,amdsmi_init.py,src/CMakeLists.txt):/usr/bin/python3; drop a dead guard; strip the system lib in Release.JIRA ID
Resolves ROCM-3941
Test Plan
python3 projects/amdsmi/tools/build_wheel.py --project-dir projects/amdsmi --no-repair(mirrors the manylinux CI).Test Result
build_wheel.pybuilds clean; the wheel bundles onlylibamd_smi_python.soand ships_AMDSMI_ALLOW_SYSTEM_FALLBACK = False.import amdsmi+amdsmi_init()succeed via/usr/bin/python3with no.pth/pip across the AMD CI images.Submission Checklist