Python: query distutils to find site-packages directory by adamjstewart · Pull Request #24095 · spack/spack

adamjstewart · 2021-06-03T03:09:08Z

Third-party Python libraries may be installed in one of several directories:

lib/pythonX.Y/site-packages for Spack-installed Python
lib64/pythonX.Y/site-packages for system Python on RHEL/CentOS/Fedora
lib/pythonX/dist-packages for system Python on Debian/Ubuntu

Previously, Spack packages were hard-coded to use the (1). Now, we query the Python installation itself and ask it which to use. Ever since #21446 this is how we've been determining where to install Python libraries anyway.

Note: there are still many packages that are hard-coded to use (1). I can change them in this PR, but I don't have the bandwidth to test all of them.

Fixes #24076
Fixes #24526

@skosukhin @permeakra @wspear can you test this?

var/spack/repos/builtin/packages/python/package.py

permeakra

still doesn't work. I think, tuning the glob rule is required. After switching to this pr branch, I got cython libs installed into

/home/permeakra/spack/opt/spack/linux-ubuntu20.04-haswell/gcc-9.3.0/py-cython-0.29.22-iyt6swlafaunff4uawkj7w6adtq2wusm/lib/python3/dist-packages/

but a directory

/home/permeakra/spack/opt/spack/linux-ubuntu20.04-haswell/gcc-9.3.0/py-cython-0.29.22-iyt6swlafaunff4uawkj7w6adtq2wusm/lib/python3.8/site-packages/

still existed and was pulled into PYTHONPATH instead.

adamjstewart · 2021-06-10T04:03:20Z

Can everyone test this latest version? You don't need to rebuild anything, just try to use an external Python with this latest commit. I was able to successfully install py-numpy and the PYTHONPATH was set up correctly.

It looks like some of the unit tests are failing because we install a fake python binary that can't actually be run. Still need to fix this.

wspear · 2021-06-10T16:11:23Z

After double checking that the 'before' state failed, applying this this change made my py-numpy install work. Looks good to me.

adamjstewart · 2021-06-10T16:47:23Z

@alalazo to get the test to pass, I think there's two approaches we could take:

Monkeypatch the test so we don't try to run python to query where site_packages_dir is during activation
Use a try-except in site_packages_dir and fallback to lib/pythonX.Y/site-packages if python crashes

(2) might be helpful in the case where someone is using an external Python and distutils is not installed (I think I remember someone reporting a case like this, was it @michaelkuhn?)

adamjstewart · 2021-06-11T19:26:33Z

@alalazo can you explain the clingo test failure?

lib/spack/spack/build_systems/python.py

alalazo · 2021-06-17T05:28:13Z

can you explain the clingo test failure?

For clingo unit tests we currently bootstrap clingo from sources (with cmake and bison detected as externals to cut on the wall-clock time). Apparently this PR breaks Spack's ability to build an importable clingo module. By just looking at logs it seems here Spack wrongly detects the current Python interpreter to be Python 3.8 instead of Python 2.7 in the failing job.

adamjstewart · 2021-06-17T13:02:50Z

Any idea how the changes in this PR could have broken that? I didn't change any of the detection logic.

alalazo · 2021-06-17T14:31:47Z

Any idea how the changes in this PR could have broken that? I didn't change any of the detection logic.

No, I need to try reproduce the issue.

adamjstewart · 2021-07-16T04:18:12Z

@spackbot run pipeline

spackbot-app · 2021-07-16T04:18:14Z

I've started that pipeline for you!

adamjstewart · 2021-07-16T04:20:09Z

Style tests are now fixed. Failing clingo builds are also now magically fixed? I'm suspicious about that, but won't question it if it helps this PR to get finally merged.

adamjstewart · 2021-07-16T13:08:40Z

@spackbot run pipeline

spackbot-app · 2021-07-16T13:08:42Z

I've started that pipeline for you!

adamjstewart · 2021-07-16T14:45:09Z

@tgamblin can you re-review?

spack#24095 introduced a couple of bugs, which are fixed here: 1. The module path is computed incorrectly for bootstrapped clingo 2. We remove too many paths for `sys.path` in case of failures

#24095 introduced a couple of bugs, which are fixed here: 1. The module path is computed incorrectly for bootstrapped clingo 2. We remove too many paths for `sys.path` in case of failures

spack#24095 introduced a couple of bugs, which are fixed here: 1. The module path is computed incorrectly for bootstrapped clingo 2. We remove too many paths for `sys.path` in case of failures

Add a workflow to test bootstrapping clingo on different platforms so that we can detect changes that break it. Compute `site_packages_dir` in `bootstrap.py` as it was before #24095, until we figure a better way to override that attribute.

adamjstewart · 2021-08-15T01:51:17Z

