Skip to content

Commit

Permalink
Fix interpreter selection when building a PEX to use `[python-setup].…
Browse files Browse the repository at this point in the history
…interpreter_search_paths` (#10965)

### Problem

`PEX_PYTHON_PATH` is not used building a PEX, unless RC files are permitted, which we must ban. This means that `[python-setup].interpreter_search_paths` had zero impact when building a PEX, and Pex always looked at `$PATH` (controlled by `[pex].executable_search_paths`)

### Solution

Use the new `--python-path` build-time flag introduced in pex-tool/pex#1077.

Also fix the argv for our rule to select an interpreter, which was putting Pex flags in the `--` passthrough args section, so they were being ignored.

### Result

`[python-setup].interpreter_search_paths` correctly controls the interpreter search paths both when building a PEX and running a PEX.

[ci skip-rust]
  • Loading branch information
Eric-Arellano authored Oct 15, 2020
1 parent 962d283 commit 02b75b1
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 34 deletions.
2 changes: 1 addition & 1 deletion 3rdparty/python/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ mypy==0.782
mypy-extensions==0.4.3
packaging==20.4
pathspec==0.8.0
pex==2.1.18
pex==2.1.19
pip==18.1
pluggy==0.13.1
psutil==5.7.0
Expand Down
2 changes: 1 addition & 1 deletion 3rdparty/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ mypy==0.782

packaging==20.4
pathspec==0.8.0
pex==2.1.18
pex==2.1.19
psutil==5.7.0
pystache==0.5.4
# This should be kept in sync with `pytest.py`.
Expand Down
11 changes: 5 additions & 6 deletions src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,20 +419,19 @@ async def find_interpreter(interpreter_constraints: PexInterpreterConstraints) -
"-c",
# N.B.: The following code snippet must be compatible with Python 2.7 and
# Python 3.5+.
#
# When hashing, we pick 8192 for efficiency of reads and fingerprint updates
# (writes) since it's a common OS buffer size and an even multiple of the
# hash block size.
dedent(
"""\
import hashlib
import os
import sys
import hashlib, os, sys
python = os.path.realpath(sys.executable)
print(python)
hasher = hashlib.sha256()
with open(python, "rb") as fp:
# We pick 8192 for efficiency of reads and fingerprint updates
# (writes) since it's a common OS buffer size and an even
# multiple of the hash block size.
for chunk in iter(lambda: fp.read(8192), b""):
hasher.update(chunk)
print(hasher.hexdigest())
Expand Down
13 changes: 9 additions & 4 deletions src/python/pants/backend/python/util_rules/pex_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@
from pants.util.frozendict import FrozenDict
from pants.util.logging import LogLevel
from pants.util.meta import classproperty, frozen_after_init
from pants.util.strutil import create_path_env_var


class PexBinary(ExternalTool):
"""The PEX (Python EXecutable) tool (https://github.com/pantsbuild/pex)."""

options_scope = "download-pex-bin"
name = "pex"
default_version = "v2.1.18"
default_version = "v2.1.19"

@classproperty
def default_known_versions(cls):
Expand All @@ -41,8 +42,8 @@ def default_known_versions(cls):
(
cls.default_version,
plat,
"3a47dcbcf49294c06439fdda50675546edc9ea09a116c4c57ee5a8e337a63509",
"2678219",
"9d3c492c8b5847ce52d806cf188562c5d5acac3e95ae9df65b8fcf451edde296",
"2680733",
)
)
for plat in ["darwin", "linux"]
Expand Down Expand Up @@ -129,8 +130,9 @@ async def setup_pex_cli_process(
pex_root_path = ".cache/pex_root"
argv = pex_env.create_argv(
downloaded_pex_bin.exe,
*request.argv,
*cert_args,
"--python-path",
create_path_env_var(pex_env.interpreter_search_paths),
"--pex-root",
pex_root_path,
# Ensure Pex and its subprocesses create temporary files in the the process execution
Expand All @@ -144,6 +146,9 @@ async def setup_pex_cli_process(
# CWD can find the TMPDIR.
"--tmpdir",
tmpdir,
# NB: This comes at the end of the argv because the request may use `--` passthrough args,
# which must come at the end.
*request.argv,
python=request.python,
)
env = {
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/util_rules/pex_cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_custom_ca_certs(rule_runner: RuleRunner) -> None:
Process,
[PexCliProcess(argv=["some", "--args"], description="")],
)
assert proc.argv[2:6] == ("some", "--args", "--cert", "certsfile")
assert proc.argv[2:4] == ("--cert", "certsfile")
files = rule_runner.request(DigestContents, [proc.input_digest])
chrooted_certs_file = [f for f in files if f.path == "certsfile"]
assert len(chrooted_certs_file) == 1
Expand Down
45 changes: 24 additions & 21 deletions src/python/pants/backend/python/util_rules/pex_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,12 @@ class PexEnvironment(EngineAwareReturnType):
bootstrap_python: Optional[PythonExecutable] = None

def create_argv(
self, pex_path: str, *args: str, python: Optional[PythonExecutable] = None
self, pex_filepath: str, *args: str, python: Optional[PythonExecutable] = None
) -> Tuple[str, ...]:
python = python or self.bootstrap_python
argv = [python.path] if python else []
argv.extend((pex_path, *args))
argv.append(pex_filepath)
argv.extend(args)
return tuple(argv)

def environment_dict(self, *, python_configured: bool) -> Mapping[str, str]:
Expand All @@ -125,7 +126,8 @@ def environment_dict(self, *, python_configured: bool) -> Mapping[str, str]:
**self.subprocess_environment_dict,
)
# NB: We only set `PEX_PYTHON_PATH` if the Python interpreter has not already been
# pre-selected by Pants. Otherwise, Pex may try to find another interpreter.
# pre-selected by Pants. Otherwise, Pex would inadvertently try to find another interpreter
# when running PEXes. (Creating a PEX will ignore this env var in favor of `--python-path`.)
if not python_configured:
d["PEX_PYTHON_PATH"] = create_path_env_var(self.interpreter_search_paths)
return d
Expand Down Expand Up @@ -164,30 +166,31 @@ async def find_pex_python(
"-c",
# N.B.: The following code snippet must be compatible with Python 2.7 and
# Python 3.5+.
#
# We hash the underlying Python interpreter executable to ensure we detect
# changes in the real interpreter that might otherwise be masked by Pyenv
# shim scripts found on the search path. Naively, just printing out the full
# version_info would be enough, but that does not account for supported abi
# changes (e.g.: a pyenv switch from a py27mu interpreter to a py27m
# interpreter.)
#
# When hashing, we pick 8192 for efficiency of reads and fingerprint updates
# (writes) since it's a common OS buffer size and an even multiple of the
# hash block size.
dedent(
"""\
import sys
major, minor = sys.version_info[:2]
if (major, minor) == (2, 7) or (major == 3 and minor >= 5):
# Here we hash the underlying python interpreter executable to
# ensure we detect changes in the real interpreter that might
# otherwise be masked by pyenv shim scripts found on the search
# path. Naively, just printing out the full version_info would be
# enough, but that does not account for supported abi changes (e.g.:
# a pyenv switch from a py27mu interpreter to a py27m interpreter.
import hashlib
hasher = hashlib.sha256()
with open(sys.executable, "rb") as fp:
# We pick 8192 for efficiency of reads and fingerprint updates
# (writes) since it's a common OS buffer size and an even
# multiple of the hash block size.
for chunk in iter(lambda: fp.read(8192), b""):
hasher.update(chunk)
sys.stdout.write(hasher.hexdigest())
sys.exit(0)
else:
if (major, minor) != (2, 7) and not (major == 3 and minor >= 5):
sys.exit(1)
import hashlib
hasher = hashlib.sha256()
with open(sys.executable, "rb") as fp:
for chunk in iter(lambda: fp.read(8192), b""):
hasher.update(chunk)
sys.stdout.write(hasher.hexdigest())
"""
),
],
Expand Down

0 comments on commit 02b75b1

Please sign in to comment.