Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add interpreter constraints option and use constraints to search for compatible interpreters at exec time #427

Merged
merged 44 commits into from
Dec 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
460a9ae
Add interpreter constraints option to pex CLI and write data to PEX-I…
Oct 20, 2017
45e4640
Improve integration tests
Oct 23, 2017
7ae3b61
Move helper function to interpreter constraints lib, refactor lib, an…
Oct 23, 2017
a6175dd
Add pex python path to supported env variables; add interpreter const…
Oct 23, 2017
528d93e
Resolve interpreters for testing from .tox dir instead of system-spec…
Oct 24, 2017
4fc655d
Refactor tests that use Python interpreters
Oct 24, 2017
ca8a9c6
Add mock patching for pex python path search api test
Oct 24, 2017
02fa8c5
Add interpreter bootstrapping as test helper method, make all interpr…
Oct 26, 2017
64c2b0c
Travis CI debugging
Oct 26, 2017
3408fb3
Add pytest.mark modification to skip test in CI
Oct 26, 2017
a1c4fed
Travis CI debug
Oct 26, 2017
cef5bf5
Travis CI debug
Oct 26, 2017
2957b80
Merge branch 'clivingston/add-pex-python-path' of github.com:CMLiving…
Oct 26, 2017
0877ad0
Travis CI debug 2
Oct 26, 2017
762e9d3
Travis CI test 3
Oct 26, 2017
ebf830a
Remove logging
Oct 26, 2017
233918e
Modify pex bootstrapper to repsect pex_python_path environment variab…
Nov 8, 2017
c0ef007
Add validation for conflicting constraints
Nov 16, 2017
0ffe8c2
Refactor and clean up code to enfore DRY
Nov 16, 2017
b5a86ea
Enforce interpreter selection at build time in addition to runtime se…
Nov 20, 2017
bcfd6c4
Add ability to read from pexrc at pex buildtime; fix test hermeticity…
Nov 21, 2017
d5566b5
More travis debug
Nov 21, 2017
350deb9
More travis debugging
Nov 21, 2017
ce099b6
Travis debug 2
Nov 21, 2017
155d18d
Travis debug 3
Nov 21, 2017
ff3e569
Travis test 4
Nov 21, 2017
b7d8510
Travis debug 5
Nov 21, 2017
0b3764d
Travis debug 4
Nov 21, 2017
5832b10
Travis debug 9
Nov 21, 2017
2b5012d
Travis debug 7
Nov 21, 2017
575a8ad
Travis debug 8
Nov 21, 2017
ce0dac6
Add travis PATH variable to yml
Nov 21, 2017
a2db930
Add pex python testing for backwards compatibility
Nov 21, 2017
1150b65
Readd once problematic tests now that travis has custom set PATH env var
Nov 21, 2017
3a143d1
Refactor methods and clean up doc strings/literals
Nov 22, 2017
b045ba5
Refactor testing methods for readability, refactor TRACER logic in pe…
Nov 22, 2017
66ef8ee
Refactor tests to improve readability
Nov 22, 2017
6a2bd58
Move build time pex rc reading logic into Variables and add more gran…
Nov 23, 2017
8311619
Add default arg to from_rc
Nov 28, 2017
e7590c2
Remove rc param from Variables constructor and modify related tests. …
Nov 29, 2017
6b3e551
Add a --rcfile flag to allow pexrc usage in integration tests
Nov 30, 2017
34773e5
Add comment regarding PEX_PYTHON
Nov 30, 2017
508226f
Add comments to interpreter constraints and cleanup docstring
Dec 1, 2017
524456b
Final cleanups to docs and minor refactoring for tests
Dec 2, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@
/.idea
/.coverage*
/htmlcov
/.pyenv_test
10 changes: 10 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ dist: precise

# TRAVIS_PYTHON_VERSION

env:
global:
# This is necessary to have consistent testing of methods that rely on falling back to PATH for
# selecting a python interpreter.
- PATH=/home/travis/build/pantsbuild/pex/.tox/py36/bin:/home/travis/build/pantsbuild/pex/.tox/py27/bin

cache:
directories:
- .pyenv_test