Unfortunately, it looks like we're going to have to find a different way to do this. Python 3.10 is deprecating distutils and it will be removed completely in 3.12: https://docs.python.org/3.10/whatsnew/3.10.html#distutils

alalazo · 2021-08-16T07:21:59Z

I'll try to think of a solution, let me know if you have ideas. We need something like this with a package override to finish #25371 (the only thing missing there would be bootstrapping pytest for developers).

This is a direct followup to #13557 which caches additional attributes that were added in #24095 that are expensive to compute. I had to reopen #25556 in another PR to invalidate the GitLab CI cache, but see #25556 for prior discussion. ### Before ```console $ time spack env activate . real 2m13.037s user 1m25.584s sys 0m43.654s $ time spack env view regenerate ==> Updating view at /Users/Adam/.spack/.spack-env/view real 16m3.541s user 10m28.892s sys 4m57.816s $ time spack env deactivate real 2m30.974s user 1m38.090s sys 0m49.781s ``` ### After ```console $ time spack env activate . real 0m8.937s user 0m7.323s sys 0m1.074s $ time spack env view regenerate ==> Updating view at /Users/Adam/.spack/.spack-env/view real 2m22.024s user 1m44.739s sys 0m30.717s $ time spack env deactivate real 0m10.398s user 0m8.414s sys 0m1.630s ``` Fixes #25555 Fixes #25541 * Speedup environment activation, part 2 * Only query distutils a single time * Fix KeyError bug * Make vermin happy * Manual memoize * Add comment on cross-compiling * Use platform-specific include directory * Fix multiple bugs * Fix python_inc discrepancy * Fix import tests

adamjstewart added python build-environment run-environment labels Jun 3, 2021

adamjstewart commented Jun 3, 2021

View reviewed changes

var/spack/repos/builtin/packages/python/package.py Outdated Show resolved Hide resolved

permeakra suggested changes Jun 3, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

adamjstewart changed the title ~~Python: handle dist-packages and site-packages~~ Python: query distutils to find site-packages directory Jun 10, 2021

adamjstewart requested a review from alalazo June 10, 2021 03:33

becker33 reviewed Jun 16, 2021

View reviewed changes

lib/spack/spack/build_systems/python.py Show resolved Hide resolved

adamjstewart added 2 commits July 15, 2021 10:35

Fix root directory used in import tests

c2fc71c

Rely on site_packages_dir property

bc2b69e

adamjstewart dismissed tgamblin’s stale review via bc2b69e July 15, 2021 15:35

adamjstewart force-pushed the packages/python branch from 0b601af to bc2b69e Compare July 15, 2021 15:35

tgamblin enabled auto-merge (squash) July 16, 2021 13:19

tgamblin approved these changes Jul 16, 2021

View reviewed changes

tgamblin merged commit c37df94 into spack:develop Jul 16, 2021

adamjstewart deleted the packages/python branch July 16, 2021 15:28

alalazo mentioned this pull request Jul 21, 2021

Bootstrapping from sources is broken on develop #25011

Closed

3 tasks

alalazo mentioned this pull request Jul 21, 2021

Fix bootstrap from sources #25012

Merged

This was referenced Jul 28, 2021

Clingo bootstrap doesn't work on Gentoo #24928

Closed

Test bootstrapping in a workflow #25138

Merged

This was referenced Aug 22, 2021

Environment activation is slow #25555

Closed

Speedup environment activation, part 2 #25556

Closed

Speedup environment activation, part 2 #25633

Merged

This was referenced Sep 16, 2021

Installation issue: sz+python when using external python #25973

Closed

Python: use platform-specific site packages dir #25998

Merged

Conversation

adamjstewart commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

permeakra left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

adamjstewart commented Jun 10, 2021

Uh oh!

wspear commented Jun 10, 2021

Uh oh!

adamjstewart commented Jun 10, 2021

Uh oh!

adamjstewart commented Jun 11, 2021

Uh oh!

Uh oh!

alalazo commented Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamjstewart commented Jun 17, 2021

Uh oh!

alalazo commented Jun 17, 2021

Uh oh!

adamjstewart commented Jul 16, 2021

Uh oh!

spackbot-app bot commented Jul 16, 2021

Uh oh!

adamjstewart commented Jul 16, 2021

Uh oh!

adamjstewart commented Jul 16, 2021

Uh oh!

spackbot-app bot commented Jul 16, 2021

Uh oh!

adamjstewart commented Jul 16, 2021

Uh oh!

adamjstewart commented Aug 15, 2021

Uh oh!

alalazo commented Aug 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

adamjstewart commented Jun 3, 2021 •

edited

Loading

alalazo commented Jun 17, 2021 •

edited

Loading