Fix venv creation in Python environments #297628
Conversation
github-actions
bot
added
the
6.topic: python
|
@domenkozar @3541 New PR |
ofborg
bot
added
10.rebuild-darwin-stdenv
ofborg
bot
added
10.rebuild-darwin: 501+
|
I suggest we merge this to staging and give it a go. |
There was a problem hiding this comment.
Correct me if I am wrong, but does this assume that all executables in linked Python packages are Python scripts? This is an assumption that cannot be made; it's not uncommon to have shell scripts in bin/ and binaries can occur as well. Replacing the shebang can be done, but it needs to be checked that it is a Python interpreter, and preferably also exactly the same interpreter. The latter should actually always be the case, if not, we have another problem (e.g. with overriding somewhere).
In time we should aim to not wrap packages at build time.
There was a problem hiding this comment.
No, it only assumes that if files named bin/foo and bin/.foo-wrapped exist, then bin/foo is a wrapper and bin/.foo-wrapped is a python script.
There was a problem hiding this comment.
In time we should aim to not wrap packages at build time.
Yes! I have an idea for how to do that.
FRidh
left a comment
FRidh
left a comment
There was a problem hiding this comment.
Thank you for extending the tests, this gives a lot more trust in the functioning of this approach.
Please do separate the make-wrapper changes in a separate commit as they are a separate unit.
The way we build python environments is subtly broken. A python
environment should be semantically identical to a vanilla Python
installation in, say, /usr/local. The current implementation, however,
differs in two important ways. The first is that it's impossible to use
python packages from the environment in python virtual environments. The
second is that the nix-generated environment appears to be a venv, but
it's not.
This commit changes the way python environments are built:
* When generating wrappers for python executables, we inherit argv[0]
from the wrapper. This causes python to initialize its configuration
in the environment with all the correct paths.
* We remove the sitecustomize.py file from the base python package.
This file was used tweak the python configuration after it was
incorrectly initialized. That's no longer necessary.
The end result is that python environments no longer appear to be venvs,
and behave more like a vanilla python installation. In addition it's
possible to create a venv using an environment and use packages from
both the environment and the venv.
When building a python environment's bin directory, we now detect wrapped python scripts from installed packages, and generate unwrapped copies with the environment's python executable as the interpreter.
|
@FRidh Done! |
|
Did one last manual check, and everything still works as expected. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/nixpkgs-news-a-weekly-recap-for-the-nix-community/42137/5 |
| makeWrapper "$path/bin/$prg" "$out/bin/$prg" --set NIX_PYTHONPREFIX "$out" --set NIX_PYTHONEXECUTABLE ${pythonExecutable} --set NIX_PYTHONPATH ${pythonPath} ${lib.optionalString (!permitUserSite) ''--set PYTHONNOUSERSITE "true"''} ${lib.concatStringsSep " " makeWrapperArgs} | ||
| if [ -f ".$prg-wrapped" ]; then | ||
| echo "#!${pythonExecutable}" > "$out/bin/$prg" | ||
| sed -e '1d' -e '3d' ".$prg-wrapped" >> "$out/bin/$prg" |
There was a problem hiding this comment.
Assuming line 3 here is wrong. In normal wrappers this is line 2 and when injecting the wrapper this could be any line, since it is skipping comments IIRC.
There was a problem hiding this comment.
@cwp : This fix is really important to get proper python environments and it would be really cool to see if you could make another PR which fixes this, as it got reverted. Thanks alot for your work!
|
@cwp this PR got reverted because of the broken |
|
cc @domenkozar |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
As I understand correctly this should fix the |
Partially reverts NixOS#297628. The --resolve-argv0 option in makeWrapper does nothing, and no packages use it.
The --resolve-argv0 option was originally introduced in NixOS#297628 for wrapping Python executables. No other packages currently use it. This commit adjusts the behavior of --resolve-argv0: it now also resolves the dirname of argv0 to its realpath. This ensures that python venvs always resolve sys.base_prefix to the Nix store path (e.g., /nix/store/xxx-python3-env), regardless of how the Nix python environment's python is invoked.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/python-interpreter-from-python-withpackages-has/70287/1 |
11 participants
The problem
The way we build python environments is subtly broken. A python environment should be semantically
identical to a vanilla Python installation in, say,
/usr/local. The current implementation,however, differs in two important ways.
The first is that it's impossible to use python packages from the environment in python virtual
environments. Here's a demonstration:
Another problem is that the nix installation of python appears to python code to be a virtual
environment. The canonical way to detect a Python venv is to compare
sys.prefixtosys.base_prefix. In the base python installation, they will have the same value, but a venv willupdate
sys.prefixto point to the venv.The nix-generated environment appears to be a venv, but it's not. Most notably, it lacks a
pyvenv.cfgfile in the root directory, but the structure of the file tree and the symlinks itcontains are also a bit different. Sophisticated python code that manipulates virtualenvs fails when
run in a nix environment.
The cause
The build machinery for Python packages does some clever manipulation of the Python runtime to make
the python interpreter and python packages that are individually defined as nix derivations
available to python code without copying them all into a single directory the way a vanilla Python
installation does.
The python interpreter packages use the sitecustomize
hook to read environment variables and manipulate the python runtime:
sys.executablebased onNIX_PYTHONEXECUTABLEsys.prefixbased onNIX_PYTHONPREFIXsys.pathbased onNIX_PYTHONPATHWhen a python environment with additional packages is built, a wrapper is created
that invokes the python interpreter with the correct environment supply the sitecustomize module
with the information it needs. This wrapper is what causes the issues mentioned above. The python
interpeter relies very heavily on the path with which it is invoked to initialize its runtime. The
wrapper is quite transparent; it invokes the python interpreter with the "real" path of the
interpreter rather than that of the wrapper. This leads the python runtime to get built in the
context of the bare interpreter, rather than the full environment. The sitecustomize module then
makes some tweaks, but it can't completely compensate for the "incorrect" initialization of the
runtime.
The solution
The fix is pretty simple. Rather than invoking python with the "correct" path to the interpreter,
we invoke it with the path to the wrapper. This causes python to initialize its runtime correctly,
and makes all the sitecustomize machinery unnecessary. Rather than manipulating the
python runtime, we rely on symlinks to make the various nix packages available in the enviornment.
This approach has several benefits:
sitecustomizefor their own purposesTesting
This change includes tweaks to the existing python environment tests, and several new tests.