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 17 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
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ dist: precise

# TRAVIS_PYTHON_VERSION

cache:
directories:
- .pyenv_test

matrix:
include:
- language: python
Expand Down
32 changes: 23 additions & 9 deletions pex/bin/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from pex.http import Context
from pex.installer import EggInstaller
from pex.interpreter import PythonInterpreter
from pex.interpreter_constraints import check_requirements_are_well_formed, matched_interpreters
from pex.iterator import Iterator
from pex.package import EggPackage, SourcePackage
from pex.pex import PEX
Expand Down Expand Up @@ -289,6 +290,17 @@ 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(
'--python-shebang',
dest='python_shebang',
Expand Down Expand Up @@ -507,14 +519,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 +529,12 @@ def build_pex(args, options, resolver_option_builder):
for interpreter in options.python or [None]
]

if options.interpreter_constraint:
constraints = options.interpreter_constraint
# TODO: add check to see if constraints are mutually exclusive (bad) so no time is wasted
check_requirements_are_well_formed(constraints)
Copy link
Contributor

Choose a reason for hiding this comment

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

how about calling this validate_constraints() and moving the TODO here into it's function definition?

interpreters = list(matched_interpreters(interpreters, constraints, meet_all_constraints=True))

Copy link
Contributor

Choose a reason for hiding this comment

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

seems to me like in the case where no --python args are explicitly passed, that interpreters here pre-constraint filtration will only contain the currently executing interpreter which is probably insufficient for any users targeting e.g. python3 with constraints while executing pex with python2.

this would mean that anytime a user passes --constraint they'd also need to pass one or more --python args to surface the interpreters to actually apply the constraints to - which feels awkward.

I think we'll probably need to:

  1. make --python mutually exclusive with --constraints - or at least warn when both are passed.
  2. fall back to path or PPP search when --constraints is passed without --python.
  3. potentially deprecate --python in favor of --constraints, if it makes sense.

this will pave a way for a future where both build and runtime interpreter selection behave identically, afaict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea of a combination of 1 and 2. Disallow both options as using both seems like an anti-pattern and fall back to PPP/PATH when --constraints is passed without --python.

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

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

interpreter = _lowest_version_interpreter(interpreters)
# TODO: make whether to take min or max version interpreter configurable
interpreter = min(interpreters)
Copy link
Contributor

Choose a reason for hiding this comment

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

this TODO might be better as a github issue link instead?

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

pex_info = pex_builder.info
Expand All @@ -544,6 +555,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 constraints:
pex_builder.add_interpreter_constraint(ic)

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

Expand Down
44 changes: 44 additions & 0 deletions pex/interpreter_constraints.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
Copy link
Contributor Author

@CMLivingston CMLivingston Oct 23, 2017

Choose a reason for hiding this comment

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

This new library is extent of the intersecting interpreter constraints logic that makes sense to move out of Pants and into Pex. It will be cleanly consumable by Pants and the only modification I made is to add a meet_all_constraints kwarg for meeting all filters passed instead of at least one, which is backward compatible with the Pants use cases.

I operated on the assumption that the other methods of this lib have been tested or are functioning properly so I did not add any new tests for them but I am more than happy to.

Copy link

Choose a reason for hiding this comment

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

Should this get bumped to 2017?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

# Licensed under the Apache License, Version 2.0 (see LICENSE).

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

from .interpreter import PythonIdentity


def _matches(interpreter, filters, meet_all_constraints=False):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added meet_all_constraints because for some reason the Pants usage of this code (currently in Pants codebase) only requires a match against one of the filters to be considered a "matching interpreter".

if meet_all_constraints:
return all(interpreter.identity.matches(filt) for filt in filters)
else:
return any(interpreter.identity.matches(filt) for filt in filters)
Copy link
Contributor

Choose a reason for hiding this comment

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

how about parameterizing the function applied here to enforce DRY?

check = all if meet_all_constraints else any
return check(interpreter.identity.matches(filt) for filt in filters)



def _matching(interpreters, filters, meet_all_constraints=False):
for interpreter in interpreters:
if _matches(interpreter, filters, meet_all_constraints):
yield interpreter
Copy link
Contributor

Choose a reason for hiding this comment

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

this and _matches are probably thin enough that it may not make sense to break them out into separate functions?



def check_requirements_are_well_formed(constraints):
# Check that the compatibility requirements are well-formed.
for req in constraints:
try:
PythonIdentity.parse_requirement(req)
except ValueError as e:
from .common import die
Copy link
Contributor

Choose a reason for hiding this comment

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