matrix:
include:
- language: python
Expand Down
46 changes: 37 additions & 9 deletions pex/bin/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
from pex.http import Context
from pex.installer import EggInstaller
from pex.interpreter import PythonInterpreter
from pex.interpreter_constraints import validate_constraints
from pex.iterator import Iterator
from pex.package import EggPackage, SourcePackage
from pex.pex import PEX
from pex.pex_bootstrapper import find_compatible_interpreters
from pex.pex_builder import PEXBuilder
from pex.platforms import Platform
from pex.requirements import requirements_from_file
Expand Down Expand Up @@ -289,6 +291,24 @@ def configure_clp_pex_environment(parser):
'can be passed multiple times to create a multi-interpreter compatible pex. '
'Default: Use current interpreter.')

group.add_option(
'--interpreter-constraint',
dest='interpreter_constraint',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, a-b-c will be automatically convert to a_b_c for variable names, so you don't necessarily need to specify dest here.

default=[],
type='str',
action='append',
help='A constraint that determines the interpreter compatibility for '
'this pex, using the Requirement-style format, e.g. "CPython>=3", or ">=2.7" '
'for requirements agnostic to interpreter class. This option can be passed multiple '
'times.')

group.add_option(
'--rcfile',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: for consistency, change option to --rc-file

dest='rc_file',
default=None,
help='An additional path to a pexrc file to read during configuration parsing. '
'Used primarily for testing.')

