Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't assume Python modules are in sysconfig.get_path('purelib') #3646

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented Oct 11, 2021

sysconfig.get_path('purelib') returns a location where pure Python modules
should be installed, which is not necessarily where they are loaded from.

Since Fedora 36, sysconfig uses a different install scheme for RPM packages
and for other packages (e.g. installed by pip, setuptools, distutils...).
Without additional context, the scheme for "other" is the default.
Hence, sysconfig.get_path('purelib') returns /usr/local/lib/python3.10/site-packages.
But dracut needs the files installed from RPM packages.

See the announcement about the change in Fedora's Python:
https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/AAGUFQZ4RZDU7KUN4HA43KQJCMSFR3GW/

Instead of directly asking for RPM locations,
we won't assume "where to install stuff" and "where is stuff imported from"
is the same question and we inspect sys.path instead.
To be able to reference specific Python modules,
we use importlib.util.find_spec() (available since Python 3.4).

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=2012513

This change also has 2 added benefits:

  • It loads information from all Python import paths,
    not just 2, so when we add modules from /usr/lib64/python3.10/site-packages,
    this script won't need changes.

  • It does not assume where hardcoded modules are installed,
    so when requests turn into a single-file library requests.py or to an
    extension module (both unlikely, but technically possible),
    or sysconfig becomes a package with init.py,
    this script won't need changes.

@hroncok
Copy link
Contributor Author

hroncok commented Oct 11, 2021

BTW this is a backward-compatible fix.

sysconfig.get_path('purelib') returns a location where pure Python modules
should be installed, which is not necessarily where they are loaded from.

Since Fedora 36, sysconfig uses a different install scheme for RPM packages
and for other packages (e.g. installed by pip, setuptools, distutils...).
Without additional context, the scheme for "other" is the default.
Hence, sysconfig.get_path('purelib') returns /usr/local/lib/python3.10/site-packages.
But dracut needs the files installed from RPM packages.

See the announcement about the change in Fedora's Python:
https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/AAGUFQZ4RZDU7KUN4HA43KQJCMSFR3GW/

Instead of directly asking for RPM locations,
we won't assume "where to install stuff" and "where is stuff imported from"
is the same question and we inspect sys.path instead.
To be able to reference specific Python modules,
we use importlib.util.find_spec() (available since Python 3.4).

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=2012513

This change also has 2 added benefits:

 - It loads information from all Python import paths,
   not just 2, so when we add modules from /usr/lib64/python3.10/site-packages,
   this script won't need changes.

 - It does not assume where hardcoded modules are installed,
   so when requests turn into a single-file library requests.py or to an
   extension module (both unlikely, but technically possible),
   or sysconfig becomes a package with __init__.py,
   this script won't need changes.
@hroncok
Copy link
Contributor Author

hroncok commented Oct 11, 2021

I've amended the commit message slightly, but the force push had no code changes.

@hroncok
Copy link
Contributor Author

hroncok commented Oct 11, 2021

The packit failure is not related.

@VladimirSlavik
Copy link
Contributor

/kickstart-test --testtype smoke

@poncovka poncovka added the master Please, use the `f39` label instead. label Oct 11, 2021
Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me too. Thanks!

Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Looks good to me as well. :)

@M4rtinK M4rtinK merged commit f8af3e7 into rhinstaller:master Oct 11, 2021
@hroncok hroncok deleted the dracut_no_sysconfig_paths branch October 11, 2021 15:30
@hroncok
Copy link
Contributor Author

hroncok commented Oct 11, 2021

Thank you all for the super fast reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master Please, use the `f39` label instead.
Development

Successfully merging this pull request may close these issues.

5 participants