Automatically import binary extensions whose suffixes include the python version #15

ian-hurst · 2024-10-18T18:58:41Z

TODO:

Write new tests or update the old ones to cover new functionality.
Update doc-strings where appropriate.
Update or write new documentation in packit/packit.dev.

For an edge case where the current interpreter version differs from that of the rpm library, and where that library's extension suffixes are included in the _rpm.so filename (e.g. _rpm.cpython-39-x86_64-linux-gnu.so on RHEL 9.x), add a new reload_module method that finds the matching .so and imports it directly.

The basic operation is:

gather a list of valid extension suffixes for the rpm library's interpreter, via importlib.machinery.EXTENSION_SUFFIXES
try the import but catch any ModuleNotFoundError
on ModuleNotFoundError, loop through the suffix list, looking for a matching {module}.{suffix}.so on disk
when a match is found, import it as rpm.{module} directly by path

This is probably naive or unwise in some way, but I wanted to take a stab at it so I can use rpm-shim in on RHEL 9.x systems at my company without needing to use the sudo ln workaround

Example from python3.11 on el9 (Oracle 9.x):

DEBUG:rpm-shim:Collected sitepackages and extension suffixes for /usr/libexec/platform-python:
{'sitepackages': ['/usr/local/lib64/python3.9/site-packages',
                  '/usr/local/lib/python3.9/site-packages',
                  '/usr/lib64/python3.9/site-packages',
                  '/usr/lib/python3.9/site-packages'],
 'suffixes': ['.cpython-39-x86_64-linux-gnu.so', '.abi3.so', '.so']}
DEBUG:rpm-shim:Collected sitepackages and extension suffixes for /usr/bin/python3:
{'sitepackages': ['/usr/local/lib64/python3.9/site-packages',
                  '/usr/local/lib/python3.9/site-packages',
                  '/usr/lib64/python3.9/site-packages',
                  '/usr/lib/python3.9/site-packages'],
 'suffixes': ['.cpython-39-x86_64-linux-gnu.so', '.abi3.so', '.so']}
DEBUG:rpm-shim:Collected sitepackages and extension suffixes for /usr/bin/python3.11:
{'sitepackages': ['/usr/local/lib64/python3.11/site-packages',
                  '/usr/local/lib/python3.11/site-packages',
                  '/usr/lib64/python3.11/site-packages',
                  '/usr/lib/python3.11/site-packages'],
 'suffixes': ['.cpython-311-x86_64-linux-gnu.so', '.abi3.so', '.so']}
DEBUG:rpm-shim:Trying /usr/local/lib64/python3.9/site-packages
DEBUG:rpm-shim:Trying /usr/local/lib/python3.9/site-packages
DEBUG:rpm-shim:Trying /usr/lib64/python3.9/site-packages
DEBUG:rpm-shim:Module rpm._rpm not found in /usr/lib64/python3.9/site-packages/rpm, looking for alternative filenames trying valid extension suffixes
DEBUG:rpm-shim:Trying to load rpm._rpm from /usr/lib64/python3.9/site-packages/rpm/_rpm.cpython-39-x86_64-linux-gnu.so
DEBUG:rpm-shim:Loaded rpm._rpm from /usr/lib64/python3.9/site-packages/rpm/_rpm.cpython-39-x86_64-linux-gnu.so
DEBUG:rpm-shim:Reloaded rpm
DEBUG:rpm-shim:Import successfull

softwarefactory-project-zuul · 2024-10-18T19:00:49Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/6b4be3a6a114400daf04880055052416

❌ pre-commit FAILURE in 1m 27s

softwarefactory-project-zuul · 2024-10-18T19:09:53Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/bdf7c2152e61436888096aafa7b5fe72

❌ pre-commit FAILURE in 1m 32s

softwarefactory-project-zuul · 2024-10-18T20:37:38Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/646609e208f646b1bc5cc22c1709138d

❌ pre-commit FAILURE in 1m 25s

softwarefactory-project-zuul · 2024-10-18T20:44:53Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/e58135cd79c84a139adf47891c5e1d5d

❌ pre-commit FAILURE in 1m 27s

softwarefactory-project-zuul · 2024-10-18T20:58:26Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/41ab5710e2a74f638ac7c13ab567e727

✔️ pre-commit SUCCESS in 1m 29s

softwarefactory-project-zuul · 2024-10-18T23:17:06Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/5be05a6b44944a7e9699df95bac279f4

✔️ pre-commit SUCCESS in 1m 29s

softwarefactory-project-zuul · 2024-10-21T15:24:40Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/a2d0847998bc4d7f911532f6b7451173

