Skip to content

Commit

Permalink
Fix --venv mode venv dir hash.
Browse files Browse the repository at this point in the history
Previously the venv dir hash locked unconstrained PEXes into a venv using
the interpreter they were first run with. Now, when a `--venv` mode PEX
is either unconstrained or only constrained by a PEX_PYTHON_PATH that
admits the interpreter being used to execute the `--venv` mode PEX, that
interpreter is recorded in the venv dir hash to differentiate a venv
dedicated to that interpreter.

Fixes pex-tool#1422
  • Loading branch information
jsirois committed Aug 29, 2021
1 parent 8a85f14 commit 815c086
Show file tree
Hide file tree
Showing 8 changed files with 233 additions and 53 deletions.
2 changes: 1 addition & 1 deletion pex/pex_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def _bootstrap(entry_point):
def ensure_venv(pex):
# type: (PEX) -> str
pex_info = pex.pex_info()
venv_dir = pex_info.venv_dir
venv_dir = pex_info.venv_dir(pex_file=pex.path(), interpreter=pex.interpreter)
if venv_dir is None:
raise AssertionError(
"Expected PEX-INFO for {} to have the components of a venv directory".format(pex.path())
Expand Down
7 changes: 3 additions & 4 deletions pex/pex_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ def __maybe_run_venv__(pex, pex_root, pex_path):
from pex.variables import venv_dir
venv_home = venv_dir(
pex_file=pex,
pex_root=pex_root,
pex_hash={pex_hash!r},
interpreter_constraints={interpreter_constraints!r},
strip_pex_env={strip_pex_env!r},
has_interpreter_constraints={has_interpreter_constraints!r},
pex_path=pex_path,
)
venv_pex = os.path.join(venv_home, 'pex')
Expand Down Expand Up @@ -552,8 +552,7 @@ def _prepare_code(self):
bootstrap_dir=self._pex_info.bootstrap,
pex_root=self._pex_info.raw_pex_root,
pex_hash=self._pex_info.pex_hash,
interpreter_constraints=self._pex_info.interpreter_constraints,
strip_pex_env=self._pex_info.strip_pex_env,
has_interpreter_constraints=bool(self._pex_info.interpreter_constraints),
pex_path=self._pex_info.pex_path,
is_unzip=self._pex_info.unzip,
is_venv=self._pex_info.venv,
Expand Down
19 changes: 12 additions & 7 deletions pex/pex_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ def __init__(self, info=None):
raise ValueError(
"PexInfo can only be seeded with a dict, got: " "%s of type %s" % (info, type(info))
)
self._pex_info = dict(info) if info else {} # type Dict[str, Any]
self._distributions = self._pex_info.get("distributions", {})
self._pex_info = dict(info) if info else {} # type: Dict[str, Any]
self._distributions = self._pex_info.get("distributions", {}) # type: Dict[str, str]
# cast as set because pex info from json must store interpreter_constraints as a list
self._interpreter_constraints = set(self._pex_info.get("interpreter_constraints", set()))
requirements = self._pex_info.get("requirements", [])
Expand Down Expand Up @@ -249,18 +249,22 @@ def venv_copies(self, value):
# type: (bool) -> None
self._pex_info["venv_copies"] = value

@property
def venv_dir(self):
# type: () -> Optional[str]
def venv_dir(
self,
pex_file, # type: str
interpreter=None, # type: Optional[PythonInterpreter]
):
# type: (...) -> Optional[str]
if not self.venv:
return None
if self.pex_hash is None:
raise ValueError("The venv_dir was requested but no pex_hash was set.")
return variables.venv_dir(
pex_file=pex_file,
pex_root=self.pex_root,
pex_hash=self.pex_hash,
interpreter_constraints=self.interpreter_constraints,
strip_pex_env=self.strip_pex_env,
has_interpreter_constraints=bool(self.interpreter_constraints),
interpreter=interpreter,
pex_path=self.pex_path,
)

Expand Down Expand Up @@ -399,6 +403,7 @@ def add_distribution(self, location, sha):

@property
def distributions(self):
# type: () -> Dict[str, str]
return self._distributions

@property
Expand Down
9 changes: 3 additions & 6 deletions pex/pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ def _check_returncode(self, stderr=None):
super(_LogScrapeJob, self)._check_returncode(stderr=stderr)


@attr.s(frozen=True)
class Pip(object):
_PATCHED_MARKERS_FILE_ENV_VAR_NAME = "_PEX_PATCHED_MARKERS_FILE"

Expand Down Expand Up @@ -377,13 +378,9 @@ def patch_markers():
fp.close()
isolated_pip_builder.set_executable(fp.name, "__pex_patched_pip__.py")
isolated_pip_builder.freeze()
pex_info = PexInfo.from_pex(pip_pex_path)
pex_info.add_interpreter_constraint(str(pip_interpreter.identity.requirement))
return cls(ensure_venv(PEX(pip_pex_path, interpreter=pip_interpreter, pex_info=pex_info)))
return cls(ensure_venv(PEX(pip_pex_path, interpreter=pip_interpreter)))

def __init__(self, pip_pex_path):
# type: (str) -> None
self._pip_pex_path = pip_pex_path # type: str
_pip_pex_path = attr.ib() # type: str

@staticmethod
def _calculate_resolver_version(package_index_configuration=None):
Expand Down
1 change: 1 addition & 0 deletions pex/tools/commands/venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ def iter_valid_venv_pythons():
"PEX_PYTHON",
"PEX_PYTHON_PATH",
"PEX_VERBOSE",
"PEX_EMIT_WARNINGS",
"__PEX_EXE__",
"__PEX_UNVENDORED__",
# This is _not_ used (it is ignored), but it's present under CI and simplest to
Expand Down
127 changes: 102 additions & 25 deletions pex/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,26 @@
# Due to the PEX_ properties, disable checkstyle.
# checkstyle: noqa

from __future__ import absolute_import
from __future__ import absolute_import, print_function

import hashlib
import json
import os
import re
import sys
from contextlib import contextmanager
from textwrap import dedent

from pex import pex_warnings
from pex.common import can_write_dir, die, safe_mkdtemp
from pex.inherit_path import InheritPath
from pex.typing import TYPE_CHECKING, Generic, overload
from pex.typing import TYPE_CHECKING, Generic, cast, overload
from pex.venv_bin_path import BinPath

if TYPE_CHECKING:
from typing import (
Callable,
Dict,
Iterable,
Iterator,
Optional,
Tuple,
Expand All @@ -35,6 +36,9 @@
_O = TypeVar("_O")
_P = TypeVar("_P")

# N.B.: This is an expensive import and we only need it for type checking.
from pex.interpreter import PythonInterpreter


class NoValueError(Exception):
"""Indicates a property has no value set.
Expand Down Expand Up @@ -668,45 +672,118 @@ def unzip_dir(


def venv_dir(
pex_file, # type: str
pex_root, # type: str
pex_hash, # type: str
interpreter_constraints, # type: Iterable[str]
strip_pex_env, # type: bool
has_interpreter_constraints, # type: bool
interpreter=None, # type: Optional[PythonInterpreter]
pex_path=None, # type: Optional[str]
):
# type: (...) -> str
# The venv contents are affected by which interpreter is selected as well as which PEX files are
# in play. The former can be influenced by interpreter constraints changes as well as PEX_PYTHON
# and PEX_PYTHON_PATH. The latter is influenced via PEX_PATH.
venv_contents = {
"interpreter_constraints": sorted(interpreter_constraints),
"PEX_PYTHON": ENV.PEX_PYTHON,
"PEX_PYTHON_PATH": ENV.PEX_PYTHON_PATH,
"strip_pex_env": strip_pex_env,
"pex_path": {},
} # type: Dict[str, Any]

# 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 is influenced by interpreter
# constraints, PEX_PYTHON and PEX_PYTHON_PATH.

pex_path_contents = {} # type: Dict[str, Dict[str, str]]
venv_contents = {"pex_path": pex_path_contents} # type: Dict[str, Any]

# PexInfo.pex_path and PEX_PATH are merged by PEX at runtime, so we include both and hash just
# the distributions since those are the only items used from PEX_PATH adjoined PEX files; i.e.:
# neither the entry_point nor any other PEX file data or metadata is used.
def add_pex_path_items(pex_path):
if not pex_path:
def add_pex_path_items(path):
# type: (Optional[str]) -> None
if not path:
return
from pex.pex_info import PexInfo

pex_path_contents = venv_contents["pex_path"]
for pex in pex_path.split(":"):
for pex in path.split(":"):
pex_path_contents[pex] = PexInfo.from_pex(pex).distributions

add_pex_path_items(pex_path)
add_pex_path_items(ENV.PEX_PATH)

# There are two broad cases of interpreter selection, Pex chooses and user chooses. For the
# former we hash Pex's selection criteria and for the latter we write down the actual
# interpreter the user selected. N.B.: The pex_hash already includes an encoding of the
# interpreter constraints, if any; so we don't hash those here again for the Pex chooses case.

# This is relevant in all cases of interpreter selection and restricts the valid search path
# for interpreter constraints, PEX_PYTHON and otherwise unconstrained PEXes.
venv_contents["PEX_PYTHON_PATH"] = ENV.PEX_PYTHON_PATH

interpreter_path = None # type: Optional[str]
imprecise_pex_python = ENV.PEX_PYTHON and not os.path.isabs(ENV.PEX_PYTHON)
if imprecise_pex_python:
# For PEX_PYTHON=python3.7, for example, we just write down the constraint and let the PEX
# runtime determine the path of an appropriate python3.7, if any (Pex chooses).
venv_contents["PEX_PYTHON"] = ENV.PEX_PYTHON
elif not has_interpreter_constraints:
# Otherwise we can calculate the exact interpreter the PEX runtime will use. This ensures
# ~unconstrained PEXes get a venv per interpreter used to invoke them with (user chooses).
interpreter_binary = os.path.realpath(
interpreter.binary if interpreter else (ENV.PEX_PYTHON or sys.executable)
)
if not ENV.PEX_PYTHON_PATH or interpreter_binary.startswith(
tuple(ENV.PEX_PYTHON_PATH.split(":"))
):
interpreter_path = interpreter_binary
if interpreter_path:
venv_contents["interpreter"] = interpreter_path

venv_contents_hash = hashlib.sha1(
json.dumps(venv_contents, sort_keys=True).encode("utf-8")
).hexdigest()
return os.path.join(
_expand_pex_root(pex_root),
"venvs",
pex_hash,
venv_contents_hash,
)
venv_path = os.path.join(_expand_pex_root(pex_root), "venvs", pex_hash, venv_contents_hash)

def warn(message):
# type: (str) -> None
from pex.pex_info import PexInfo

pex_warnings.configure_warnings(PexInfo.from_pex(pex_file), ENV)
pex_warnings.warn(message)

# N.B.: We know ENV.PEX_PYTHON is a str when imprecise_pex_python is True but mypy does not.
if imprecise_pex_python and not re.match(r".*[^\d][\d]+\.[\d+]$", cast(str, ENV.PEX_PYTHON)):
warn(
dedent(
"""\
Using a venv selected by PEX_PYTHON={pex_python} for {pex_file} at {venv_path}.
If `{pex_python}` is upgraded or downgraded at some later date, this venv will still
be used. To force re-creation of the venv using the upgraded or downgraded
`{pex_python}` you will need to delete it at that point in time.
To avoid this warning, either specify a Python binary with major and minor version
in its name, like PEX_PYTHON=python{current_python_version} or else re-build the PEX
with `--no-emit-warnings` or re-run the PEX with PEX_EMIT_WARNINGS=False.
""".format(
pex_python=ENV.PEX_PYTHON,
pex_file=os.path.normpath(pex_file),
venv_path=venv_path,
current_python_version=".".join(map(str, sys.version_info[:2])),
)
)
)
if not interpreter_path and ENV.PEX_PYTHON_PATH:
warn(
dedent(
"""\
Using a venv restricted by PEX_PYTHON_PATH={ppp} for {pex_file} at {venv_path}.
If the contents of `{ppp}` changes at some later date, this venv and the interpreter
selected from `{ppp}` will still be used. To force re-creation of the venv using
the new pythons available on `{ppp}` you will need to delete it at that point in
time.
To avoid this warning, re-build the PEX with `--no-emit-warnings` or re-run the PEX
with PEX_EMIT_WARNINGS=False.
"""
).format(
ppp=ENV.PEX_PYTHON_PATH,
pex_file=os.path.normpath(pex_file),
venv_path=venv_path,
)
)

return venv_path
Loading

0 comments on commit 815c086

Please sign in to comment.