From cb0827e55ac2bcedcfe9fa7132f7f40d5b3573d5 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Mon, 31 Mar 2025 17:44:44 +1100 Subject: [PATCH 1/4] Filter out PEX_PYTHON, PEX_PYTHON_PATH and PEX_TOOLS --- pex/sh_boot.py | 29 ++++++++++++++++++++++++++--- pex/variables.py | 3 ++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pex/sh_boot.py b/pex/sh_boot.py index 50ab689e6..00c325cb3 100644 --- a/pex/sh_boot.py +++ b/pex/sh_boot.py @@ -181,6 +181,28 @@ def create_sh_boot_script( pex_info.raw_pex_root, pex_hash, expand_pex_root=False ) + vars_for_no_fast_path = [ + # If any of these environment variables are set, we're executing in a non-default manner + # that may result in different venv contents. In this case, having a warm cache can _behave_ + # different to a cold cache (not just _perform_ differently): if the already venv exists, it + # may not reflect the configuration that these env vars are implying. + # + # This means any use of these variables _always_ uses the Python-ful slow-path, even if a + # correctly-configured venv already exists (and even if they're equivalent to the default + # settings). Determining the correct path when these are set requires reproducing (in highly + # portable shell) the hashing logic from `venv_dir` in `pex.variables`. + # + # These variables should be kept in sync with `venv_dir` in `pex.variables`. + "PEX_PYTHON", + "PEX_PYTHON_PATH", + "PEX_PATH", + # PEX_TOOLS requires executing PEX code, but the in-venv code is PEX free and doesn't inspect + # `PEX_TOOLS=1`. + # + # (NB. unlike the ones above, this doesn't influence the venv contents.) + "PEX_TOOLS", + ] + return dedent( """\ # N.B.: This script should stick to syntax defined for POSIX `sh` and avoid non-builtins. @@ -199,11 +221,11 @@ def create_sh_boot_script( PEX_ROOT="${{PEX_ROOT:-${{DEFAULT_PEX_ROOT}}}}" INSTALLED_PEX="${{PEX_ROOT}}/{pex_installed_relpath}" - if [ -n "${{VENV}}" -a -x "${{INSTALLED_PEX}}" -a -z "${{PEX_TOOLS:-}}" ]; then + if [ -n "${{VENV}}" -a -x "${{INSTALLED_PEX}}" -a {check_no_fast_path} ]; then # We're a --venv execution mode PEX installed under the PEX_ROOT and the venv # interpreter to use is embedded in the shebang of our venv pex script; so just - # execute that script directly... except if we're needing to execute PEX code, in - # the form of the tools. + # execute that script directly... except if we're executing in a non-default manner, + # where we'll likely need to execute PEX code. export PEX="{pex}" exec "${{INSTALLED_PEX}}/bin/python" ${{VENV_PYTHON_ARGS}} "${{INSTALLED_PEX}}" \\ "$@" @@ -267,4 +289,5 @@ def create_sh_boot_script( shlex_quote(venv_python_arg) for venv_python_arg in venv_python_args ), pex="$0" if layout is Layout.ZIPAPP else '$(dirname "$0")', + check_no_fast_path=" -a ".join(" -z {}".format(name) for name in vars_for_no_fast_path), ) diff --git a/pex/variables.py b/pex/variables.py index dd536abf4..c9f5fa8de 100644 --- a/pex/variables.py +++ b/pex/variables.py @@ -834,7 +834,8 @@ def venv_dir( # The venv contents are affected by which PEX files are in play as well as which interpreter # is selected. The former is influenced via PEX_PATH and the latter is influenced by interpreter - # constraints, PEX_PYTHON and PEX_PYTHON_PATH. + # constraints, PEX_PYTHON and PEX_PYTHON_PATH. (NB. `create_sh_boot_script` in `pex.sh_boot` + # has code to disable its fast-path that must be kept in sync with the logic here.) pex_path_contents = {} # type: Dict[str, Dict[str, str]] venv_contents = {"pex_path": pex_path_contents} # type: Dict[str, Any] From 11a36892af2ff6845a027126cf15d67b7b2b89dd Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Mon, 31 Mar 2025 17:48:44 +1100 Subject: [PATCH 2/4] Add tests --- tests/integration/test_sh_boot.py | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/integration/test_sh_boot.py b/tests/integration/test_sh_boot.py index 3b381afae..ecd431fb7 100644 --- a/tests/integration/test_sh_boot.py +++ b/tests/integration/test_sh_boot.py @@ -345,3 +345,61 @@ def _check_pex_tools(): # run the tools with seeded PEX_ROOT _check_pex_tools() + + +@execution_mode +def test_issue_2728_pex_python_and_pex_python_path( + tmpdir, # type: Any + execution_mode_args, # type: List[str] +): + # type: (...) -> None + pex = os.path.realpath(os.path.join(str(tmpdir), "pex.sh")) + + run_pex_command(args=["-o", pex] + execution_mode_args).assert_success() + pex_root = os.path.join(str(tmpdir), "pex_root") + + def _check_pex_with_python(): + for env in [dict(PEX_PYTHON="FIXME"), dict(PEX_PYTHON_PATH="FIXME")]: + output = subprocess.check_output( + args=[pex, "-c", "import sys; print(sys.version)"], + env=make_env(PEX_ROOT=pex_root, **env), + stderr=subprocess.STDOUT, + ) + assert False, "FIXME" + + _check_pex_with_python() + + # ensure the PEX_ROOT is fully seeded e.g. with a venv for venv execution mode + subprocess.check_output(args=[pex, "-c", ""], env=make_env(PEX_ROOT=pex_root)) + + _check_pex_with_python() + + +@execution_mode +def test_issue_2728_pex_path( + tmpdir, # type: Any + execution_mode_args, # type: List[str] +): + # type: (...) -> None + pex = os.path.realpath(os.path.join(str(tmpdir), "pex.sh")) + + run_pex_command(args=["-o", pex] + execution_mode_args).assert_success() + pex_root = os.path.join(str(tmpdir), "pex_root") + + cowsay = os.path.join(str(tmpdir), "cowsay.pex") + run_pex_command(args=["cowsay==4.0", "-o", cowsay]).assert_success() + + def _check_pex_with_path(): + output = subprocess.check_output( + args=[pex, "-c", "import cowsay; print(cowsay.__version__)"], + env=make_env(PEX_ROOT=pex_root, PEX_PATH=cowsay), + stderr=subprocess.STDOUT, + ) + assert b"4.0\n" == output + + _check_pex_with_path() + + # ensure the PEX_ROOT is fully seeded e.g. with a venv for venv execution mode + subprocess.check_output(args=[pex, "-c", ""], env=make_env(PEX_ROOT=pex_root)) + + _check_pex_with_path() From fec51c3e6b853342b3fe6fb0086f01b92838d1d3 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Mon, 31 Mar 2025 17:50:59 +1100 Subject: [PATCH 3/4] Release metadata --- CHANGES.md | 6 ++++++ pex/version.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index cbb594129..30ce66757 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Release Notes +## 2.33.8 + +This release fixes running a PEX with any of `PEX_PYTHON=...`, `PEX_PYTHON_PATH=...` or `PEX_PATH=...` set for PEXes using venv-execution and sh-bootstrapping (that is, built with `--sh-boot --venv=...` ). Previously, those environment variables were ignored if the venv already existed in the `PEX_ROOT` (for instance, if the PEX had already been run). + +* Avoid fast-path in `--sh-boot` script for `PEX_PYTHON`, `PEX_PYTHON_PATH` and `PEX_PATH`. (#2729) + ## 2.33.7 This release fixes `PEX_TOOLS=1 ./path/to/pex` for PEXes using venv-execution and sh-bootstrapping (that is, built with `--sh-boot --venv=... --include-tools` ). Previously, the `PEX_TOOLS=1` was ignored if the venv already existed in the `PEX_ROOT` (for instance, if the PEX had already been run). diff --git a/pex/version.py b/pex/version.py index 232a3aa7c..2f370edbc 100644 --- a/pex/version.py +++ b/pex/version.py @@ -1,4 +1,4 @@ # Copyright 2015 Pex project contributors. # Licensed under the Apache License, Version 2.0 (see LICENSE). -__version__ = "2.33.7" +__version__ = "2.33.8" From ca1be8c8b14890d6ca3a3d8c174983ecff57ffaf Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Mon, 7 Apr 2025 17:56:51 +1000 Subject: [PATCH 4/4] Expand list --- CHANGES.md | 6 +++--- pex/sh_boot.py | 39 +++++++++++++++++++++++++++------------ 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 30ce66757..d4673412d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,9 +2,9 @@ ## 2.33.8 -This release fixes running a PEX with any of `PEX_PYTHON=...`, `PEX_PYTHON_PATH=...` or `PEX_PATH=...` set for PEXes using venv-execution and sh-bootstrapping (that is, built with `--sh-boot --venv=...` ). Previously, those environment variables were ignored if the venv already existed in the `PEX_ROOT` (for instance, if the PEX had already been run). +This release makes running a PEX using venv-execution and sh-bootstrapping (that is, build with `--sh-boot --venv`) more likely to behave identically with a cold or warm `PEX_ROOT` cache. This includes running with `PEX_PYTHON=...`, `PEX_PYTHON_PATH=...`, `PEX_PATH=...`, `PEX_VENV=...` and `PEX_IGNORE_RCFILES=...`. -* Avoid fast-path in `--sh-boot` script for `PEX_PYTHON`, `PEX_PYTHON_PATH` and `PEX_PATH`. (#2729) +* Avoid fast-path in `--sh-boot` script for more variables. (#2729) ## 2.33.7 @@ -228,7 +228,7 @@ In addition, the 2.24.2 release included a wheel with no compression This release fixes a long-standing bug in "YOLO-mode" foreign platform speculative wheel builds. Previously if the speculatively built wheel had tags that did not match the foreign platform, the process errored -pre-emptively. This was correct for complete foreign platforms, where +pre-emptively. This was correct for complete foreign platforms, where all tag information is known, but not for all cases of abbreviated platforms, where the failure was overly aggressive in some cases. Now foreign abbreviated platform speculative builds are only rejected when diff --git a/pex/sh_boot.py b/pex/sh_boot.py index 00c325cb3..5783bf7f7 100644 --- a/pex/sh_boot.py +++ b/pex/sh_boot.py @@ -181,26 +181,41 @@ def create_sh_boot_script( pex_info.raw_pex_root, pex_hash, expand_pex_root=False ) + # There's a fast-path that execs the entrypoint directly within a venv if one exists (i.e. the + # PEX_ROOT cache is warm), but it is only possible in cases where we can be reasonably sure: + # + # - the venv is configured correctly for the current execution (if not, executing with a cold + # cache may behave differently) + # - we do not need to execute any pex code (the venv has no such code) + # + # NB. we do not consider the contents of rc files, which can set any of these options. + # + # This should be kept in sync with env vars read (or not) by the venv_pex.py code, for which + # warnings are silenced. vars_for_no_fast_path = [ - # If any of these environment variables are set, we're executing in a non-default manner - # that may result in different venv contents. In this case, having a warm cache can _behave_ - # different to a cold cache (not just _perform_ differently): if the already venv exists, it - # may not reflect the configuration that these env vars are implying. - # - # This means any use of these variables _always_ uses the Python-ful slow-path, even if a - # correctly-configured venv already exists (and even if they're equivalent to the default - # settings). Determining the correct path when these are set requires reproducing (in highly - # portable shell) the hashing logic from `venv_dir` in `pex.variables`. - # - # These variables should be kept in sync with `venv_dir` in `pex.variables`. + # This is used when loading ENV (Variables()): + "PEX_IGNORE_RCFILES", + # And ENV is used to control the venv (e.g. whether to use a venv at all, which Python + # interpreter, any extra PEXes to include): + "PEX_VENV", + # (Determining the correct path when these are set would require reproducing (in highly + # portable shell) the hashing logic from `venv_dir` in `pex.variables`.) "PEX_PYTHON", "PEX_PYTHON_PATH", "PEX_PATH", - # PEX_TOOLS requires executing PEX code, but the in-venv code is PEX free and doesn't inspect + # PEX_TOOLS requires executing PEX code, but the in-venv code is PEX-free and doesn't inspect # `PEX_TOOLS=1`. # # (NB. unlike the ones above, this doesn't influence the venv contents.) "PEX_TOOLS", + # Other variables that are used during bootstrap / not read by venv_pex.py, but don't result + # in behaviour differences between a cold or warm cache: + # + # "PEX_ROOT", + # "PEX_VERBOSE", + # "PEX_EMIT_WARNINGS", + # "PEX_MAX_INSTALL_JOBS", + # "PEX_DISABLE_VARIABLES", ] return dedent(