this import should move to the top of the file

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 should probably directly quote the req that failed to parse, so its clearer to the user exactly what it's barfing on (unless the exception covers that already?)

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 ValueError raised by the exception covers this.


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, filters, 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 filters: 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

should this param be called constraints instead? I'd expect something called a filter to be a lambda/function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, filter is used due to the precedence set by Pants. The interpreter constraints lib is forklifted out of Pants and readily consumable if we elect to refactor usages in Pants.

: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.

"""
for match in _matching(interpreters, filters, meet_all_constraints):
yield match
Copy link
Contributor

Choose a reason for hiding this comment

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

might be good for this to log.debug() here both the constraints and the paths to interpreters that were matched for better field troubleshooting.

97 changes: 87 additions & 10 deletions pex/pex_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
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,100 @@ def find_in_path(target_interpreter):
return try_path


def maybe_reexec_pex():
from .variables import ENV
if not ENV.PEX_PYTHON:
return
def _get_python_interpreter(binary):
try:
return PythonInterpreter.from_binary(binary)
# from_binary attempts to execute the interpreter
except Executor.ExecutionError:
return None


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 env variable if it is set. If not, fall back to interpreters on $PATH.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

s/env variable//

if pex_python_path:

Choose a reason for hiding this comment

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

same codes as pex/bin/pex.py? If so, we should extract this to a library.

interpreters = []
for binary in pex_python_path.split(os.pathsep):
pi = _get_python_interpreter(binary)
if pi:
interpreters.append(pi)
if not interpreters:
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe more readable?

for binary in ...:
  try:
    interpreters.append(PythonInterpreter.from_binary(binary))
  except Executor.ExecutionError:
    pass

die('No interpreters from PEX_PYTHON_PATH can be found on the system. Exiting.')
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible for the interpreter to exist, but to just fail execution? if so, this might be better stated as:

PEX_PYTHON_PATH was defined, but no valid interpreters could be identified. Exiting.

else:
# All qualifying interpreters found in $PATH
interpreters = PythonInterpreter.all()

from .common import die
from .tracer import TRACER
compatible_interpreters = list(matched_interpreters(
interpreters, compatibility_constraints, meet_all_constraints=True))
return compatible_interpreters if compatible_interpreters else None
Copy link
Contributor

Choose a reason for hiding this comment

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

this could probably just directly return the output of list(matched_interpreters(...)) vs doing the None conversion given that the check below of if compatible_interpreters would evaluate false either way.


target_python = ENV.PEX_PYTHON

def _handle_pex_python(target_python, compatibility_constraints):
"""Re-exec using the PEX_PYTHON interpreter"""
Copy link
Contributor

Choose a reason for hiding this comment

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

this docstring is no longer true.

target = find_in_path(target_python)
if not target:
die('Failed to find interpreter specified by PEX_PYTHON: %s' % target)
elif compatibility_constraints:
Copy link
Contributor

Choose a reason for hiding this comment

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

s/elif/if - these all act as guard clauses.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_matching got folded into the matched_interpreters method so I will remove the method.

pi = PythonInterpreter.from_binary(target)
if not all(pi.identity.matches(constraint) for constraint in compatibility_constraints):
Copy link
Contributor

Choose a reason for hiding this comment

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

would expect this to use a helpers in pex/interpreter_constraints.py vs reimplementing bits of that here?

die('Interpreter specified by PEX_PYTHON (%s) is not compatible with specified '
'interpreter constraints: %s' % (target, str(compatibility_constraints)))
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)
ENV.delete('PEX_PYTHON')
ENV.SHOULD_EXIT_BOOTSTRAP_REEXEC = True
Copy link
Contributor

Choose a reason for hiding this comment

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

afaict, this should continue to delete PEX_PYTHON from the env if set.

os.execve(target, [target_python] + sys.argv, ENV.copy())


def _handle_general_interpreter_selection(pex_python_path, compatibility_constraints):
"""Handle selection in the case that PEX_PYTHON_PATH is set or interpreter compatibility
constraints are specified.
"""
compatible_interpreters = _find_compatible_interpreters(
pex_python_path, compatibility_constraints)

target = None
if compatible_interpreters:
target = min(compatible_interpreters).binary
Copy link
Contributor

Choose a reason for hiding this comment

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

this should get a matching TODO/ticket link like you have on the build-side for min vs max.

if not target:
die('Failed to find compatible interpreter for constraints: %s'
% str(compatibility_constraints))
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like this die should be raised if not compatible_interpreters vs if not target. e.g.

compatible_interpreters = ...
if not compatible_interpreters:
  die(...)
target = min(compatible_interpreters).binary

unless its possible for min(...).binary to also be '' or None?

if os.path.exists(target) and os.path.realpath(target) != os.path.realpath(sys.executable):
if pex_python_path:
TRACER.log('Detected PEX_PYTHON_PATH, re-exec to %s' % target)
else:
TRACER.log('Re-exec to interpreter %s that matches constraints: %s' % (target,
str(compatibility_constraints)))
ENV.SHOULD_EXIT_BOOTSTRAP_REEXEC = True
os.execve(target, [target] + sys.argv, ENV.copy())


Copy link
Contributor

Choose a reason for hiding this comment

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

would be better to define one docstring here and redact the two in the module-private functions above.

def maybe_reexec_pex(compatibility_constraints=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

this probably shouldn't default to None.

if ENV.SHOULD_EXIT_BOOTSTRAP_REEXEC:
return
if ENV.PEX_PYTHON and ENV.PEX_PYTHON_PATH:
# both vars are defined, fall through to PEX_PYTHON_PATH resolution (give precedence to PPP)
pass
elif ENV.PEX_PYTHON:
# preserve PEX_PYTHON re-exec for backwards compatibility
Copy link
Contributor

Choose a reason for hiding this comment

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

cleaner?

if ENV.PEX_PYTHON and not ENV.PEX_PYTHON_PATH:
  _handle_pex_python(...)

_handle_pex_python(ENV.PEX_PYTHON, compatibility_constraints)

if not compatibility_constraints:
# if no compatibility constraints are specified, we want to match against
# the lowest-versioned interpreter in PEX_PYTHON_PATH if it is set
if not ENV.PEX_PYTHON_PATH:
# no PEX_PYTHON_PATH, PEX_PYTHON, or interpreter constraints, continue as normal
return

_handle_general_interpreter_selection(ENV.PEX_PYTHON_PATH, compatibility_constraints)
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be better/more readable to make the _handle* functions simply return a python interpreter path vs directly re-exec - and then you could have only one os.execve at the bottom of this function to actually re-exec against the selected interpreter.

this whole function could probably then be simplified to something along the lines of:

def maybe_reexec_pex(...):
  if ENV.SHOULD_EXIT_BOOTSTRAP_REEXEC:
    return

  selected_interpreter = None
  if ENV.PEX_PYTHON and not ENV.PEX_PYTHON_PATH:
    selected_interpreter = _select_pex_python_interpreter(...)
  elif ENV.PEX_PYTHON_PATH:
    selected_interpreter = _select_interpreter(...)

  if selected_interpreter:
    ENV.delete('PEX_PYTHON')
    ENV.delete('PEX_PYTHON_PATH')
    ENV.SHOULD_EXIT_BOOTSTRAP_REEXEC = True
    os.execve(selected_interpreter, [selected_interpreter] + sys.argv[1:], ENV.copy())  



def bootstrap_pex(entry_point):
from .finders import register_finders
register_finders()
maybe_reexec_pex()
pex_info = get_pex_info(entry_point)
with TRACER.timed('Bootstrapping pex with maybe_reexec_pex', V=2):
Copy link
Contributor

Choose a reason for hiding this comment

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

the get_pex_info here should probably fall under the TRACER.timed block.

and a more concise/descriptive string here for the timer might be "Interpreter selection", probably with V=3 to distinguish it in debug logging.

maybe_reexec_pex(pex_info.interpreter_constraints)

from . import pex
pex.PEX(entry_point).execute()
Expand Down
9 changes: 9 additions & 0 deletions pex/pex_builder.py
Original file line number Diff line number Diff line change
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
17 changes: 17 additions & 0 deletions pex/pex_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ def __init__(self, info=None):
'%s of type %s' % (info, type(info)))
self._pex_info = info or {}
self._distributions = self._pex_info.get('distributions', {})
self._interpreter_constraints = self._pex_info.get('interpreter_constraints', [])
requirements = self._pex_info.get('requirements', [])
if not isinstance(requirements, (list, tuple)):
raise ValueError('Expected requirements to be a list, got %s' % type(requirements))
Expand Down Expand Up @@ -195,6 +196,20 @@ def inherit_path(self):
def inherit_path(self, value):
self._pex_info['inherit_path'] = bool(value)

@property
def interpreter_constraints(self):
"""A list of constraints that determine 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.

