From b16ec1e59ff7d92b9f06dad526caf9adf6575f5d Mon Sep 17 00:00:00 2001 From: John Sirois Date: Tue, 15 Mar 2022 10:00:47 -0700 Subject: [PATCH] Fix venv creation to ignore ambient PEX env vars. Previously any ambient PEX env vars infected the `pex` venv script that was written out, fouling its defaults. Fixes #1668 --- pex/pex.py | 9 ++- pex/venv/pex.py | 7 +- tests/integration/venv_ITs/test_issue_1668.py | 74 +++++++++++++++++++ tests/test_pip.py | 36 +++++++-- 4 files changed, 114 insertions(+), 12 deletions(-) create mode 100644 tests/integration/venv_ITs/test_issue_1668.py diff --git a/pex/pex.py b/pex/pex.py index da5ad23aa..6b8045d3e 100644 --- a/pex/pex.py +++ b/pex/pex.py @@ -79,11 +79,12 @@ def __init__( if verify_entry_point: self._do_entry_point_verification() - def pex_info(self): - # type: () -> PexInfo + def pex_info(self, include_env_overrides=True): + # type: (bool) -> PexInfo pex_info = self._pex_info.copy() - pex_info.update(self._pex_info_overrides) - pex_info.merge_pex_path(self._vars.PEX_PATH) + if include_env_overrides: + pex_info.update(self._pex_info_overrides) + pex_info.merge_pex_path(self._vars.PEX_PATH) return pex_info @property diff --git a/pex/venv/pex.py b/pex/venv/pex.py index dd4eab910..2acf6b56e 100644 --- a/pex/venv/pex.py +++ b/pex/venv/pex.py @@ -286,9 +286,14 @@ def _populate_sources( ): # type: (...) -> Iterator[Tuple[str, str]] + # We want the venv at rest to reflect the PEX it was created from at rest; as such we use the + # PEX's at-rest PEX-INFO to perform the layout. The venv can then be executed with various PEX + # environment variables in-play that it respects (e.g.: PEX_EXTRA_SYS_PATH, PEX_INTERPRETER, + # PEX_MODULE, etc.). + pex_info = pex.pex_info(include_env_overrides=False) + # Since the pex.path() is ~always outside our control (outside ~/.pex), we copy all PEX user # sources into the venv. - pex_info = pex.pex_info() for src, dst in _copytree( src=PEXEnvironment.mount(pex.path()).path, dst=venv.site_packages_dir, diff --git a/tests/integration/venv_ITs/test_issue_1668.py b/tests/integration/venv_ITs/test_issue_1668.py new file mode 100644 index 000000000..10a9e362b --- /dev/null +++ b/tests/integration/venv_ITs/test_issue_1668.py @@ -0,0 +1,74 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +import os.path +import subprocess + +from pex.testing import PY37, ensure_python_interpreter, make_env, pex_project_dir, run_pex_command +from pex.typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any + + +def assert_venv_runtime_env_vars_ignored_during_create( + tmpdir, # type: Any + pex_venv, # type: bool +): + # type: (...) -> None + + pex_pex = os.path.join(str(tmpdir), "pex.pex") + args = [pex_project_dir(), "-c", "pex", "-o", pex_pex, "--no-strip-pex-env", "--disable-cache"] + if pex_venv: + args.append("--venv") + run_pex_command(args=args).assert_success() + + py37 = ensure_python_interpreter(PY37) + pex_root = os.path.join(str(tmpdir), "pex_root") + lock = os.path.join(str(tmpdir), "lock.json") + subprocess.check_call( + args=[ + py37, + pex_pex, + "lock", + "create", + "ansicolors==1.1.8", + "-o", + lock, + "--pex-root", + pex_root, + ], + env=make_env(PEX_SCRIPT="pex3"), + ) + ansicolors_path = ( + subprocess.check_output( + args=[ + py37, + pex_pex, + "--pex-root", + pex_root, + "--runtime-pex-root", + pex_root, + "--lock", + lock, + "--", + "-c", + "import colors; print(colors.__file__)", + ] + ) + .decode("utf-8") + .strip() + ) + assert ansicolors_path.startswith(pex_root) + + +def test_venv_runtime_env_vars_ignored_during_create_nested(tmpdir): + # type: (Any) -> None + + # N.B.: The venv being created here is the internal Pip venv Pex uses to execute Pip. It's that + # venv that used to get PEX env vars like PEX_MODULE and PEX_SCRIPT sealed in at creation time. + assert_venv_runtime_env_vars_ignored_during_create(tmpdir, pex_venv=False) + + +def test_venv_runtime_env_vars_ignored_during_create_top_level(tmpdir): + assert_venv_runtime_env_vars_ignored_during_create(tmpdir, pex_venv=True) diff --git a/tests/test_pip.py b/tests/test_pip.py index e6ffe19db..fc3c483ee 100644 --- a/tests/test_pip.py +++ b/tests/test_pip.py @@ -21,9 +21,16 @@ from pex.variables import ENV if TYPE_CHECKING: - from typing import Any, Callable, Iterator, Optional + from typing import Any, Iterator, Optional, Protocol - CreatePip = Callable[[Optional[PythonInterpreter]], Pip] + class CreatePip(Protocol): + def __call__( + self, + interpreter, # type: Optional[PythonInterpreter] + **extra_env # type: str + ): + # type: (...) -> Pip + pass @pytest.fixture @@ -47,13 +54,15 @@ def create_pip( pex_root = os.path.join(str(tmpdir), "pex_root") pip_root = os.path.join(str(tmpdir), "pip_root") - with ENV.patch(PEX_ROOT=pex_root): - - def create_pip(interpreter): - # type: (Optional[PythonInterpreter]) -> Pip + def create_pip( + interpreter, # type: Optional[PythonInterpreter] + **extra_env # type: str + ): + # type: (...) -> Pip + with ENV.patch(PEX_ROOT=pex_root, **extra_env): return Pip.create(path=pip_root, interpreter=interpreter) - yield create_pip + yield create_pip def test_no_duplicate_constraints_pex_warnings( @@ -239,3 +248,16 @@ def test_download_platform_markers_issue_1488( ) == sorted(os.listdir(download_dir)) ) + + +def test_create_confounding_env_vars_issue_1668( + create_pip, # type: CreatePip + tmpdir, # type: Any +): + # type: (...) -> None + + download_dir = os.path.join(str(tmpdir), "downloads") + create_pip(None, PEX_SCRIPT="pex3").spawn_download_distributions( + requirements=["ansicolors==1.1.8"], download_dir=download_dir + ).wait() + assert ["ansicolors-1.1.8-py2.py3-none-any.whl"] == os.listdir(download_dir)