Modify pex bootstrapper to repsect pex_python_path environment variab…

…le, but maintain support for pex_python if still set. Add relevant integration tests to test each of the test cases that deal with interpreter constraints.

.gitignore

@@ -10,4 +10,4 @@
 /.idea
 /.coverage*
 /htmlcov
-/.pyenv
+/.pyenv_test

.travis.yml

@@ -10,10 +10,7 @@ dist: precise
 
 cache:
   directories:
-    - .pyenv/
-
-install:
-  - git clone https://github.com/pyenv/pyenv.git ./.pyenv
+    - .pyenv_test
 
 matrix:
   include:

pex/bin/pex.py

@@ -23,11 +23,7 @@
 from pex.http import Context
 from pex.installer import EggInstaller
 from pex.interpreter import PythonInterpreter
-from pex.interpreter_constraints import (
-    lowest_version_interpreter,
-    matched_interpreters,
-    parse_interpreter_constraints
-)
+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
@@ -295,13 +291,15 @@ def configure_clp_pex_environment(parser):
            'Default: Use current interpreter.')
 
   group.add_option(
-    '--interpreter-constraints',
-    dest='interpreter_constraints',
-    default='',
+    '--interpreter-constraint',
+    dest='interpreter_constraint',
+    default=[],
     type='str',
-    help='A comma-seperated 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.')
+    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',
@@ -531,9 +529,10 @@ def build_pex(args, options, resolver_option_builder):
       for interpreter in options.python or [None]
     ]
 
-  if options.interpreter_constraints:
-    safe_constraints = options.interpreter_constraints.strip("'").strip('"')
-    constraints = parse_interpreter_constraints(safe_constraints)
+  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)
     interpreters = list(matched_interpreters(interpreters, constraints, meet_all_constraints=True))
 
   if not interpreters:
@@ -546,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)
   pex_builder = PEXBuilder(path=safe_mkdtemp(), interpreter=interpreter, preamble=preamble)
 
   pex_info = pex_builder.info
@@ -555,8 +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_constraints:
-    pex_info.interpreter_constraints = safe_constraints
+  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]
 

pex/interpreter_constraints.py

@@ -26,7 +26,7 @@ def check_requirements_are_well_formed(constraints):
       PythonIdentity.parse_requirement(req)
     except ValueError as e:
       from .common import die
-      die("Compatibility requirements are not formatted properly: %s", str(e))
+      die("Compatibility requirements are not formatted properly: %s" % str(e))
 
 
 def matched_interpreters(interpreters, filters, meet_all_constraints=False):
@@ -42,43 +42,3 @@ def matched_interpreters(interpreters, filters, meet_all_constraints=False):
   """
   for match in _matching(interpreters, filters, meet_all_constraints):
     yield match
-
-
-def parse_interpreter_constraints(constraints_string):
-  """Given a single string defining interpreter constraints, separate them into a list of
-    individual constraint items for PythonIdentity to consume.
-
-    Example: '>=2.7, <3'
-    Return: ['>=2.7', '<3']
-
-    Example: 'CPython>=2.7,<3'
-    Return: ['CPython>=2.7', 'CPython<3']
-  """
-
-  if 'CPython' in constraints_string:
-    ret = list(map(lambda x: 'CPython' + x.strip() if 'CPython' not in x else x.strip(),
-      constraints_string.split(',')))
-  else:
-    ret = list(map(lambda x: x.strip(), constraints_string.split(',')))
-  check_requirements_are_well_formed(ret)
-  return ret
-
-
-def lowest_version_interpreter(interpreters):
-  """Given a list of interpreters, return the one with the lowest version."""
-  if not interpreters:
-    return None
-  lowest = interpreters[0]
-  for i in interpreters[1:]:
-    lowest = lowest if lowest < i else i
-  return lowest
-
-
-def highest_version_interpreter(interpreters):
-  """Given a list of interpreters, return the one with the highest version."""
-  if not interpreters:
-    return None
-  highest = interpreters[0]
-  for i in interpreters[1:]:
-    highest = i if highest < i else highest
-  return highest

pex/pex_bootstrapper.py

@@ -4,13 +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 (
-    highest_version_interpreter,
-    matched_interpreters,
-    parse_interpreter_constraints
-)
+from .interpreter_constraints import matched_interpreters
+from .tracer import TRACER
+from .variables import ENV
 
 __all__ = ('bootstrap_pex',)
 
@@ -52,46 +51,110 @@ def get_pex_info(entry_point):
   raise ValueError('Invalid entry_point: %s' % entry_point)
 
 
-def _get_python_interpreter(binary):
-  return PythonInterpreter.from_binary(binary)
+def find_in_path(target_interpreter):
+  if os.path.exists(target_interpreter):
+    return target_interpreter
+
+  for directory in os.getenv('PATH', '').split(os.pathsep):
+    try_path = os.path.join(directory, target_interpreter)
+    if os.path.exists(try_path):
+      return try_path
 
 
-def _find_compatible_interpreter_in_pex_python_path(target_python_path, compatibility_constraints):
-  parsed_compatibility_constraints = parse_interpreter_constraints(compatibility_constraints)
-  try_binaries = []
-  for binary in target_python_path.split(os.pathsep):
-    try_binaries.append(_get_python_interpreter(binary))
+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.
+  """
+  if pex_python_path:
+    interpreters = []
+    for binary in pex_python_path.split(os.pathsep):
+      pi = _get_python_interpreter(binary)
+      if pi:
+        interpreters.append(pi)
+    if not interpreters:
+      die('No interpreters from PEX_PYTHON_PATH can be found on the system. Exiting.')
+  else:
+    # All qualifying interpreters found in $PATH
+    interpreters = PythonInterpreter.all()
+
   compatible_interpreters = list(matched_interpreters(
-    try_binaries, parsed_compatibility_constraints, meet_all_constraints=True))
-  return highest_version_interpreter(compatible_interpreters)
+    interpreters, compatibility_constraints, meet_all_constraints=True))
+  return compatible_interpreters if compatible_interpreters else None
 
 
-def maybe_reexec_pex(compatibility_constraints=None):
-  from .variables import ENV
-  if not ENV.PEX_PYTHON_PATH or not compatibility_constraints:
-    return
+def _handle_pex_python(target_python, compatibility_constraints):
+  """Re-exec using the PEX_PYTHON interpreter"""
+  target = find_in_path(target_python)
+  if not target:
+    die('Failed to find interpreter specified by PEX_PYTHON: %s' % target)
+  elif compatibility_constraints:
+    pi = PythonInterpreter.from_binary(target)
+    if not all(pi.identity.matches(constraint) for constraint in compatibility_constraints):
+      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.SHOULD_EXIT_BOOTSTRAP_REEXEC = True
+    os.execve(target, [target_python] + sys.argv, ENV.copy())
+
 
