Skip to content

Commit

Permalink
Fix venv creation in Python environments
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cwp committed Mar 22, 2024
1 parent b727e4c commit 234bb31
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ assertExecutable() {
# (if unset or empty, defaults to EXECUTABLE)
# --inherit-argv0 : the executable inherits argv0 from the wrapper.
# (use instead of --argv0 '$0')
# --resolve-argv0 : if argv0 doesn't include a / character, resolve it against PATH
# --set VAR VAL : add VAR with value VAL to the executable's environment
# --set-default VAR VAL : like --set, but only adds VAR if not already set in
# the environment
Expand Down Expand Up @@ -87,6 +88,7 @@ makeDocumentedCWrapper() {
makeCWrapper() {
local argv0 inherit_argv0 n params cmd main flagsBefore flagsAfter flags executable length
local uses_prefix uses_suffix uses_assert uses_assert_success uses_stdio uses_asprintf
local resolve_path
executable=$(escapeStringLiteral "$1")
params=("$@")
length=${#params[*]}
Expand Down Expand Up @@ -169,23 +171,32 @@ makeCWrapper() {
# Whichever comes last of --argv0 and --inherit-argv0 wins
inherit_argv0=1
;;
--resolve-argv0)
# this gets processed after other argv0 flags
uses_stdio=1
uses_string=1
resolve_argv0=1
;;
*) # Using an error macro, we will make sure the compiler gives an understandable error message
main="$main#error makeCWrapper: Unknown argument ${p}"$'\n'
;;
esac
done
[[ -z "$flagsBefore" && -z "$flagsAfter" ]] || main="$main"${main:+$'\n'}$(addFlags "$flagsBefore" "$flagsAfter")$'\n'$'\n'
[ -z "$inherit_argv0" ] && main="${main}argv[0] = \"${argv0:-${executable}}\";"$'\n'
[ -z "$resolve_argv0" ] || main="${main}argv[0] = resolve_argv0(argv[0]);"$'\n'
main="${main}return execv(\"${executable}\", argv);"$'\n'

[ -z "$uses_asprintf" ] || printf '%s\n' "#define _GNU_SOURCE /* See feature_test_macros(7) */"
printf '%s\n' "#include <unistd.h>"
printf '%s\n' "#include <stdlib.h>"
[ -z "$uses_assert" ] || printf '%s\n' "#include <assert.h>"
[ -z "$uses_stdio" ] || printf '%s\n' "#include <stdio.h>"
[ -z "$uses_string" ] || printf '%s\n' "#include <string.h>"
[ -z "$uses_assert_success" ] || printf '\n%s\n' "#define assert_success(e) do { if ((e) < 0) { perror(#e); abort(); } } while (0)"
[ -z "$uses_prefix" ] || printf '\n%s\n' "$(setEnvPrefixFn)"
[ -z "$uses_suffix" ] || printf '\n%s\n' "$(setEnvSuffixFn)"
[ -z "$resolve_argv0" ] || printf '\n%s\n' "$(resolveArgv0Fn)"
printf '\n%s' "int main(int argc, char **argv) {"
printf '\n%s' "$(indent4 "$main")"
printf '\n%s\n' "}"
Expand Down Expand Up @@ -338,6 +349,41 @@ void set_env_suffix(char *env, char *sep, char *suffix) {
"
}

resolveArgv0Fn() {
printf '%s' "\
char *resolve_argv0(char *argv0) {
if (strchr(argv0, '/') != NULL) {
return argv0;
}
char *path = getenv(\"PATH\");
if (path == NULL) {
return argv0;
}
char *path_copy = strdup(path);
if (path_copy == NULL) {
return argv0;
}
char *dir = strtok(path_copy, \":\");
while (dir != NULL) {
char *candidate = malloc(strlen(dir) + strlen(argv0) + 2);
if (candidate == NULL) {
free(path_copy);
return argv0;
}
sprintf(candidate, \"%s/%s\", dir, argv0);
if (access(candidate, X_OK) == 0) {
free(path_copy);
return candidate;
}
free(candidate);
dir = strtok(NULL, \":\");
}
free(path_copy);
return argv0;
}
"
}

# Embed a C string which shows up as readable text in the compiled binary wrapper,
# giving instructions for recreating the wrapper.
# Keep in sync with makeBinaryWrapper.extractCmd
Expand Down
4 changes: 4 additions & 0 deletions pkgs/build-support/setup-hooks/make-wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ assertExecutable() {
# (if unset or empty, defaults to EXECUTABLE)
# --inherit-argv0 : the executable inherits argv0 from the wrapper.
# (use instead of --argv0 '$0')
# --resolve-argv0 : if argv0 doesn't include a / character, resolve it against PATH
# --set VAR VAL : add VAR with value VAL to the executable's environment
# --set-default VAR VAL : like --set, but only adds VAR if not already set in
# the environment
Expand Down Expand Up @@ -177,6 +178,9 @@ makeShellWrapper() {
elif [[ "$p" == "--inherit-argv0" ]]; then
# Whichever comes last of --argv0 and --inherit-argv0 wins
argv0='$0'
elif [[ "$p" == "--resolve-argv0" ]]; then
# this is noop in shell wrappers, since bash will always resolve $0
resolve_argv0=1
else
die "makeWrapper doesn't understand the arg $p"
fi
Expand Down
2 changes: 0 additions & 2 deletions pkgs/development/interpreters/python/cpython/2.7/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,6 @@ in with passthru; stdenv.mkDerivation ({
inherit passthru;

postFixup = ''
# Include a sitecustomize.py file. Note it causes an error when it's in postInstall with 2.7.
cp ${../../sitecustomize.py} $out/${sitePackages}/sitecustomize.py
'' + lib.optionalString strip2to3 ''
rm -R $out/bin/2to3 $out/lib/python*/lib2to3
'' + lib.optionalString stripConfig ''
Expand Down
3 changes: 1 addition & 2 deletions pkgs/development/interpreters/python/cpython/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,7 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
# Strip tests
rm -R $out/lib/python*/test $out/lib/python*/**/test{,s}
'' + optionalString includeSiteCustomize ''
# Include a sitecustomize.py file
cp ${../sitecustomize.py} $out/${sitePackages}/sitecustomize.py
'' + optionalString stripBytecode ''
# Determinism: deterministic bytecode
# First we delete all old bytecode.
Expand Down
3 changes: 0 additions & 3 deletions pkgs/development/interpreters/python/pypy/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,6 @@ in with passthru; stdenv.mkDerivation rec {
ln -s $out/${executable}-c/include $out/include/${libPrefix}
ln -s $out/${executable}-c/lib-python/${if isPy3k then "3" else pythonVersion} $out/lib/${libPrefix}
# Include a sitecustomize.py file
cp ${../sitecustomize.py} $out/${if isPy38OrNewer then sitePackages else "lib/${libPrefix}/${sitePackages}"}/sitecustomize.py
runHook postInstall
'';

Expand Down
3 changes: 0 additions & 3 deletions pkgs/development/interpreters/python/pypy/prebuilt.nix
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,6 @@ in with passthru; stdenv.mkDerivation {
echo "Removing bytecode"
find . -name "__pycache__" -type d -depth -delete
# Include a sitecustomize.py file
cp ${../sitecustomize.py} $out/${sitePackages}/sitecustomize.py
runHook postInstall
'';

Expand Down
3 changes: 0 additions & 3 deletions pkgs/development/interpreters/python/pypy/prebuilt_2_7.nix
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ in with passthru; stdenv.mkDerivation {
echo "Removing bytecode"
find . -name "__pycache__" -type d -depth -delete
# Include a sitecustomize.py file
cp ${../sitecustomize.py} $out/${sitePackages}/sitecustomize.py
runHook postInstall
'';

Expand Down
39 changes: 0 additions & 39 deletions pkgs/development/interpreters/python/sitecustomize.py

This file was deleted.

89 changes: 71 additions & 18 deletions pkgs/development/interpreters/python/tests.nix
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,21 @@ let
is_virtualenv = "False";
};
} // lib.optionalAttrs (!python.isPyPy) {
# Use virtualenv from a Nix env.
nixenv-virtualenv = rec {
env = runCommand "${python.name}-virtualenv" {} ''
${pythonVirtualEnv.interpreter} -m virtualenv venv
mv venv $out
# Use virtualenv with symlinks from a Nix env.
nixenv-virtualenv-links = rec {
env = runCommand "${python.name}-virtualenv-links" {} ''
${pythonVirtualEnv.interpreter} -m virtualenv --system-site-packages --symlinks --no-seed $out
'';
interpreter = "${env}/bin/${python.executable}";
is_venv = "False";
is_nixenv = "True";
is_virtualenv = "True";
};
} // lib.optionalAttrs (!python.isPyPy) {
# Use virtualenv with copies from a Nix env.
nixenv-virtualenv-copies = rec {
env = runCommand "${python.name}-virtualenv-copies" {} ''
${pythonVirtualEnv.interpreter} -m virtualenv --system-site-packages --copies --no-seed $out
'';
interpreter = "${env}/bin/${python.executable}";
is_venv = "False";
Expand All @@ -59,27 +69,48 @@ let
is_nixenv = "True";
is_virtualenv = "False";
};
} // lib.optionalAttrs (python.isPy3k && (!python.isPyPy)) {
# Venv built using plain Python
} // lib.optionalAttrs (python.pythonAtLeast "3.8" && (!python.isPyPy)) {
# Venv built using links to plain Python
# Python 2 does not support venv
# TODO: PyPy executable name is incorrect, it should be pypy-c or pypy-3c instead of pypy and pypy3.
plain-venv = rec {
env = runCommand "${python.name}-venv" {} ''
${python.interpreter} -m venv $out
plain-venv-links = rec {
env = runCommand "${python.name}-venv-links" {} ''
${python.interpreter} -m venv --system-site-packages --symlinks --without-pip $out
'';
interpreter = "${env}/bin/${python.executable}";
is_venv = "True";
is_nixenv = "False";
is_virtualenv = "False";
};
} // lib.optionalAttrs (python.pythonAtLeast "3.8" && (!python.isPyPy)) {
# Venv built using copies from plain Python
# Python 2 does not support venv
# TODO: PyPy executable name is incorrect, it should be pypy-c or pypy-3c instead of pypy and pypy3.
plain-venv-copies = rec {
env = runCommand "${python.name}-venv-copies" {} ''
${python.interpreter} -m venv --system-site-packages --copies --without-pip $out
'';
interpreter = "${env}/bin/${python.executable}";
is_venv = "True";
is_nixenv = "False";
is_virtualenv = "False";
};

} // lib.optionalAttrs (python.pythonAtLeast "3.8") {
# Venv built using Python Nix environment (python.buildEnv)
# TODO: Cannot create venv from a nix env
# Error: Command '['/nix/store/ddc8nqx73pda86ibvhzdmvdsqmwnbjf7-python3-3.7.6-venv/bin/python3.7', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.
nixenv-venv = rec {
env = runCommand "${python.name}-venv" {} ''
${pythonEnv.interpreter} -m venv $out
nixenv-venv-links = rec {
env = runCommand "${python.name}-venv-links" {} ''
${pythonEnv.interpreter} -m venv --system-site-packages --symlinks --without-pip $out
'';
interpreter = "${env}/bin/${pythonEnv.executable}";
is_venv = "True";
is_nixenv = "True";
is_virtualenv = "False";
};
} // lib.optionalAttrs (python.pythonAtLeast "3.8") {
# Venv built using Python Nix environment (python.buildEnv)
nixenv-venv-copies = rec {
env = runCommand "${python.name}-venv-copies" {} ''
${pythonEnv.interpreter} -m venv --system-site-packages --copies --without-pip $out
'';
interpreter = "${env}/bin/${pythonEnv.executable}";
is_venv = "True";
Expand All @@ -91,11 +122,33 @@ let
testfun = name: attrs: runCommand "${python.name}-tests-${name}" ({
inherit (python) pythonVersion;
} // attrs) ''
mkdir $out
# set up the test files
cp -r ${./tests/test_environments} tests
chmod -R +w tests
substituteAllInPlace tests/test_python.py
${attrs.interpreter} -m unittest discover --verbose tests #/test_python.py
mkdir $out
# run the tests by invoking the interpreter via full path
echo "absolute path: ${attrs.interpreter}"
${attrs.interpreter} -m unittest discover --verbose tests 2>&1 | tee "$out/full.txt"
# run the tests by invoking the interpreter via $PATH
export PATH="$(dirname ${attrs.interpreter}):$PATH"
echo "PATH: $(basename ${attrs.interpreter})"
"$(basename ${attrs.interpreter})" -m unittest discover --verbose tests 2>&1 | tee "$out/path.txt"
# make sure we get the right path when invoking through a result link
ln -s "${attrs.env}" result
relative="result/bin/$(basename ${attrs.interpreter})"
expected="$PWD/$relative"
actual="$(./$relative -c "import sys; print(sys.executable)" | tee "$out/result.txt")"
if [ "$actual" != "$expected" ]; then
echo "expected $expected, got $actual"
exit 1
fi
# if we got this far, the tests passed
touch $out/success
'';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_site_prefix(self):

@unittest.skipIf(IS_PYPY or sys.version_info.major==2, "Python 2 does not have base_prefix")
def test_base_prefix(self):
if IS_VENV or IS_NIXENV or IS_VIRTUALENV:
if IS_VENV or IS_VIRTUALENV:
self.assertNotEqual(sys.prefix, sys.base_prefix)
else:
self.assertEqual(sys.prefix, sys.base_prefix)
Expand Down

1 comment on commit 234bb31

@mdorman
Copy link
Contributor

@mdorman mdorman commented on 234bb31 Apr 3, 2024

Choose a reason for hiding this comment

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

@adisbladis I think that either this commit or 9611885 has broken mailman. When systemd attempts to start the primary process, it spits out the error:

Apr 03 15:05:29 lists systemd[1]: Starting GNU Mailman Master Process...
Apr 03 15:05:29 lists mailman[1242]: Traceback (most recent call last):
Apr 03 15:05:29 lists mailman[1242]:   File "/nix/store/x2kibscz685qvf1y6rfzbfg723ybq0xi-python3.11-mailman-3.3.9/bin/mailman", line 5, in <module>
Apr 03 15:05:29 lists mailman[1242]:     from mailman.bin.mailman import main
Apr 03 15:05:29 lists mailman[1242]: ModuleNotFoundError: No module named 'mailman'
Apr 03 15:05:29 lists systemd[1]: mailman.service: Control process exited, code=exited, status=1/FAILURE
Apr 03 15:05:29 lists systemd[1]: mailman.service: Failed with result 'exit-code'.
Apr 03 15:05:29 lists systemd[1]: Failed to start GNU Mailman Master Process.

I haven't been able to do a full bisect, but I have narrowed down the failure to a commit between aeb48d594a30d47d52dc4e2365d02e8b74471277 and b649792a709bb5946bdbde416f2e4af612e5dbcd, and your two commits are very obvious potential culprits.

Perhaps you could look into this failure?

Please sign in to comment.