-
-
Notifications
You must be signed in to change notification settings - Fork 291
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
Changes from 42 commits
460a9ae
45e4640
7ae3b61
a6175dd
528d93e
4fc655d
ca8a9c6
02fa8c5
64c2b0c
3408fb3
a1c4fed
cef5bf5
2957b80
0877ad0
762e9d3
ebf830a
233918e
c0ef007
0ffe8c2
b5a86ea
bcfd6c4
d5566b5
350deb9
ce099b6
155d18d
ff3e569
b7d8510
0b3764d
5832b10
2b5012d
575a8ad
ce0dac6
a2db930
1150b65
3a143d1
b045ba5
66ef8ee
6a2bd58
8311619
e7590c2
6b3e551
34773e5
508226f
524456b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,3 +10,4 @@ | |
/.idea | ||
/.coverage* | ||
/htmlcov | ||
/.pyenv_test |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -289,6 +291,25 @@ 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', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: for consistency, change option to |
||
dest='rc_file', | ||
default=None, | ||
help='A path to a pexrc file. NOTE: this flag is for testing purposes only. It is to ' | ||
'be used in the case that a pexrc lives in the same directory as the created ' | ||
'output pex as is the case with the pex integration tests.') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about shortening this to:
|
||
|
||
group.add_option( | ||
'--python-shebang', | ||
dest='python_shebang', | ||
|
@@ -507,14 +528,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 = [ | ||
|
@@ -525,6 +538,18 @@ 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) | ||
pexrc = None | ||
if options.rc_file: | ||
pexrc = options.rc_file | ||
rc_variables = Variables.from_rc(rc=pexrc) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given
|
||
pex_python_path = rc_variables.get('PEX_PYTHON_PATH', '') | ||
interpreters = find_compatible_interpreters(pex_python_path, constraints) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems to me like in the case where no this would mean that anytime a user passes I think we'll probably need to:
this will pave a way for a future where both build and runtime interpreter selection behave identically, afaict. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
||
|
@@ -535,7 +560,8 @@ def build_pex(args, options, resolver_option_builder): | |
# options.preamble_file is None | ||
preamble = None | ||
|
||
interpreter = _lowest_version_interpreter(interpreters) | ||
interpreter = min(interpreters) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -544,6 +570,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] | ||
|
||
|
@@ -605,6 +634,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.') | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/--interpreter constraint/--interpreter-constraint/ |
||
if options.pex_root: | ||
ENV.set('PEX_ROOT', options.pex_root) | ||
else: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this get bumped to 2017? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 .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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did this TODO exist already when you moved it? If not, can we start an issue for it to track? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ |
||
# Check that the compatibility requirements are well-formed. | ||
for req in constraints: | ||
try: | ||
PythonIdentity.parse_requirement(req) | ||
except ValueError as e: | ||
die("Compatibility requirements are not formatted properly: %s" % str(e)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should probably directly quote the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ValueError raised by the exception covers this. |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. specify the requirement that failed to parse's string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. second. Including the failed string will help user to debug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the |
||
|
||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this param be called There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ |
||
""" | ||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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',) | ||
|
||
|
@@ -56,28 +61,107 @@ 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. | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/env variable// |
||
if pex_python_path: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
try: | ||
interpreters.append(PythonInterpreter.from_binary(binary)) | ||
except Executor.ExecutionError: | ||
TRACER.log("Python interpreter %s in PEX_PYTHON_PATH failed to load properly." % binary) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should probably get logged as a hard warning (i.e. |
||
if not interpreters: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe more readable?
|
||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like this
unless its possible for |
||
# 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. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Start an issue for this TODO? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know we discussed this earlier - but what was the ultimate reasoning here to leave I was sort of thinking we could yank There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree about the simplification. We discussed leaving it in as a way to give us flexibility when migrating the /etc/pexrc configs in prod and in MDE, as well as provide backwards interoperability for pex users who want to continue using a single python interpreter. However, IIRC, I think the main reason was to give us flexibility with the whole migration of /etc/pexrc and to give us a safety net in case we find that PEX_PYTHON_PATH ends up disastrously breaking things or causing other unexpected problems. I like the idea of a safety net and backwards compatibility (at least for now), and I think it wouldn't be a bad idea to keep it in for now until we have success using PPP internally, maybe with a reasonable subset of Python users successfully running and deploying py3 PEX both locally and in prod? What do you think? @UnrememberMe any thoughts here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. works for me. in that case, would be good to link to a ticket here that covers the eventual removal of |
||
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()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. won't this become There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the first arg of |
||
|
||
|
||
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() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,23 +51,23 @@ class PexInfo(object): | |
INTERNAL_CACHE = '.deps' | ||
|
||
@classmethod | ||
def make_build_properties(cls): | ||
def make_build_properties(cls, interpreter=None): | ||
from .interpreter import PythonInterpreter | ||
from pkg_resources import get_platform | ||
|
||
pi = PythonInterpreter.get() | ||
pi = interpreter or PythonInterpreter.get() | ||
return { | ||
'class': pi.identity.interpreter, | ||
'version': pi.identity.version, | ||
'platform': get_platform(), | ||
} | ||
|
||
@classmethod | ||
def default(cls): | ||
def default(cls, interpreter=None): | ||
pex_info = { | ||
'requirements': [], | ||
'distributions': {}, | ||
'build_properties': cls.make_build_properties(), | ||
'build_properties': cls.make_build_properties(interpreter), | ||
} | ||
return cls(info=pex_info) | ||
|
||
|
@@ -123,6 +123,8 @@ def __init__(self, info=None): | |
'%s of type %s' % (info, type(info))) | ||
self._pex_info = info or {} | ||
self._distributions = self._pex_info.get('distributions', {}) | ||
# cast as set because pex info from json must store interpreter_constraints as a list | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I understand this comment, or maybe it's a typo. You're casting it as a set, but interpreter constraints needs to be stored as a list? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading in a pex info using |
||
self._interpreter_constraints = set(self._pex_info.get('interpreter_constraints', set())) | ||
requirements = self._pex_info.get('requirements', []) | ||
if not isinstance(requirements, (list, tuple)): | ||
raise ValueError('Expected requirements to be a list, got %s' % type(requirements)) | ||
|
@@ -195,6 +197,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 list(self._interpreter_constraints) | ||
|
||
def add_interpreter_constraint(self, value): | ||
self._interpreter_constraints.add(str(value)) | ||
|
||
@property | ||
def ignore_errors(self): | ||
return self._pex_info.get('ignore_errors', False) | ||
|
@@ -274,11 +290,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.update(other.interpreter_constraints) | ||
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'] = list(self._interpreter_constraints) | ||
pex_info_copy['distributions'] = self._distributions.copy() | ||
return json.dumps(pex_info_copy, **kwargs) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a comment might be good here to indicate to the next person to come along why this is/was needed.