Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
5677a35 to
59b4220
Compare
9bd04d3 to
1c4301e
Compare
1c4301e to
4dfb170
Compare
c73e259 to
7ec35ea
Compare
8a557f7 to
3a02720
Compare
|
Starting to dislike the state of this PR. Making
There sure is "leaky abstractions" of python-venv, but accessing |
|
And I'm also not convinced that using runtimes for python-venv makes sense at this point in time. We can't express "depend on python-venv under the same conditions and with the same deptype as on python (if it is extended)", both from the perspective of spack dsl ( Runtimes are currently rather compiler centric where you don't have that issue, since %gcc is a node prop and has no edge type. Obviously we have to revisit this when going from %gcc to ^gcc, but I'm not comfortable changing runtime logic now. Maybe if the review came a month earlier. In |
57ccae2 to
c3b131e
Compare
|
Actually on Fedora we also have issues with install layout, which breaks clingo bootstrapping from sources. Went unnoticed because our GHA shows a false positive due to yet another bug. |
|
@tgamblin I'm going to merge this PR, as we said on Friday it's basically fine. We need to start rebuilding the release pipeline rather sooner than later. If there are small adjustments to be made, we can have a following PR and backport that on the release branch. |
|
@tgamblin Your ✔️ appeared on my browser after I hit merge, so I hope you were not working on a commit message 🙂 |
|
This looks like a welcome change in terms of further isolating Python packages in Spack - but I'm having a few issues! I am running on the Within my Spack environment view I see a folder I've tried both Here is the What am I doing wrong? Edit: All commands are within the activated environment. |
For context of this PR see the last bits of this message, this is the 3rd (?) attempt at fixing issues with Python build isolation and extension install directory layout.
This PR adds a layer of indirection to improve build isolation with and without external Python, as well as usability of environment views.
It adds
python-venvas a dependency to all packages thatextends("python"), which has the following advantages:PYTHONPATHis considered in builds, not user / system packagesbin/localsubdirs in Spack install prefixes. Currently bootstrapping ofblack,isort,mypyandflake8is broken on Debian because of this.py-pipthings likepip listwork)For a Python package
py-app, the structure is as follows:py-appdepends both onpython-venvandpython, andpython-venvdepends onpython.On the file system it looks like this:
Views with
python-venv+pythonViews work whether they're symlink, hardlink or copy type (thanks to this PR). They simply flatten the above directory structure:
which constitutes a Python venv. In case Python is external, you conveniently still get a
bin/python3.11pointing to system Python, which before this PR was not the case.spec["python"].commandThis PR additionally makes
spec["python"].commandreturnspec["python-venv"].command. The rationale is that packages in repos we do not own do not pass the underlying python to the build system, which could still result in incorrectly computed install layouts.Other attributes like
libs,headersshould be onpythonanyways and need no change. In the future we can think to makepython-venva "provider wrapper" for a virtualpython, andcpythonanother provider forpython(or something along those lines). That would ultimately just clean up internals.Spack environment views <=> Python venvs
After this PR, the following is a venv equivalent to
python3 -m venv ./view:The next example is a venv equivalent to
python3 -m venv --without-pip ./view:However, an environment that just contains
pythonand nothing else is not a venv:because there would be no
./view/pyvenv.cfgfile. This should in general not be problematic, cause the view also does not contain any Python package that need to be located relative to the view.Bootstrap environment
Bootstrap logic is simplified with this PR. For convenience, Spack automatically re-concretizes and installs bootstrap environments that do not contain
python-venv. That meansspack styleshould just work for users on Debian, no need tospack clean -b.Possible pitfalls
System Python + system Python packages
The
pyvenv.cfgfile contains the following line:which means that you cannot import system Python packages with if you marked
pythonexternal.I think this is new behavior. If necessary, we could put a variant on
python-venv +system_packagesto indicate whether it should include system site packages or not. Such a variant would then also disable build isolation. This PR does not introduce such a variant, I'm just stating this as a workaround if necessary.Context / Alternatives considered
This all started fixing the underlying problem of #31131 properly s.t. we could install setuptools from sources instead of wheel, so that it would be easier to add patches for its vendored distutils, to fix incorrect rpath flags (we currently apply those patches in
pythonitself, but python 3.12 dropped distutils and recommends distutils from setuptools)PythonPipBuilder#40224) I tried a temporary virtual environment for the build, that ensured pip could not execute hooks from unrelated user and system packages. This had two problems:-Sflags to pip to ensure it doesn't see hooks from user / system packages, but that doesn't really work because pip runs sub-processes that do not inherit these flags.Closes #42445
Closes #40601
Closes #37632
Closes #35214
Closes #33923
Closes #32769
Closes #32662
Closes #42445
Fixes #31939
Fixes #31131