✔️ pre-commit SUCCESS in 1m 27s

nforro

Thanks, this looks interesting!
I left some comments. It would be also nice to have some test cases for this.

rpm/__init__.py

ian-hurst · 2024-10-21T17:30:17Z

Thanks for the feedback! Going through it now. I'm happy to add testcases if this is ultimately seen as viable

softwarefactory-project-zuul · 2024-10-23T16:05:57Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/8360c49282c247d9a654a1f633156d7e

❌ pre-commit FAILURE in 1m 31s

ian-hurst · 2024-10-23T16:07:22Z

Ok, I reworked this to just loop through *.cpython*.so on ModuleNotFoundError exception. If the glob matches any binary extensions, I just load them. One benefit of this is I no longer need any of the suffix stuff. Debug output ends up:

DEBUG:rpm-shim:Collected sitepackages for /usr/libexec/platform-python:
/usr/local/lib64/python3.9/site-packages
/usr/local/lib/python3.9/site-packages
/usr/lib64/python3.9/site-packages
/usr/lib/python3.9/site-packages
DEBUG:rpm-shim:Collected sitepackages for /usr/bin/python3:
/usr/local/lib64/python3.9/site-packages
/usr/local/lib/python3.9/site-packages
/usr/lib64/python3.9/site-packages
/usr/lib/python3.9/site-packages
DEBUG:rpm-shim:Collected sitepackages for /usr/bin/python3.11:
/usr/local/lib64/python3.11/site-packages
/usr/local/lib/python3.11/site-packages
/usr/lib64/python3.11/site-packages
/usr/lib/python3.11/site-packages
DEBUG:rpm-shim:Trying /usr/lib/python3.11/site-packages
DEBUG:rpm-shim:Trying /usr/lib64/python3.9/site-packages
DEBUG:rpm-shim:The rpm._rpm module was not found in /usr/lib64/python3.9/site-packages/rpm, try loading binary extensions directly
DEBUG:rpm-shim:Found binary extensions:
[PosixPath('/usr/lib64/python3.9/site-packages/rpm/_rpm.cpython-39-x86_64-linux-gnu.so')]
DEBUG:rpm-shim:importing /usr/lib64/python3.9/site-packages/rpm/_rpm.cpython-39-x86_64-linux-gnu.so as rpm._rpm
DEBUG:rpm-shim:imported extensions, reloading rpm
DEBUG:rpm-shim:Import successfull

softwarefactory-project-zuul · 2024-10-23T16:21:02Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/a3adc043bb99435b82bb8ace9b78434f

✔️ pre-commit SUCCESS in 1m 35s

softwarefactory-project-zuul · 2024-10-23T16:25:21Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/5a31cc6223864119aaf7d37d3b89fa2c

✔️ pre-commit SUCCESS in 1m 33s

nforro · 2024-10-23T18:17:01Z

Ok, I reworked this to just loop through *.cpython*.so on ModuleNotFoundError exception. If the glob matches any binary extensions, I just load them. One benefit of this is I no longer need any of the suffix stuff.

While this is more straightforward, I think I liked the previous solution better. Doing it this way it could attempt to load unnecessary modules and that could cause problems, I can imagine a situation where in an unversioned sitepackages path (I think Debian/Ubuntu use those) there are binary modules in multiple versions and loading all of them would be impossible. Getting module name from the exception and constructing filename by trying valid suffixes seems like a better way.

ian-hurst · 2024-10-23T19:31:19Z

While this is more straightforward, I think I liked the previous solution better. Doing it this way it could attempt to load unnecessary modules and that could cause problems, I can imagine a situation where in an unversioned sitepackages path (I think Debian/Ubuntu use those) there are binary modules in multiple versions and loading all of them would be impossible. Getting module name from the exception and constructing filename by trying valid suffixes seems like a better way.

yeah, definitely a drawback to this method. The alternative, I think, would be to do something like reload() in a loop, and break when either:

reload() succeeds, or
the helper has exhausted all binary extensions, and reload() is still failing