group.add_option(
'--python-shebang',
dest='python_shebang',
Expand Down Expand Up @@ -507,14 +527,6 @@ def get_interpreter(python_interpreter, interpreter_cache_dir, repos, use_wheel)
return interpreter


def _lowest_version_interpreter(interpreters):
"""Given a list of interpreters, return the one with the lowest version."""
lowest = interpreters[0]
for i in interpreters[1:]:
lowest = lowest if lowest < i else i
return lowest


def build_pex(args, options, resolver_option_builder):
with TRACER.timed('Resolving interpreters', V=2):
interpreters = [
Expand All @@ -525,6 +537,15 @@ def build_pex(args, options, resolver_option_builder):
for interpreter in options.python or [None]
]

if options.interpreter_constraint:
# NB: options.python and interpreter constraints cannot be used together, so this will not
# affect usages of the interpreter(s) specified by the "--python" command line flag.
constraints = options.interpreter_constraint
validate_constraints(constraints)
rc_variables = Variables.from_rc(rc=options.rc_file)
pex_python_path = rc_variables.get('PEX_PYTHON_PATH', '')
interpreters = find_compatible_interpreters(pex_python_path, constraints)

if not interpreters:
die('Could not find compatible interpreter', CANNOT_SETUP_INTERPRETER)

Expand All @@ -535,7 +556,8 @@ def build_pex(args, options, resolver_option_builder):
# options.preamble_file is None
preamble = None

interpreter = _lowest_version_interpreter(interpreters)
interpreter = min(interpreters)

pex_builder = PEXBuilder(path=safe_mkdtemp(), interpreter=interpreter, preamble=preamble)

pex_info = pex_builder.info
Expand All @@ -544,6 +566,9 @@ def build_pex(args, options, resolver_option_builder):
pex_info.always_write_cache = options.always_write_cache
pex_info.ignore_errors = options.ignore_errors
pex_info.inherit_path = options.inherit_path
if options.interpreter_constraint:
for ic in options.interpreter_constraint:
pex_builder.add_interpreter_constraint(ic)

resolvables = [Resolvable.get(arg, resolver_option_builder) for arg in args]

Expand Down Expand Up @@ -605,6 +630,9 @@ def main(args=None):
args, cmdline = args, []

options, reqs = parser.parse_args(args=args)
if options.python and options.interpreter_constraint:
die('The "--python" and "--interpreter-constraint" options cannot be used together.')

if options.pex_root:
ENV.set('PEX_ROOT', options.pex_root)
else:
Expand Down
39 changes: 39 additions & 0 deletions pex/interpreter_constraints.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Copyright 2017 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

# A library of functions for filtering Python interpreters based on compatibility constraints

from .common import die
from .interpreter import PythonIdentity
from .tracer import TRACER


def validate_constraints(constraints):
# TODO: add check to see if constraints are mutually exclusive (bad) so no time is wasted:
# https://github.com/pantsbuild/pex/issues/432
for req in constraints:
# Check that the compatibility requirements are well-formed.
try:
PythonIdentity.parse_requirement(req)
except ValueError as e:
die("Compatibility requirements are not formatted properly: %s" % str(e))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this error should probably cite the requirement string that failed to parse, e.g.

die('Compatibility requirement "%s" is not formatted properly: %e' % (req, str(e)))

Copy link
Contributor

@kwlzn kwlzn Nov 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specify the requirement that failed to parse's string.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second. Including the failed string will help user to debug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the e string contains this requirement. Sorry if this comment shows up elsewhere, I have no clue why it is not persisting in github.


def matched_interpreters(interpreters, constraints, meet_all_constraints=False):
"""Given some filters, yield any interpreter that matches at least one of them, or all of them
if meet_all_constraints is set to True.

:param interpreters: a list of PythonInterpreter objects for filtering
:param constraints: A sequence of strings that constrain the interpreter compatibility for this
pex, using the Requirement-style format, e.g. ``'CPython>=3', or just ['>=2.7','<3']``
for requirements agnostic to interpreter class.
:param meet_all_constraints: whether to match against all filters.
Defaults to matching interpreters that match at least one filter.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a line to this docstring for what exactly we're yielding?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:return interpreter: returns a generator that yields compatible interpreters
"""
check = all if meet_all_constraints else any
for interpreter in interpreters:
if check(interpreter.identity.matches(filt) for filt in constraints):
TRACER.log("Constraints on interpreters: %s, Matching Interpreter: %s"
% (constraints, interpreter.binary), V=3)
yield interpreter
109 changes: 97 additions & 12 deletions pex/pex_bootstrapper.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import print_function
import os
import sys

from .common import open_zip
from .common import die, open_zip
from .executor import Executor
from .interpreter import PythonInterpreter
from .interpreter_constraints import matched_interpreters
from .tracer import TRACER
from .variables import ENV

__all__ = ('bootstrap_pex',)

Expand Down Expand Up @@ -56,28 +61,108 @@ def find_in_path(target_interpreter):
return try_path


def maybe_reexec_pex():
from .variables import ENV
if not ENV.PEX_PYTHON:
return
def find_compatible_interpreters(pex_python_path, compatibility_constraints):
"""Find all compatible interpreters on the system within the supplied constraints and use
PEX_PYTHON_PATH if it is set. If not, fall back to interpreters on $PATH.
"""
if pex_python_path:
interpreters = []
for binary in pex_python_path.split(os.pathsep):
try:
interpreters.append(PythonInterpreter.from_binary(binary))
except Executor.ExecutionError:
print("Python interpreter %s in PEX_PYTHON_PATH failed to load properly." % binary,
file=sys.stderr)
if not interpreters:
die('PEX_PYTHON_PATH was defined, but no valid interpreters could be identified. Exiting.')
else:
if not os.getenv('PATH', ''):
# no $PATH, use sys.executable
interpreters = [PythonInterpreter.get()]
else:
# get all qualifying interpreters found in $PATH
interpreters = PythonInterpreter.all()

return list(matched_interpreters(
interpreters, compatibility_constraints, meet_all_constraints=True))

from .common import die
from .tracer import TRACER

target_python = ENV.PEX_PYTHON
def _select_pex_python_interpreter(target_python, compatibility_constraints):
target = find_in_path(target_python)

if not target:
die('Failed to find interpreter specified by PEX_PYTHON: %s' % target)
if compatibility_constraints:
pi = PythonInterpreter.from_binary(target)
if not list(matched_interpreters([pi], compatibility_constraints, meet_all_constraints=True)):
die('Interpreter specified by PEX_PYTHON (%s) is not compatible with specified '
'interpreter constraints: %s' % (target, str(compatibility_constraints)))
if not os.path.exists(target):
die('Target interpreter specified by PEX_PYTHON %s does not exist. Exiting.' % target)
return target


def _select_interpreter(pex_python_path, compatibility_constraints):
compatible_interpreters = find_compatible_interpreters(
pex_python_path, compatibility_constraints)

if not compatible_interpreters:
die('Failed to find compatible interpreter for constraints: %s'
% str(compatibility_constraints))
# TODO: https://github.com/pantsbuild/pex/issues/430
target = min(compatible_interpreters).binary

if os.path.exists(target) and os.path.realpath(target) != os.path.realpath(sys.executable):
TRACER.log('Detected PEX_PYTHON, re-exec to %s' % target)
return target


def maybe_reexec_pex(compatibility_constraints):
"""
Handle environment overrides for the Python interpreter to use when executing this pex.

This function supports interpreter filtering based on interpreter constraints stored in PEX-INFO
metadata. If PEX_PYTHON is set in a pexrc, it attempts to obtain the binary location of the
interpreter specified by PEX_PYTHON. If PEX_PYTHON_PATH is set, it attempts to search the path for
a matching interpreter in accordance with the interpreter constraints. If both variables are
present in a pexrc, this function gives precedence to PEX_PYTHON_PATH and errors out if no
compatible interpreters can be found on said path. If neither variable is set, fall through to
plain pex execution using PATH searching or the currently executing interpreter.

:param compatibility_constraints: list of requirements-style strings that constrain the
Python interpreter to re-exec this pex with.

"""
if ENV.SHOULD_EXIT_BOOTSTRAP_REEXEC:
return

selected_interpreter = None
with TRACER.timed('Selecting runtime interpreter based on pexrc', V=3):
if ENV.PEX_PYTHON and not ENV.PEX_PYTHON_PATH:
# preserve PEX_PYTHON re-exec for backwards compatibility
# TODO: Kill this off completely in favor of PEX_PYTHON_PATH
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start an issue for this TODO?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is on the line below this one.

# https://github.com/pantsbuild/pex/issues/431
selected_interpreter = _select_pex_python_interpreter(ENV.PEX_PYTHON,
compatibility_constraints)
elif ENV.PEX_PYTHON_PATH:
selected_interpreter = _select_interpreter(ENV.PEX_PYTHON_PATH, compatibility_constraints)

if selected_interpreter:
ENV.delete('PEX_PYTHON')
os.execve(target, [target_python] + sys.argv, ENV.copy())
ENV.delete('PEX_PYTHON_PATH')
ENV.SHOULD_EXIT_BOOTSTRAP_REEXEC = True
cmdline = [selected_interpreter] + sys.argv[1:]
TRACER.log('Re-executing: cmdline="%s", sys.executable="%s", PEX_PYTHON="%s", '
'PEX_PYTHON_PATH="%s", COMPATIBILITY_CONSTRAINTS="%s"'
% (cmdline, sys.executable, ENV.PEX_PYTHON, ENV.PEX_PYTHON_PATH,
compatibility_constraints))
os.execve(selected_interpreter, cmdline, ENV.copy())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this become MyPython MyPython original_params since on line 152, cmdline has already prepended selected_interpreter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first arg of cmdline is ignored in the os.execve API (not sure why exactly, but it is).



def bootstrap_pex(entry_point):
from .finders import register_finders
register_finders()
maybe_reexec_pex()
pex_info = get_pex_info(entry_point)
maybe_reexec_pex(pex_info.interpreter_constraints)

from . import pex
pex.PEX(entry_point).execute()
Expand Down
11 changes: 10 additions & 1 deletion pex/pex_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ def __init__(self, path=None, interpreter=None, chroot=None, pex_info=None, prea
interpreter exit.
"""
self._chroot = chroot or Chroot(path or safe_mkdtemp())
self._pex_info = pex_info or PexInfo.default()
self._frozen = False
self._interpreter = interpreter or PythonInterpreter.get()
self._shebang = self._interpreter.identity.hashbang()
self._logger = logging.getLogger(__name__)
self._preamble = to_bytes(preamble or '')
self._copy = copy
self._distributions = set()
self._pex_info = pex_info or PexInfo.default(interpreter)

def _ensure_unfrozen(self, name='Operation'):
if self._frozen:
Expand Down Expand Up @@ -166,6 +166,15 @@ def add_requirement(self, req):
self._ensure_unfrozen('Adding a requirement')
self._pex_info.add_requirement(req)

def add_interpreter_constraint(self, ic):
"""Add an interpreter constraint to the PEX environment.

:param ic: A version constraint on the interpreter used to build and run this PEX environment.

"""
self._ensure_unfrozen('Adding an interpreter constraint')
self._pex_info.add_interpreter_constraint(ic)

def set_executable(self, filename, env_filename=None):
"""Set the executable for this environment.

Expand Down
Loading