This property will be used at exec time when bootstrapping a pex to search PEX_PYTHON_PATH
for a list of compatible interpreters.
"""
return self._interpreter_constraints

def add_interpreter_constraint(self, value):
self._interpreter_constraints.append(str(value))

@property
def ignore_errors(self):
return self._pex_info.get('ignore_errors', False)
Expand Down Expand Up @@ -274,11 +289,13 @@ def update(self, other):
raise TypeError('Cannot merge a %r with PexInfo' % type(other))
self._pex_info.update(other._pex_info)
self._distributions.update(other.distributions)
self._interpreter_constraints.extend(other.interpreter_constraints)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably dedupe somehow, perhaps via internal storage as a set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Python sets are not JSON serializable but I can use sets to reduce duplications and cast them as lists.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, so why not store self._interpreter_constraints internally as a set, so that you can set.update etc it. and then in dump() you'd call list() to cast to list before serializing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good

self._requirements.update(other.requirements)

def dump(self, **kwargs):
pex_info_copy = self._pex_info.copy()
pex_info_copy['requirements'] = list(self._requirements)
pex_info_copy['interpreter_constraints'] = self._interpreter_constraints
pex_info_copy['distributions'] = self._distributions.copy()
return json.dumps(pex_info_copy, **kwargs)

Expand Down
23 changes: 23 additions & 0 deletions pex/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import contextlib
import os
import random
import subprocess
import sys
import tempfile
from collections import namedtuple
Expand Down Expand Up @@ -277,3 +278,25 @@ def combine_pex_coverage(coverage_file_iter):

combined.write()
return combined.filename


def bootstrap_python_installer():
install_location = os.getcwd() + '/.pyenv_test'
Copy link
Contributor

Choose a reason for hiding this comment

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

os.path.join(os.getcwd(), '.pyenv_test')

if not os.path.exists(install_location) or not os.path.exists(install_location + '/bin/pyenv'):
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

for _ in range(3):
try:
subprocess.call(['git', 'clone', 'https://github.com/pyenv/pyenv.git', install_location])
except StandardError:
continue
else:
break
else:
raise RuntimeError("Helper method could not clone pyenv from git")


def ensure_python_interpreter(version):
bootstrap_python_installer()
install_location = os.getcwd() + '/.pyenv_test/versions/' + version
if not os.path.exists(install_location):
os.environ['PYENV_ROOT'] = os.getcwd() + '/.pyenv_test'
subprocess.call([os.getcwd() + '/.pyenv_test/bin/pyenv', 'install', version])
Copy link
Contributor

Choose a reason for hiding this comment

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

os.path.join throughout this function.

33 changes: 33 additions & 0 deletions pex/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,18 @@ def PEX_PYTHON(self):
"""
return self._get_string('PEX_PYTHON', default=None)