-  from .common import die
-  from .tracer import TRACER
+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_python_path = ENV.PEX_PYTHON_PATH
-  lowest_version_compatible_interpreter = _find_compatible_interpreter_in_pex_python_path(
-    target_python_path, compatibility_constraints)
-  target = lowest_version_compatible_interpreter.binary
+  target = None
+  if compatible_interpreters:
+    target = min(compatible_interpreters).binary
   if not target:
-    die('Failed to find compatible interpreter in PEX_PYTHON_PATH for constraints: %s'
-        % compatibility_constraints)
+    die('Failed to find compatible interpreter for constraints: %s'
+        % str(compatibility_constraints))
   if os.path.exists(target) and os.path.realpath(target) != os.path.realpath(sys.executable):
-    TRACER.log('Detected PEX_PYTHON_PATH, re-exec to %s' % target)
-    ENV.delete('PEX_PYTHON_PATH')
+    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())
 
 
+def maybe_reexec_pex(compatibility_constraints=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
+    _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)
+
+
 def bootstrap_pex(entry_point):
   from .finders import register_finders
   register_finders()
   pex_info = get_pex_info(entry_point)
-  maybe_reexec_pex(pex_info.interpreter_constraints)
+  with TRACER.timed('Bootstrapping pex with maybe_reexec_pex', V=2):
+    maybe_reexec_pex(pex_info.interpreter_constraints)
 
   from . import pex
   pex.PEX(entry_point).execute()

pex/pex_builder.py

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

pex/pex_info.py

@@ -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))
@@ -197,18 +198,17 @@ def inherit_path(self, value):
 
   @property
   def interpreter_constraints(self):
-    """A comma-seperated list of constraints that determine the interpreter compatibility for this
+    """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._pex_info.get('interpreter_constraints', False)
+    return self._interpreter_constraints
 
-  @interpreter_constraints.setter
-  def interpreter_constraints(self, value):
-    self._pex_info['interpreter_constraints'] = value
+  def add_interpreter_constraint(self, value):
+    self._interpreter_constraints.append(str(value))
 
   @property
   def ignore_errors(self):
@@ -289,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)
     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)
 

pex/testing.py

@@ -19,7 +19,6 @@
 from .util import DistributionHelper, named_temporary_file
 
 
-
 @contextlib.contextmanager
 def temporary_dir():
   td = tempfile.mkdtemp()
@@ -282,15 +281,22 @@ def combine_pex_coverage(coverage_file_iter):
 
 
 def bootstrap_python_installer():
-  install_location = os.getcwd() + '/.pyenv'
-  if not os.path.exists(install_location):
-    p = subprocess.Popen(["git", "clone", 'https://github.com/pyenv/pyenv.git', install_location],
-      stdout=subprocess.PIPE)
-    print(p.communicate())
+  install_location = os.getcwd() + '/.pyenv_test'
+  if not os.path.exists(install_location) or not os.path.exists(install_location + '/bin/pyenv'):
+    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/versions/' + version
+  install_location = os.getcwd() + '/.pyenv_test/versions/' + version
   if not os.path.exists(install_location):
-    os.environ['PYENV_ROOT'] = os.getcwd() + '/.pyenv'
-    subprocess.call([os.getcwd() + '/.pyenv/bin/pyenv', 'install', version])
+    os.environ['PYENV_ROOT'] = os.getcwd() + '/.pyenv_test'
+    subprocess.call([os.getcwd() + '/.pyenv_test/bin/pyenv', 'install', version])