From 234bb31f611f43f8b744b305ab82035de637aaca Mon Sep 17 00:00:00 2001 From: Colin Putney Date: Thu, 21 Mar 2024 19:22:03 -0600 Subject: [PATCH] Fix venv creation in Python environments 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. --- .../make-binary-wrapper.sh | 46 ++++++++++ .../build-support/setup-hooks/make-wrapper.sh | 4 + .../python/cpython/2.7/default.nix | 2 - .../interpreters/python/cpython/default.nix | 3 +- .../interpreters/python/pypy/default.nix | 3 - .../interpreters/python/pypy/prebuilt.nix | 3 - .../interpreters/python/pypy/prebuilt_2_7.nix | 3 - .../interpreters/python/sitecustomize.py | 39 -------- .../development/interpreters/python/tests.nix | 89 +++++++++++++++---- .../tests/test_environments/test_python.py | 2 +- 10 files changed, 123 insertions(+), 71 deletions(-) delete mode 100644 pkgs/development/interpreters/python/sitecustomize.py diff --git a/pkgs/build-support/setup-hooks/make-binary-wrapper/make-binary-wrapper.sh b/pkgs/build-support/setup-hooks/make-binary-wrapper/make-binary-wrapper.sh index 6cd01f6bf6307..3948342a36fc9 100644 --- a/pkgs/build-support/setup-hooks/make-binary-wrapper/make-binary-wrapper.sh +++ b/pkgs/build-support/setup-hooks/make-binary-wrapper/make-binary-wrapper.sh @@ -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 @@ -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[*]} @@ -169,6 +171,12 @@ 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' ;; @@ -176,6 +184,7 @@ makeCWrapper() { 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) */" @@ -183,9 +192,11 @@ makeCWrapper() { printf '%s\n' "#include " [ -z "$uses_assert" ] || printf '%s\n' "#include " [ -z "$uses_stdio" ] || printf '%s\n' "#include " + [ -z "$uses_string" ] || printf '%s\n' "#include " [ -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' "}" @@ -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 diff --git a/pkgs/build-support/setup-hooks/make-wrapper.sh b/pkgs/build-support/setup-hooks/make-wrapper.sh index 11b332bfc3eb2..cba158bd31ea9 100644 --- a/pkgs/build-support/setup-hooks/make-wrapper.sh +++ b/pkgs/build-support/setup-hooks/make-wrapper.sh @@ -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 @@ -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 diff --git a/pkgs/development/interpreters/python/cpython/2.7/default.nix b/pkgs/development/interpreters/python/cpython/2.7/default.nix index dda254fca3894..86b09fa877685 100644 --- a/pkgs/development/interpreters/python/cpython/2.7/default.nix +++ b/pkgs/development/interpreters/python/cpython/2.7/default.nix @@ -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 '' diff --git a/pkgs/development/interpreters/python/cpython/default.nix b/pkgs/development/interpreters/python/cpython/default.nix index 0191517aa9ef7..e2f6a1b42bbeb 100644 --- a/pkgs/development/interpreters/python/cpython/default.nix +++ b/pkgs/development/interpreters/python/cpython/default.nix @@ -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. diff --git a/pkgs/development/interpreters/python/pypy/default.nix b/pkgs/development/interpreters/python/pypy/default.nix index c64c65df350ef..c11ca7330c2cb 100644 --- a/pkgs/development/interpreters/python/pypy/default.nix +++ b/pkgs/development/interpreters/python/pypy/default.nix @@ -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 ''; diff --git a/pkgs/development/interpreters/python/pypy/prebuilt.nix b/pkgs/development/interpreters/python/pypy/prebuilt.nix index 4b47c642eca4c..70f8711ef0866 100644 --- a/pkgs/development/interpreters/python/pypy/prebuilt.nix +++ b/pkgs/development/interpreters/python/pypy/prebuilt.nix @@ -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 ''; diff --git a/pkgs/development/interpreters/python/pypy/prebuilt_2_7.nix b/pkgs/development/interpreters/python/pypy/prebuilt_2_7.nix index 37a06f9f61ed5..f0b60c2333f56 100644 --- a/pkgs/development/interpreters/python/pypy/prebuilt_2_7.nix +++ b/pkgs/development/interpreters/python/pypy/prebuilt_2_7.nix @@ -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 ''; diff --git a/pkgs/development/interpreters/python/sitecustomize.py b/pkgs/development/interpreters/python/sitecustomize.py deleted file mode 100644 index d79a4696d8eaa..0000000000000 --- a/pkgs/development/interpreters/python/sitecustomize.py +++ /dev/null @@ -1,39 +0,0 @@ -""" -This is a Nix-specific module for discovering modules built with Nix. - -The module recursively adds paths that are on `NIX_PYTHONPATH` to `sys.path`. In -order to process possible `.pth` files `site.addsitedir` is used. - -The paths listed in `PYTHONPATH` are added to `sys.path` afterwards, but they -will be added before the entries we add here and thus take precedence. - -Note the `NIX_PYTHONPATH` environment variable is unset in order to prevent leakage. - -Similarly, this module listens to the environment variable `NIX_PYTHONEXECUTABLE` -and sets `sys.executable` to its value. -""" -import site -import sys -import os -import functools - -paths = os.environ.pop('NIX_PYTHONPATH', None) -if paths: - functools.reduce(lambda k, p: site.addsitedir(p, k), paths.split(':'), site._init_pathinfo()) - -# Check whether we are in a venv or virtualenv. -# For Python 3 we check whether our `base_prefix` is different from our current `prefix`. -# For Python 2 we check whether the non-standard `real_prefix` is set. -# https://stackoverflow.com/questions/1871549/determine-if-python-is-running-inside-virtualenv -in_venv = (sys.version_info.major == 3 and sys.prefix != sys.base_prefix) or (sys.version_info.major == 2 and hasattr(sys, "real_prefix")) - -if not in_venv: - executable = os.environ.pop('NIX_PYTHONEXECUTABLE', None) - prefix = os.environ.pop('NIX_PYTHONPREFIX', None) - - if 'PYTHONEXECUTABLE' not in os.environ and executable is not None: - sys.executable = executable - if prefix is not None: - # Sysconfig does not like it when sys.prefix is set to None - sys.prefix = sys.exec_prefix = prefix - site.PREFIXES.insert(0, prefix) diff --git a/pkgs/development/interpreters/python/tests.nix b/pkgs/development/interpreters/python/tests.nix index df4484f9ec68e..0251a903a7ae8 100644 --- a/pkgs/development/interpreters/python/tests.nix +++ b/pkgs/development/interpreters/python/tests.nix @@ -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"; @@ -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"; @@ -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 ''; diff --git a/pkgs/development/interpreters/python/tests/test_environments/test_python.py b/pkgs/development/interpreters/python/tests/test_environments/test_python.py index 0fc4b8a9e91c1..538273f65dbc7 100644 --- a/pkgs/development/interpreters/python/tests/test_environments/test_python.py +++ b/pkgs/development/interpreters/python/tests/test_environments/test_python.py @@ -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)