@property
def PEX_PYTHON_PATH(self):
"""String

A colon-seperated string containing paths to binaires of blessed Python interpreters
Copy link
Contributor

Choose a reason for hiding this comment

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

binaries

for overriding the Python interpreter used to invoke this PEX. Must be absolute paths to the
interpreter.

Ex: "/path/to/python27:/path/to/python36"
"""
return self._get_string('PEX_PYTHON_PATH', default=None)

@property
def PEX_ROOT(self):
"""Directory
Expand Down Expand Up @@ -300,6 +312,27 @@ def PEX_IGNORE_RCFILES(self):
"""
return self._get_bool('PEX_IGNORE_RCFILES', default=False)

@property
def SHOULD_EXIT_BOOTSTRAP_REEXEC(self):
"""Boolean

Whether to re-exec in maybe_reexec_pex function of pex_bootstrapper.py. Default: false.
This is neccesary because that function relies on checking against variables present in the
ENV to determine whether to re-exec or return to current execution. When we introduced
interpreter constraints to a pex, these constraints can influence interpreter selection without
the need for a PEX_PYTHON or PEX_PYTHON_PATH ENV variable set. Since re-exec previously checked
for PEX_PYTHON or PEX_PYTHON_PATH but not constraints, this would result in a loop with no
stopping criteria. Setting SHOULD_EXIT_BOOTSTRAP_REEXEC will let the runtime know to break out
of a second execution of maybe_reexec_pex if neither PEX_PYTHON or PEX_PYTHON_PATH are set but
interpreter constraints are specified.
"""
value = self._environ.get('SHOULD_EXIT_REEXEC', 'False')
Copy link
Contributor

Choose a reason for hiding this comment

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

would expect the environ var to map 1:1 to the property name here.

return False if value == 'False' else True
Copy link
Contributor

Choose a reason for hiding this comment

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

would be better to not rely on the string value here as anything other than indicator of truthyness, i.e.:

return bool(self._environ.get(..., False))


@SHOULD_EXIT_BOOTSTRAP_REEXEC.setter
def SHOULD_EXIT_BOOTSTRAP_REEXEC(self, value):
self._environ['SHOULD_EXIT_REEXEC'] = str(value)


# Global singleton environment
ENV = Variables()
Loading