If that sounds good (or if there's a better way) I'm happy to go with it.

nforro · 2024-10-23T20:31:59Z

The alternative, I think, would be to do something like reload() in a loop, and break when either:

reload() succeeds, or

the helper has exhausted all binary extensions, and reload() is still failing

If that sounds good (or if there's a better way) I'm happy to go with it.

Yes, that's what I had in mind - loop until there are no more recoverable ModuleNotFoundError exceptions or until it's not possible to recover.

ian-hurst · 2024-10-23T20:36:46Z

Yes, that's what I had in mind - loop until there are no more recoverable ModuleNotFoundError exceptions or until it's not possible to recover.

Alright, super! Working on it now

softwarefactory-project-zuul · 2024-10-24T00:06:44Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/4349007404be4154839f78d61b01b50c

❌ pre-commit FAILURE in 1m 38s

softwarefactory-project-zuul · 2024-10-24T00:11:47Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f6afee9ce72e44748e0b9a8ad8cf402a

❌ pre-commit FAILURE in 1m 33s

nforro

These are all style suggestions, the code looks fine 👍

rpm/__init__.py

ian-hurst · 2024-10-24T16:41:08Z

These are all style suggestions, the code looks fine 👍

Alright, thanks for the feedback! I've taken all the suggested changes now

softwarefactory-project-zuul · 2024-10-24T16:41:11Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f2b367997ba14e6ebe8a4eba1bf38c1a

❌ pre-commit FAILURE in 1m 33s

rpm/__init__.py

softwarefactory-project-zuul · 2024-10-24T16:59:49Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/6379262207d84f35a176c196b4d5482a

✔️ pre-commit SUCCESS in 1m 33s

nforro

Great, thanks. Only some test cases are missing, but I'll try to come up with some, unless you want to do it yourself.

Could you please squash the commits into one?

ian-hurst · 2024-10-24T17:57:58Z

Alright, thanks so much for your review and feedback. I love this little shim and I'm so happy to be able to contribute. As an internet rando, I don't actually have write access, so I can't do the merge step myself.

Only some test cases are missing, but I'll try to come up with some, unless you want to do it yourself.

I'm not familiar with your test framework, but I can think of a few basic designs. The obvious one is the positive test case, on rhel9, install python3.11, create a virtualenv, then import rpm. It might also be good to test that the binary extension import by path works when multiple extensions are used. Can't do that on rhel9 which only has one. But on rhel8 you could create that condition by running sudo rm -f /usr/lib64/python3.6/site-packages/rpm/_{rpm,rpmb,rpms}.so. The .cpython-36m-x86_64-linux-gnu.so files would remain and then you'd be able to test your loop. I did this manually and it works.

But seems like it might be good to have some negative test cases:

if _rpm*.so are absent, import should fail
if _rpm*.so are present, but have invalid suffixes for the original interpreter (probably never occurs in the real world but a good test that things fail)

nforro · 2024-10-25T12:01:59Z

Alright, thanks so much for your review and feedback. I love this little shim and I'm so happy to be able to contribute. As an internet rando, I don't actually have write access, so I can't do the merge step myself.

I will merge the PR once you squash the commits into one (or more if you feel that's necessary, but I think one is fine). Also, could you please follow the guidelines when doing it? 🙂

I'm not familiar with your test framework

There isn't really any framework, just a script that imports rpm, either in a virtualenv or directly on a system.

The obvious one is the positive test case, on rhel9, install python3.11, create a virtualenv, then import rpm.

Yes, that's what I had in mind, and it shouldn't be diffcult to modify existing test case to install and run Python 3.11.

It might also be good to test that the binary extension import by path works when multiple extensions are used. Can't do that on rhel9 which only has one. But on rhel8 you could create that condition by running sudo rm -f /usr/lib64/python3.6/site-packages/rpm/_{rpm,rpmb,rpms}.so. The .cpython-36m-x86_64-linux-gnu.so files would remain and then you'd be able to test your loop. I did this manually and it works.

Nice idea.

ian-hurst · 2024-10-25T14:52:39Z

I will merge the PR once you squash the commits into one (or more if you feel that's necessary, but I think one is fine). Also, could you please follow the guidelines when doing it? 🙂

sure, will do!

The obvious one is the positive test case, on rhel9, install python3.11, create a virtualenv, then import rpm.

Yes, that's what I had in mind, and it shouldn't be diffcult to modify existing test case to install and run Python 3.11.

Ok, I'm happy to write tests. Just looking through the workflow now, the same test is run on all flavors of linux. How would you approach adding a test that only runs on one of them?

nforro · 2024-10-25T15:01:46Z

Ok, I'm happy to write tests.

Thanks!

Just looking through the workflow now, the same test is run on all flavors of linux. How would you approach adding a test that only runs on one of them?

There could be another environment variable that would be passed to the container and the scripts would react accordingly. Though technically for the simplest case of installing and running in Python 3.11 on RHEL9 (ubi9) it can be simply added to INSTALL_DEPS_CMD, tox should run for all interpreters present on the system.

Actually, even the removal of binary modules could be done in INSTALL_DEPS_CMD, there is probably no need to edit tests/entrypoint.sh.

softwarefactory-project-zuul · 2024-10-25T15:50:58Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/b780898245f24445bee720dba9d57757

✔️ pre-commit SUCCESS in 1m 37s

softwarefactory-project-zuul · 2024-10-25T16:12:03Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/cdb145239293426881f49e88e0a151c2

✔️ pre-commit SUCCESS in 1m 26s

softwarefactory-project-zuul · 2024-10-25T16:34:04Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/e28bdd640a5d4903b9fab70b64bc4fbf

✔️ pre-commit SUCCESS in 1m 25s

softwarefactory-project-zuul · 2024-10-25T16:48:31Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/fcceae3c5bb447e1a841cfb34f42e989

✔️ pre-commit SUCCESS in 1m 25s

softwarefactory-project-zuul · 2024-10-25T16:53:46Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f097dee2705647ae99b0240e6b19a4ab

✔️ pre-commit SUCCESS in 1m 40s

softwarefactory-project-zuul · 2024-10-25T17:08:47Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/b58f392926144fc4bd8c14bab1924efc

✔️ pre-commit SUCCESS in 1m 26s

softwarefactory-project-zuul · 2024-10-25T17:16:02Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/ae25cc9e167c45e399f8610a190850aa

✔️ pre-commit SUCCESS in 1m 35s

ian-hurst · 2024-10-25T17:47:30Z

Ok, I have this squashed and a couple scenarios added. Unfortunately (or really, fortunately) the scenario I set up with python36 reveals an issue. _rpms and _rpmb aren't being imported. Why not? Because no exception is raised when they fail. Here is /usr/lib64/python3.6/site-packages/rpm/__init__.py:

# try to import build bits but dont require it
try:
    from rpm._rpmb import *
except ImportError:
    pass

# try to import signing bits but dont require it
try:
    from rpm._rpms import *
except ImportError:
    pass

Nothing is raised, so nothing is caught, and then nothing is helped.

nforro · 2024-10-25T18:29:52Z

Ok, I have this squashed and a couple scenarios added.

Excellent, thanks. The commit message title is a bit long, but that's ok. It shouldn't end with a dot though.

Unfortunately (or really, fortunately) the scenario I set up with python36 reveals an issue. _rpms and _rpmb aren't being imported. Why not? Because no exception is raised when they fail. Here is /usr/lib64/python3.6/site-packages/rpm/__init__.py:
# try to import build bits but dont require it
try:
    from rpm._rpmb import *
except ImportError:
    pass

# try to import signing bits but dont require it
try:
    from rpm._rpms import *
except ImportError:
    pass
Nothing is raised, so nothing is caught, and then nothing is helped.

Ok, it's a bit hackish, but I suppose we could do something like sed -i 's/^ pass/ raise/' /usr/lib64/python3.6/site-packages/rpm/__init__.py after installation.

ian-hurst · 2024-10-25T20:07:22Z

Ok, it's a bit hackish, but I suppose we could do something like sed -i 's/^ pass/ raise/' /usr/lib64/python3.6/site-packages/rpm/__init__.py after installation.

Yeah, and this is still an improvement, since none of the existing python-rpm versions require this. All old versions include the unversioned .so files, and all new versions have just one extension, _rpm, which works. But still, it's agonizing to be so close to a robust solution and then have to settle.

One thing I toyed with was modifying importlib.machinery.EXTENSION_SUFFIXES. If you make that the union of the list from both interpreters, you can use pkgutil to inspect all modules cleanly:

>>> importlib.machinery.EXTENSION_SUFFIXES
['.cpython-36m-x86_64-linux-gnu.so', '.cpython-311-x86_64-linux-gnu.so', '.abi3.so', '.so']
>>> [print(i) for i in pkgutil.iter_modules(['/usr/lib64/python3.6/site-packages/rpm'])]
ModuleInfo(module_finder=FileFinder('/usr/lib64/python3.6/site-packages/rpm'), name='_rpm', ispkg=False)
ModuleInfo(module_finder=FileFinder('/usr/lib64/python3.6/site-packages/rpm'), name='_rpmb', ispkg=False)
ModuleInfo(module_finder=FileFinder('/usr/lib64/python3.6/site-packages/rpm'), name='_rpms', ispkg=False)
ModuleInfo(module_finder=FileFinder('/usr/lib64/python3.6/site-packages/rpm'), name='transaction', ispkg=False)

So you could use that to load _rpmb and _rpms dynamically. But you would have to assume that all modules should be imported. It's really too bad the actual loader machinery doesn't care what's in EXTENSION_SUFFIXES. If it did, we'd be home free. It's clear python just does not want you to use anything but native suffixes, haha.

…hon version

softwarefactory-project-zuul · 2024-10-25T20:21:50Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/fcbc88f3091b49e3af507740bf1b30d3

✔️ pre-commit SUCCESS in 1m 31s

nforro · 2024-10-25T20:34:14Z

Ok, let me merge this. Thanks again for your contribution.

softwarefactory-project-zuul · 2024-10-25T20:36:26Z

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/a0075131843e41f595a87c4eefd25551

✔️ pre-commit SUCCESS in 1m 25s

softwarefactory-project-zuul · 2024-10-25T20:36:29Z

Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry.

ian-hurst · 2024-10-25T20:39:42Z

Ok, let me merge this. Thanks again for your contribution.

My pleasure, thank you so much for taking this change, and for walking me through your process

ian-hurst force-pushed the import_helper branch from 5c48547 to a20dfcc Compare October 18, 2024 23:15

nforro requested changes Oct 21, 2024

View reviewed changes

nforro reviewed Oct 24, 2024

View reviewed changes

rpm/__init__.py Show resolved Hide resolved

nforro approved these changes Oct 24, 2024

View reviewed changes

ian-hurst force-pushed the import_helper branch from 54b6a16 to b3c25d4 Compare October 25, 2024 15:48

ian-hurst force-pushed the import_helper branch from 7fcaee7 to bbe85f4 Compare October 25, 2024 16:51

ian-hurst force-pushed the import_helper branch from b8ff96f to 180dfd1 Compare October 25, 2024 17:13

ian-hurst changed the title ~~Discover extension modules with different suffixes than the current interpreter's~~ Automatically import binary extensions with unexpected suffixes. Oct 25, 2024

Automatically import binary extensions whose suffixes include the pyt…

c06a940

…hon version

ian-hurst force-pushed the import_helper branch from 180dfd1 to c06a940 Compare October 25, 2024 20:19

ian-hurst changed the title ~~Automatically import binary extensions with unexpected suffixes.~~ Automatically import binary extensions whose suffixes include the python version Oct 25, 2024

nforro added the mergeit Zuul, merge it! label Oct 25, 2024

nforro merged commit 15c2742 into packit:main Oct 25, 2024
8 checks passed

Automatically import binary extensions whose suffixes include the python version #15

Automatically import binary extensions whose suffixes include the python version #15

Conversation

ian-hurst commented Oct 18, 2024 • edited Loading

softwarefactory-project-zuul bot commented Oct 18, 2024

softwarefactory-project-zuul bot commented Oct 18, 2024

softwarefactory-project-zuul bot commented Oct 18, 2024

softwarefactory-project-zuul bot commented Oct 18, 2024

softwarefactory-project-zuul bot commented Oct 18, 2024

softwarefactory-project-zuul bot commented Oct 18, 2024

softwarefactory-project-zuul bot commented Oct 21, 2024

nforro left a comment

Choose a reason for hiding this comment

ian-hurst commented Oct 21, 2024

softwarefactory-project-zuul bot commented Oct 23, 2024

ian-hurst commented Oct 23, 2024 • edited Loading

softwarefactory-project-zuul bot commented Oct 23, 2024

softwarefactory-project-zuul bot commented Oct 23, 2024

nforro commented Oct 23, 2024

ian-hurst commented Oct 23, 2024

nforro commented Oct 23, 2024

ian-hurst commented Oct 23, 2024

softwarefactory-project-zuul bot commented Oct 24, 2024

softwarefactory-project-zuul bot commented Oct 24, 2024

nforro left a comment

Choose a reason for hiding this comment

ian-hurst commented Oct 24, 2024

softwarefactory-project-zuul bot commented Oct 24, 2024

softwarefactory-project-zuul bot commented Oct 24, 2024

nforro left a comment

Choose a reason for hiding this comment

ian-hurst commented Oct 24, 2024

nforro commented Oct 25, 2024

ian-hurst commented Oct 25, 2024

nforro commented Oct 25, 2024 • edited Loading

softwarefactory-project-zuul bot commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

ian-hurst commented Oct 25, 2024

nforro commented Oct 25, 2024

ian-hurst commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

nforro commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

softwarefactory-project-zuul bot commented Oct 25, 2024

ian-hurst commented Oct 25, 2024

ian-hurst commented Oct 18, 2024 •

edited

Loading

ian-hurst commented Oct 23, 2024 •

edited

Loading

nforro commented Oct 25, 2024 •

edited

Loading