Repository navigation
fix(amdsmi): isolate the tests package scriptlets and require the distro install matrix - #9110
Merged
ammallya merged 9 commits intoAug 11, 2026
Conversation
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 24, 2026 14:05
79dcf9b to
1c078d6
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 24, 2026 14:05
eff9ea2 to
8dca306
Compare
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 24, 2026 14:31
1c078d6 to
139becc
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 24, 2026 14:31
8dca306 to
8f4526d
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 24, 2026 15:48
139becc to
a2997ad
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 24, 2026 15:48
8f4526d to
cd56f28
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 27, 2026 07:57
a2997ad to
d52c7bb
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 27, 2026 07:57
b953eee to
624a03f
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
2 times, most recently
from
July 27, 2026 15:16
d52c7bb to
b9aad05
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 27, 2026 15:16
624a03f to
12ca26e
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 27, 2026 15:37
b9aad05 to
31d09ea
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 27, 2026 15:37
12ca26e to
021ac87
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 27, 2026 15:54
31d09ea to
bcde5a9
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
2 times, most recently
from
July 27, 2026 19:58
e13b797 to
8290c33
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 27, 2026 19:58
bcde5a9 to
b3db3eb
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 27, 2026 20:48
b3db3eb to
0148360
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 27, 2026 20:48
8290c33 to
84fb513
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
July 29, 2026 06:10
84fb513 to
16cc3e3
Compare
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 29, 2026 06:10
0148360 to
44bc5f3
Compare
gabrpham
approved these changes
Jul 29, 2026
marifamd
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
July 30, 2026 17:42
c7a5d9f to
cdb896e
Compare
dmitrii-galantsev
added a commit
that referenced
this pull request
Aug 5, 2026
amd-smi-lib-tests depends on amd-smi-lib, so removing or upgrading the tests package must never disturb the main package's state. #9110 fixes that; this adds the static regression guard it did not ship with. The assertions target the invariant rather than any one remedy: the tests component may carry its own scriptlets, or simply be left without the main package's, and either satisfies them. Verified green against #9110's implementation and red against the pre-fix templates (15 failures). Records the CPack 3.28.3 finding behind the RPM half of the bug: an unprefixed CPACK_RPM_<KIND> scriptlet variable is copied into every component RPM, so a global assignment has to be paired with a CPACK_RPM_TESTS_<KIND> override to keep it out of the tests package. JIRA ID: ROCM-3941
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the amdsmi packaging and ABI gate behavior by isolating the tests-package uninstall hooks from the main package, tightening the ABI compatibility test, and scoping Python discovery to wheel-specific builds.
Changes:
- Prevent
amd-smi-lib-testsinstall/uninstall from inheriting main-package RPM/DEB scriptlets that could break a still-installed main package. - Make the Python ABI gate fail if the binding scan returns zero matches (avoids a silent no-op gate).
- Require
find_package(Python3)only whenBUILD_PYTHON_WHEEL=ON, and add safer fallbacks for determiningPython3_SITELIB.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| projects/amdsmi/tests/python/test_abi_compat.py | Adds a non-empty assertion to ensure the ABI scan doesn’t silently match nothing. |
| projects/amdsmi/RPM/tests-preun.in | Introduces tests-only RPM pre-uninstall cleanup for test __pycache__. |
| projects/amdsmi/RPM/tests-noop.in | Adds a no-op RPM scriptlet to prevent tests component from running main scriptlets. |
| projects/amdsmi/py-interface/CMakeLists.txt | Scopes Python discovery to wheel builds and hardens sitelib detection/fallbacks. |
| projects/amdsmi/DEBIAN/amd-smi-lib-tests/prerm.in | Replaces inherited main-package cleanup with tests-only __pycache__ removal on remove. |
| projects/amdsmi/CMakeLists.txt | Wires per-component RPM scriptlets for the tests component to avoid inheriting global hooks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dmitrii-galantsev
added a commit
that referenced
this pull request
Aug 5, 2026
amd-smi-lib-tests depends on amd-smi-lib, so removing or upgrading the tests package must never disturb the main package's state. #9110 fixes that; this adds the static regression guard it did not ship with. The assertions target the invariant rather than any one remedy: the tests component may carry its own scriptlets, or simply be left without the main package's, and either satisfies them. Verified green against #9110's implementation and red against the pre-fix templates (15 failures). Records the CPack 3.28.3 finding behind the RPM half of the bug: an unprefixed CPACK_RPM_<KIND> scriptlet variable is copied into every component RPM, so a global assignment has to be paired with a CPACK_RPM_TESTS_<KIND> override to keep it out of the tests package. JIRA ID: ROCM-3941
dmitrii-galantsev
added a commit
that referenced
this pull request
Aug 5, 2026
amd-smi-lib-tests depends on amd-smi-lib, so removing or upgrading the tests package must never disturb the main package's state. #9110 fixes that; this adds the static regression guard it did not ship with. The assertions target the invariant rather than any one remedy: the tests component may carry its own scriptlets, or simply be left without the main package's, and either satisfies them. Verified green against #9110's implementation and red against the pre-fix templates (15 failures). Records the CPack 3.28.3 finding behind the RPM half of the bug: an unprefixed CPACK_RPM_<KIND> scriptlet variable is copied into every component RPM, so a global assignment has to be paired with a CPACK_RPM_TESTS_<KIND> override to keep it out of the tests package. JIRA ID: ROCM-3941
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
August 5, 2026 17:51
3821035 to
3b3e9ad
Compare
dmitrii-galantsev
added a commit
that referenced
this pull request
Aug 5, 2026
amd-smi-lib-tests depends on amd-smi-lib, so removing or upgrading the tests package must never disturb the main package's state. #9110 fixes that; this adds the static regression guard it did not ship with. The assertions target the invariant rather than any one remedy: the tests component may carry its own scriptlets, or simply be left without the main package's, and either satisfies them. Verified green against #9110's implementation and red against the pre-fix templates (15 failures). Records the CPack 3.28.3 finding behind the RPM half of the bug: an unprefixed CPACK_RPM_<KIND> scriptlet variable is copied into every component RPM, so a global assignment has to be paired with a CPACK_RPM_TESTS_<KIND> override to keep it out of the tests package. JIRA ID: ROCM-3941
dmitrii-galantsev
approved these changes
Aug 5, 2026
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
August 5, 2026 22:15
ce25324 to
6fb98a0
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
August 5, 2026 22:15
3b3e9ad to
6d51660
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
August 5, 2026 22:19
6fb98a0 to
f5e6a5f
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
August 5, 2026 22:19
6d51660 to
ede02ff
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
August 7, 2026 17:08
f5e6a5f to
c211c63
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
August 7, 2026 17:08
ede02ff to
1ae025d
Compare
Packaging install-path and multi-Python-version test coverage for the AMD SMI Python packaging rework. - 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; 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
The RPM declares a versioned interpreter dependency whose form differs by distro family: `python(abi) = X.Y` on RHEL, but `pythonXY` with no dot (e.g. python311) on SLES/openSUSE. The GPU build matrix never exercises the SLES form, so a broken dependency token could ship unnoticed. Build the RPM in an openSUSE Leap 15.6 container and assert the pythonXY requirement both appears and resolves to an installable provider, then check the module ships on an import path. No GPU required. JIRA ID: ROCM-3941
Without timeout-minutes the job inherits the six-hour default, so a hung zypper refresh or a stalled container ties up a runner for the rest of the day instead of failing fast. Forty-five minutes matches the other packaging jobs, which build the project in a container the same way. JIRA ID: ROCM-3941
…iptlets - the global CPACK_RPM_*_SCRIPT_FILE vars stamp the main scriptlets onto every component RPM, so removing amd-smi-lib-tests ran the main package's ldconfig and cleanup and corrupted the still-installed main package - override the tests RPM hooks per-component: a tests-only pre-uninstall that clears just its own __pycache__, and no-op post/post-uninstall - rewrite the DEB tests prerm to the same tests-only cleanup so it no longer removes the main package's linker entry, bytecode, doc or symlink - clean only on a full erase, never on upgrade Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> (cherry picked from commit 64f9625)
- the no-clang branch copies the committed wrapper with plain cmake and never uses the interpreter, but still ran find_package(Python3 REQUIRED), so a C-only build failed at configure before reaching the sitelib step - gate the find_package on BUILD_PYTHON_WHEEL so a C-only build with an explicit AMDSMI_SYSTEM_PYTHON_SITELIB no longer needs an interpreter Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> (cherry picked from commit 16cc3e3)
- test_unguarded_bindings_are_stable subtracts a set from STABLE_SYMBOLS, so an empty scan trivially passes and the gate silently checks nothing - assert the scan is non-empty first, so a wrapper layout change that breaks _scan_unguarded_bindings() fails loudly Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> (cherry picked from commit af45ac4)
Requiring Python3 only for the wheel build means find_package(Python3) no longer runs for a plain C build: BUILD_PYTHON_WHEEL and BUILD_WRAPPER both default to OFF, so the default distro package build takes the NOT GOOD_CLANG_FOUND branch and never calls it. Python3_SITELIB is then undefined. The module-destination detection still fell back to that variable when the site-packages query failed, which would set AMDSMI_SYSTEM_PYTHON_SITELIB to "" and install the module to the prefix root -- the silent misplacement this detection exists to catch, reported only by a warning that trails off after "falling back to Python3_SITELIB=". Fall back only when Python3_SITELIB actually holds a value, and fail loudly otherwise, matching the neighbouring branch that already refuses to guess a destination no interpreter has on sys.path. JIRA ID: ROCM-3941
Scoping only the tests package left the same hole open for asan. An unprefixed CPACK_RPM_<KIND>_SCRIPT_FILE is copied into every component RPM, and on the Debian side a component without its own control-extra inherits the generic one, so amd-smi-lib-asan still shipped the main package's postinst/prerm/post/preun/postun. That matters even though asan installs under its own prefix: the state those scripts manage -- /etc/ld.so.conf.d/x86_64-libamd_smi_lib.conf, /var/log/amd_smi_lib and /etc/logrotate.d/amd_smi.conf -- is at absolute paths shared by every variant. Installing the asan package repoints the main package's linker registration at the asan library, and removing it deletes that registration, the log directory and the logrotate config out from under a still-installed amd-smi-lib. Name the owner explicitly for every script instead, which is what rocm-core, clr and rdc do. The name is the component GROUP, not the component: generic_package_post() puts dev into the "runtime" group, so CPack emits one package per group and a CPACK_*_DEV_* variable is silently ignored. Anything not named ships nothing, so a component added later cannot inherit by accident. Dropping the generic Debian control-extra also makes the tests package deterministic: it previously depended on two prerm files colliding by filename in the control archive. RPM/tests-noop.in is no longer needed now that nothing leaks, so it goes. Verified with a three-component CPack project: dev carries three scriptlets, tests one, and a component named nowhere carries none. JIRA ID: ROCM-3941
The tests pre-uninstall built its path from @CPACK_PACKAGING_INSTALL_PREFIX@, the prefix baked in at configure time, while every other RPM scriptlet uses $RPM_INSTALL_PREFIX0, the prefix the package was actually installed under. For a relocated install the two differ, so the cleanup would look in the wrong tree and leave the generated __pycache__ behind. JIRA ID: ROCM-3941
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-followup-sles
branch
from
August 10, 2026 16:50
c211c63 to
4dd278e
Compare
dmitrii-galantsev
force-pushed
the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
from
August 10, 2026 16:50
1ae025d to
cc983b3
Compare
Base automatically changed from
users/marifamd/amdsmi-packaging-followup-sles
to
develop
August 11, 2026 21:54
Collaborator
|
Flaky github fetch sources error, build and test for smi are good |
ammallya
deleted the
users/marifamd/amdsmi-packaging-scriptlet-isolation
branch
August 11, 2026 21:55
ammallya
pushed a commit
that referenced
this pull request
Aug 12, 2026
…ation (#9695) Stacked on #9110 — this targets that branch, not `develop`. ``` develop └─ #9034 packaging install-path and multi-Python coverage └─ #9035 SLES RPM interpreter dependency check └─ #9110 component scriptlet isolation └─ #9695 (this PR) ``` ## What this is The regression tests #9110 didn't ship with, plus fixes for several unrelated defects in the packaging test suite that let real breakage through. Nothing here changes #9110's fix — this PR adds tests and CI only, apart from the two behavior changes called out at the bottom. ## Regression coverage for #9110 #9110 stops `amd-smi-lib-tests` from tearing down the still-installed `amd-smi-lib`. Two tests pin it: - **`test_packaging_scriptlets.py`** — static guard on the templates and the CPack wiring. Written against the *invariant* rather than one particular remedy, so it accepts either "give the tests component its own scriptlets" or "scope the globals to the owner". - **`run_amdsmi_component_removal_test.py`** + a workflow step — installs both packages in the distro container, removes only the tests package, and asserts the main package survives: still installed, linker registration intact, module still importable and still owned by the main package, log directory and logrotate config untouched. The static guard alone is not sufficient. It can only confirm which CPack variables were *written*, never that CPack *honors* them, so it passes against packaging that is genuinely broken. The container test is what closes that gap. ## Also fixed — the test oracles themselves - **CPack path check** accepted any path merely *containing* `site-packages`. Verified false positives on real synthetic packages: `/usr/lib/python3/not-site-packages/…`, `/opt/acme/site-packages/…`, and a wrong Python minor. It now matches against the target interpreter's `site.getsitepackages()`, and picks that interpreter the way packaging does (`/usr/libexec/platform-python`, then `/usr/bin/python3`) rather than `sys.executable`, which differs on the RHEL family. It also parses `dpkg -c` without mangling symlink entries or paths containing spaces. - **Upgrade round trip** verified a bare `import amdsmi`. A pip wheel under `/usr/local` shadows the packaged module, so all three phases could validate the *same unchanged wheel* while the system package really did change — reproduced on Ubuntu 22.04 and AlmaLinux 9. It now requires the imported module to be owned by the package under test, comparing the owning package name **exactly**: a substring test accepts `amd-smi-lib-tests`, whose name contains the main package's. OLD and NEW must also differ, so equal versions can't no-op past the test. - **Build harness** ran that round trip *after* installing the wheel; moved ahead of the install and wheel stages. - **Post-install** now warns, non-fatally and destroying nothing, when a pip-installed `amdsmi` shadows the packaged module. - **`test_run_amdsmi_build.py`** never registered the module in `sys.modules` before `exec_module`, which `@dataclass` requires on Python 3.12+. All 16 of its tests were uncollectable. ## Testing The new unit tests are hermetic — no root, Docker, network or GPU — and run on Python 3.6 through 3.14 via the version harness. Both new guards fail on the pre-fix code for the right reason, not merely because a helper is missing: the scriptlet guard fails on the pre-fix templates, and the CPack path guard drives the old CLI through a fake `dpkg` shim and reports the unimportable path it used to accept. Each commit is self-contained and was verified individually, so the stack bisects cleanly. ## Notes for review - Two behavior changes only: the post-install warning, which deletes nothing and always exits 0, and stage ordering inside a test harness. Everything else is test coverage or CI. - The upgrade-round-trip ownership check uses exact package-name comparison rather than the `"amd-smi-lib"` prefix suggested in review, because that string is also a prefix of `amd-smi-lib-tests`. The sibling case is covered by a test. ## JIRA ID ROCM-3941
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Isolate the component maintainer scripts and tighten the ABI/wheel build gates. Stacks on #9035.
Scriptlet ownership
fix: stop the tests package from running the main package scriptlets— theamd-smi-lib-testspackage inherited the main package's post/preun scriptlets, so removing the tests package could run the main package'sld.so.conf.dcleanup and corrupt the main install.fix: give every component its own maintainer scripts— scoping only the tests package left the same hole open forasan. An unprefixedCPACK_RPM_<KIND>_SCRIPT_FILEis copied into every component RPM, and a Debian component without its own control-extra inherits the generic one. The scripts manage absolute paths shared by every variant (/etc/ld.so.conf.d,/var/log/amd_smi_lib,/etc/logrotate.d), so an inheriting component clobbers the main package even under a different prefix. Name the owning component group for every script instead; anything not named ships nothing, so a component added later cannot inherit by accident.fix: resolve the tests cleanup path at install time— the tests pre-uninstall built its path from the configure-time prefix instead of$RPM_INSTALL_PREFIX0, so a relocated install cleaned the wrong tree.Build gates
test: fail the ABI gate when the binding scan matches nothing— the ABI compatibility gate silently passed when the ctypes binding scan matched zero symbols (e.g. a generator rename); it now fails loudly.build: only require Python3 for the wheel build— scope thefind_package(Python3)requirement toBUILD_PYTHON_WHEEL=ONso the default (non-wheel) build does not fail on a missing Python3.fix: don't fall back to an unset Python3_SITELIB— withfind_package(Python3)no longer running for a plain C build, the module-destination fallback toPython3_SITELIBwould install to the prefix root. Fall back only when it holds a value, and fail loudly otherwise.JIRA ID
ROCM-3941