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

Python: improve site_packages_dir handling #28346

Merged
merged 2 commits into from
Jan 14, 2022

Conversation

adamjstewart
Copy link
Member

@adamjstewart adamjstewart commented Jan 10, 2022

This PR includes the following changes. I'll do my best to provide the background and motivation behind these changes. Some of these changes were extracted from #27798, which will be rebased after this is merged.

  • Query sysconfig instead of distutils.sysconfig

In Python 3.10, the distutils module has been deprecated. In Python 3.12, it will be removed entirely. To future-proof our Python build system, we should use sysconfig instead (added in Python 2.7 and 3.2). Also, the Debian/Ubuntu system Python contains a bug where distutils.sysconfig and sysconfig return different purelib/platlib paths. Since build backends like setuptools use sysconfig, we also need to use sysconfig for compatibility.

  • Add both purelib and platlib to the PYTHONPATH

On the majority of systems, purelib and platlib are identical, providing the location where third-party Python libraries are installed. Historically, we've been calling this site_packages_dir. However, when using the system Python on RHEL/CentOS/Fedora, purelib uses lib while platlib uses lib64. Different packages will end up in different directories depending on whether they contain C/C++ code or are written in pure Python. Since Spack has no way of knowing this, we should always add both purelib and platlib to the PYTHONPATH.

  • Fix bug in which dependencies are added to PYTHONPATH

Previously we were recursively adding all build/run/test dependencies. However, this caused build deps of build deps to be added. The correct solution is to add all direct build/run/test deps, and all recursive run deps of those.

External references

Fixes #15304
Fixes #20043
Fixes #22299
Fixes #24076
Fixes #26546
Fixes #27497

@spackbot-app
Copy link

spackbot-app bot commented Jan 10, 2022

Hi @adamjstewart! I noticed that the following package(s) don't yet have maintainers:

  • hoomd-blue
  • py-pyqt5
  • qscintilla

Are you interested in adopting any of these package(s)? If so, simply add the following to the package class:

    maintainers = ['adamjstewart']

If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with spack blame:

$ spack blame hoomd-blue

Thank you for your help! Please don't add maintainers without their consent.

You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer.

becker33
becker33 previously approved these changes Jan 10, 2022
Copy link
Member

@becker33 becker33 left a comment

Choose a reason for hiding this comment

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

Some comments, may or may not be worth addressing but I'll approve and defer to you on whether to make any changes before merging.

lib/spack/spack/build_systems/python.py Show resolved Hide resolved
lib/spack/spack/build_systems/sip.py Outdated Show resolved Hide resolved
@adamjstewart
Copy link
Member Author

@becker33 ready for another round of review. All usage of site_packages_dir has been replaced by either python_purelib or python_platlib. I rather arbitrarily decided between purelib and platlib based on whether the package had non-Python dependencies or not. As to whether these packages follow that rule or not, only CI will tell...

@adamjstewart
Copy link
Member Author

@spackbot run pipeline

@spackbot-app
Copy link

spackbot-app bot commented Jan 11, 2022

I've started that pipeline for you!

becker33
becker33 previously approved these changes Jan 11, 2022
Copy link
Member

@becker33 becker33 left a comment

Choose a reason for hiding this comment

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

Assuming the pipeline comes back successful, I think the platlib vs purelib decisions look reasonable.

@adamjstewart
Copy link
Member Author

Failing build systems check in GitLab CI will be fixed by #28360.

@adamjstewart
Copy link
Member Author

@becker33 @scheibelp is this good to go?

@adamjstewart adamjstewart merged commit e0f0445 into spack:develop Jan 14, 2022
@adamjstewart adamjstewart deleted the packages/purelib-platlib branch January 14, 2022 02:11
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Feb 1, 2022
* Python: improve site_packages_dir handling

* Replace all site_packages_dir with purelib/platlib
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Feb 1, 2022
* Python: improve site_packages_dir handling

* Replace all site_packages_dir with purelib/platlib
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Feb 8, 2022
* Python: improve site_packages_dir handling

* Replace all site_packages_dir with purelib/platlib
EthanS94 pushed a commit to EthanS94/spack that referenced this pull request Feb 17, 2022
* Python: improve site_packages_dir handling

* Replace all site_packages_dir with purelib/platlib
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment