Simplify PythonSetup usage

build-support/bin/ci.sh

@@ -97,13 +97,14 @@ fi
 
 # TODO(John sirois): Re-plumb build such that it grabs constraints from the built python_binary
 # target(s).
+# TODO: This doesn't seem necessary? We set this in pants.ini.
 INTERPRETER_CONSTRAINTS=(
   "CPython>=2.7,<3"
 )
 for constraint in ${INTERPRETER_CONSTRAINTS[@]}; do
   INTERPRETER_ARGS=(
     ${INTERPRETER_ARGS[@]}
-    --interpreter="${constraint}"
+    --python-setup-interpreter-constraints="${constraint}"
   )
 done
 

pants.ini

@@ -274,7 +274,7 @@ restrict_push_urls: [
 # We only support pants running under 2.7 for now with 3.3+ support to be added later.
 # Any example or test targets that are meant to test interpreters outside pants own
 # acceptable set should specify an explicit compatibility constraint.
-interpreter_requirement: CPython>=2.7,<3
+interpreter_constraints: ["CPython>=2.7,<3"]
 interpreter_cache_dir: %(pants_bootstrapdir)s/python_cache/interpreters
 resolver_cache_dir: %(pants_bootstrapdir)s/python_cache/requirements
 

src/python/pants/backend/python/interpreter_cache.py

@@ -54,7 +54,6 @@ def __init__(self, python_setup, python_repos, logger=None):
     safe_mkdir(self._cache_dir)
     self._interpreters = set()
     self._logger = logger or (lambda msg: True)
-    self._default_filters = (python_setup.interpreter_requirement or b'',)
 
   @property
   def interpreters(self):
@@ -145,7 +144,10 @@ def setup(self, paths=(), force=False, filters=(b'',)):
       cache, using the Requirement-style format, e.g. ``'CPython>=3', or just ['>=2.7','<3']``
       for requirements agnostic to interpreter class.
     """
-    filters = self._default_filters if not any(filters) else filters
+    # We filter the interpreter cache itself (and not just the interpreters we pull from it)
+    # because setting up some python versions (e.g., 3<=python<3.3) crashes, and this gives us
+    # an escape hatch.
+    filters = self._python_setup.interpreter_constraints if not any(filters) else filters
     setup_paths = paths or os.getenv('PATH').split(os.pathsep)
     self._setup_cached(filters)
     def unsatisfied_filters():

src/python/pants/backend/python/python_chroot.py

@@ -249,7 +249,8 @@ def _resolve_multi(self, requirements, find_links):
     context = self._python_repos.get_network_context()
 
     for platform in platforms:
-      requirements_cache_dir = os.path.join(self._python_setup.resolver_cache_dir, str(self._interpreter.identity))
+      requirements_cache_dir = os.path.join(self._python_setup.resolver_cache_dir,
+                                            str(self._interpreter.identity))
       distributions[platform] = resolve(
         requirements=[req.requirement for req in requirements],
         interpreter=self._interpreter,

src/python/pants/backend/python/subsystems/BUILD

@@ -3,6 +3,7 @@
 
 python_library(
   dependencies = [
+    '3rdparty/python:setuptools',
     'src/python/pants/subsystem',
   ],
 )

src/python/pants/python/python_setup.py → src/python/pants/backend/python/subsystems/python_setup.py

@@ -7,8 +7,6 @@
 
 import os
 
-from pex.fetcher import Fetcher, PyPIFetcher
-from pex.http import Context
 from pkg_resources import Requirement
 
 from pants.subsystem.subsystem import Subsystem
@@ -21,8 +19,23 @@ class PythonSetup(Subsystem):
   @classmethod
   def register_options(cls, register):
     super(PythonSetup, cls).register_options(register)
+    # TODO: On removal, make ['CPython>=2.7,<3'] the default for --interpreter-constraints.
     register('--interpreter-requirement', advanced=True, default='CPython>=2.7,<3',
+             removal_version='1.5.0.dev0', removal_hint='Use --interpreter-constraints instead.',
              help='The interpreter requirement string for this python environment.')
+    # Note: This will replace two options:
+    # A) The global --interpreter option in the old python tasks.
+    #    That flag is only relevant in the python backend, and should never have been
+    #    global to begin with.
+    # B) The --interpreter-requirement option above.  That flag merely served to set the
+    #    effective default for when no other constraints were set, so we might as well
+    #    roll that into the more general constraints.
+    register('--interpreter-constraints', advanced=True, default=[], type=list,
+             metavar='<requirement>',
+             help="Constrain the selected Python interpreter.  Specify with requirement syntax, "
+                  "e.g. 'CPython>=2.6,<3' or 'PyPy'. Multiple constraints will be ORed together. "
+                  "These constraints are applied in addition to any compatibilities required by "
+                  "the relevant targets.")
     register('--setuptools-version', advanced=True, default='30.0.0',
              help='The setuptools version for this python environment.')
     register('--wheel-version', advanced=True, default='0.29.0',
@@ -50,8 +63,9 @@ def register_options(cls, register):
                   'If unspecified, a standard path under the workdir is used.')
 
   @property
-  def interpreter_requirement(self):
-    return self.get_options().interpreter_requirement
+  def interpreter_constraints(self):
+    return (self.get_options().interpreter_constraints or self.get_options().interpreter or
+            [self.get_options().interpreter_requirement or b''])
 
   @property
   def setuptools_version(self):
@@ -115,34 +129,3 @@ def _failsafe_parse(self, requirement):
       return Requirement.parse(requirement, replacement=False)
     except TypeError:
       return Requirement.parse(requirement)
-
-
-class PythonRepos(Subsystem):
-  """A python code repository."""
-  options_scope = 'python-repos'
-
-  @classmethod
-  def register_options(cls, register):
-    super(PythonRepos, cls).register_options(register)
-    register('--repos', advanced=True, type=list, default=[], fingerprint=True,
-             help='URLs of code repositories.')
-    register('--indexes', advanced=True, type=list, fingerprint=True,
-             default=['https://pypi.python.org/simple/'], help='URLs of code repository indexes.')
-
-  @property
-  def repos(self):
-    return self.get_options().repos
-
-  @property
-  def indexes(self):
-    return self.get_options().indexes
-
-  def get_fetchers(self):
-    fetchers = []
-    fetchers.extend(Fetcher([url]) for url in self.repos)
-    fetchers.extend(PyPIFetcher(url) for url in self.indexes)
-    return fetchers
-
-  def get_network_context(self):
-    # TODO(wickman): Add retry, conn_timeout, threads, etc configuration here.
-    return Context.get()

src/python/pants/backend/python/tasks/python_task.py

@@ -16,11 +16,12 @@
 
 from pants.backend.python.interpreter_cache import PythonInterpreterCache
 from pants.backend.python.python_chroot import PythonChroot
+from pants.backend.python.subsystems.python_setup import PythonSetup
 from pants.base import hash_utils
 from pants.binaries.thrift_binary import ThriftBinary
 from pants.ivy.bootstrapper import Bootstrapper
 from pants.ivy.ivy_subsystem import IvySubsystem
-from pants.python.python_setup import PythonRepos, PythonSetup
+from pants.python.python_repos import PythonRepos
 from pants.task.task import Task
 
 
@@ -39,7 +40,6 @@ def subsystem_dependencies(cls):
 
   def __init__(self, *args, **kwargs):
     super(PythonTask, self).__init__(*args, **kwargs)
-    self._compatibilities = self.get_options().interpreter or [b'']
     self._interpreter_cache = None
     self._interpreter = None
 
@@ -53,9 +53,7 @@ def interpreter_cache(self):
       # Cache setup's requirement fetching can hang if run concurrently by another pants proc.
       self.context.acquire_lock()
       try:
-        # We pass in filters=compatibilities because setting up some python versions
-        # (e.g., 3<=python<3.3) crashes, and this gives us an escape hatch.
-        self._interpreter_cache.setup(filters=self._compatibilities)
+        self._interpreter_cache.setup()
       finally:
         self.context.release_lock()
     return self._interpreter_cache

src/python/pants/backend/python/tasks2/BUILD

@@ -7,6 +7,7 @@ python_library(
     '3rdparty/python/twitter/commons:twitter.common.collections',
     '3rdparty/python/twitter/commons:twitter.common.dirutil',
     'src/python/pants/backend/python:interpreter_cache',
+    'src/python/pants/backend/python/subsystems',
     'src/python/pants/backend/python/targets',
     'src/python/pants/base:build_environment',
     'src/python/pants/base:exceptions',

src/python/pants/backend/python/tasks2/pex_build_util.py

@@ -12,14 +12,15 @@
 from pex.resolver import resolve
 from twitter.common.collections import OrderedSet
 
+from pants.backend.python.subsystems.python_setup import PythonSetup
 from pants.backend.python.targets.python_binary import PythonBinary
 from pants.backend.python.targets.python_library import PythonLibrary
 from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
 from pants.backend.python.targets.python_tests import PythonTests
 from pants.base.build_environment import get_buildroot
 from pants.base.exceptions import TaskError
 from pants.build_graph.resources import Resources
-from pants.python.python_setup import PythonRepos, PythonSetup
+from pants.python.python_repos import PythonRepos
 
 
 def has_python_sources(tgt):

src/python/pants/backend/python/tasks2/select_interpreter.py

@@ -11,10 +11,11 @@
 from pex.interpreter import PythonIdentity, PythonInterpreter
 
 from pants.backend.python.interpreter_cache import PythonInterpreterCache
+from pants.backend.python.subsystems.python_setup import PythonSetup
 from pants.backend.python.targets.python_target import PythonTarget
 from pants.base.fingerprint_strategy import DefaultFingerprintHashingMixin, FingerprintStrategy
 from pants.invalidation.cache_manager import VersionedTargetSet
-from pants.python.python_setup import PythonRepos, PythonSetup
+from pants.python.python_repos import PythonRepos
 from pants.task.task import Task
 from pants.util.dirutil import safe_mkdir_for
 
@@ -47,19 +48,6 @@ def subsystem_dependencies(cls):
   def product_types(cls):
     return [PythonInterpreter]
 
-  @classmethod
-  def register_options(cls, register):
-    super(SelectInterpreter, cls).register_options(register)
-    # Note: This replaces the global --interpreter flag in the old python tasks.
-    # That flag is only relevant in the python backend, and should never have been
-    # global to begin with.
-    register('--constraints', advanced=True, default=[], type=list,
-             metavar='<requirement>',
-             help="Constrain the selected Python interpreter.  Specify with requirement syntax, "
-                  "e.g. 'CPython>=2.6,<3' or 'PyPy'. Multiple constraints will be ORed together. "
-                  "These constraints are applied in addition to any compatibilities required by "
-                  "the relevant targets.")
-
   def execute(self):
     interpreter = None
     python_tgts = self.context.targets(lambda tgt: isinstance(tgt, PythonTarget))
@@ -78,15 +66,10 @@ def execute(self):
                                                    PythonRepos.global_instance(),
                                                    logger=self.context.log.debug)
 
-        # We filter the interpreter cache itself (and not just the interpreters we pull from it)
-        # because setting up some python versions (e.g., 3<=python<3.3) crashes, and this gives us
-        # an escape hatch.
-        filters = self.get_options().constraints or [b'']
-
         # Cache setup's requirement fetching can hang if run concurrently by another pants proc.
         self.context.acquire_lock()
         try:
-          interpreter_cache.setup(filters=filters)
+          interpreter_cache.setup()
         finally:
           self.context.release_lock()
 

src/python/pants/bin/BUILD

@@ -68,7 +68,7 @@ python_library(
     '3rdparty/python:setuptools',
     'src/python/pants/option:option',
     'src/python/pants/python',
-    'src/python/pants/subsystem:subsystem',
+    'src/python/pants/subsystem',
     'src/python/pants/util:dirutil',
     'src/python/pants/util:memo',
     'src/python/pants:version',

src/python/pants/bin/plugin_resolver.py

@@ -16,7 +16,7 @@
 from pkg_resources import Requirement
 
 from pants.option.global_options import GlobalOptionsRegistrar
-from pants.python.python_setup import PythonRepos, PythonSetup
+from pants.python.python_repos import PythonRepos
 from pants.subsystem.subsystem import Subsystem
 from pants.util.dirutil import safe_open
 from pants.util.memo import memoized_property
@@ -80,14 +80,13 @@ def _resolve_plugins(self):
     # guaranteed a properly initialized interpreter with wheel support so we enforce eggs only for
     # bdists with this custom precedence.
     precedence = (EggPackage, SourcePackage)
-
     logger.info('Resolving new plugins...:\n  {}'.format('\n  '.join(self._plugin_requirements)))
     return resolver.resolve(self._plugin_requirements,
                             fetchers=self._python_repos.get_fetchers(),
                             context=self._python_repos.get_network_context(),
                             precedence=precedence,
                             cache=self.plugin_cache_dir,
-                            cache_ttl=self._python_setup.resolver_cache_ttl,
+                            cache_ttl=10 * 365 * 24 * 60 * 60,  # Effectively never expire.
                             allow_prereleases=PANTS_SEMVER.is_prerelease)
 
   @memoized_property
@@ -99,10 +98,6 @@ def plugin_cache_dir(self):
   def _python_repos(self):
     return self._create_global_subsystem(PythonRepos)
 
-  @memoized_property
-  def _python_setup(self):
-    return self._create_global_subsystem(PythonSetup)
-
   def _create_global_subsystem(self, subsystem_type):
     options_scope = subsystem_type.options_scope
     return subsystem_type(options_scope, self._options.for_scope(options_scope))
@@ -111,9 +106,9 @@ def _create_global_subsystem(self, subsystem_type):
   def _options(self):
     # NB: The PluginResolver runs very early in the pants startup sequence before the standard
     # Subsystem facility is wired up.  As a result PluginResolver is not itself a Subsystem with
-    # (PythonRepos, PythonSetup) returned as `dependencies()`.  Instead it does the minimum possible
-    # work to hand-roll bootstrapping of the Subsystems it needs.
-    subsystems = Subsystem.closure([PythonRepos, PythonSetup])
+    # PythonRepos as a dependency.  Instead it does the minimum possible work to hand-roll
+    # bootstrapping of the Subsystem it needs.
+    subsystems = Subsystem.closure([PythonRepos])
     known_scope_infos = [subsystem.get_scope_info() for subsystem in subsystems]
     options = self._options_bootstrapper.get_full_options(known_scope_infos)
 

src/python/pants/option/global_options.py

@@ -154,6 +154,8 @@ def register_options(cls, register):
              help='Kill nailguns before exiting')
     register('-i', '--interpreter', advanced=True, default=[], type=list,
              metavar='<requirement>',
+             removal_version='1.5.0.dev0',
+             removal_hint='Use --interpreter-constraints in scope python-setup instead.',
              help="Constrain what Python interpreters to use.  Uses Requirement format from "
                   "pkg_resources, e.g. 'CPython>=2.6,<3' or 'PyPy'. By default, no constraints "
                   "are used.  Multiple constraints may be added.  They will be ORed together.")

src/python/pants/python/BUILD

@@ -4,7 +4,6 @@
 python_library(
   dependencies = [
     '3rdparty/python:pex',
-    '3rdparty/python:setuptools',
     'src/python/pants/subsystem',
   ]
 )

src/python/pants/python/python_repos.py

@@ -0,0 +1,42 @@
+# coding=utf-8
+# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+from pex.fetcher import Fetcher, PyPIFetcher
+from pex.http import Context
+
+from pants.subsystem.subsystem import Subsystem
+
+
+class PythonRepos(Subsystem):
+  """A python code repository."""
+  options_scope = 'python-repos'
+
+  @classmethod
+  def register_options(cls, register):
+    super(PythonRepos, cls).register_options(register)
+    register('--repos', advanced=True, type=list, default=[], fingerprint=True,
+             help='URLs of code repositories.')
+    register('--indexes', advanced=True, type=list, fingerprint=True,
+             default=['https://pypi.python.org/simple/'], help='URLs of code repository indexes.')
+
+  @property
+  def repos(self):
+    return self.get_options().repos
+
+  @property
+  def indexes(self):
+    return self.get_options().indexes
+
+  def get_fetchers(self):
+    fetchers = []
+    fetchers.extend(Fetcher([url]) for url in self.repos)
+    fetchers.extend(PyPIFetcher(url) for url in self.indexes)
+    return fetchers
+
+  def get_network_context(self):
+    # TODO(wickman): Add retry, conn_timeout, threads, etc configuration here.
+    return Context.get()