New-style BinaryTool Subsystems for isort and go distribution.

contrib/go/src/python/pants/contrib/go/subsystems/go_distribution.py

@@ -9,54 +9,28 @@
 from collections import OrderedDict, namedtuple
 
 from pants.base.workunit import WorkUnit, WorkUnitLabel
-from pants.binaries.binary_util import BinaryUtil
+from pants.binaries.binary_tool import NativeTool
 from pants.fs.archive import TGZ
-from pants.subsystem.subsystem import Subsystem
 from pants.util.contextutil import temporary_dir
 from pants.util.memo import memoized_property
 from pants.util.process_handler import subprocess
 
 
-class GoDistribution(object):
+class GoDistribution(NativeTool):
   """Represents a self-bootstrapping Go distribution."""
 
-  class Factory(Subsystem):
-    options_scope = 'go-distribution'
+  options_scope = 'go-distribution'
+  name = 'go'
+  default_version = '1.8.3'
+  archive_type = 'tgz'
 
-    @classmethod
-    def subsystem_dependencies(cls):
-      return (BinaryUtil.Factory,)
-
-    @classmethod
-    def register_options(cls, register):
-      register('--supportdir', advanced=True, default='bin/go',
-               help='Find the go distributions under this dir.  Used as part of the path to lookup '
-                    'the distribution with --binary-util-baseurls and --pants-bootstrapdir')
-      register('--version', advanced=True, default='1.8.3', fingerprint=True,
-               help='Go distribution version.  Used as part of the path to lookup the distribution '
-                    'with --binary-util-baseurls and --pants-bootstrapdir')
-
-    def create(self):
-      # NB: create is an instance method to allow the user to choose global or scoped.
-      # It's not unreasonable to imagine multiple go versions in play; for example: when
-      # transitioning from the 1.x series to the 2.x series.
-      binary_util = BinaryUtil.Factory.create()
-      options = self.get_options()
-      return GoDistribution(binary_util, options.supportdir, options.version)
-
-  def __init__(self, binary_util, relpath, version):
-    self._binary_util = binary_util
-    self._relpath = relpath
-    self._version = version
-
-  @property
-  def version(self):
-    """Returns the version of the Go distribution.
-
-    :returns: The Go distribution version number string.
-    :rtype: string
-    """
-    return self._version
+  @classmethod
+  def register_options(cls, register):
+    super(GoDistribution, cls).register_options(register)
+    register('--supportdir', advanced=True, default='bin/go',
+             removal_version='1.7.0.dev0', removal_hint='No longer supported.',
+             help='Find the go distributions under this dir.  Used as part of the path to lookup '
+                  'the distribution with --binary-util-baseurls and --pants-bootstrapdir')
 
   @memoized_property
   def goroot(self):
@@ -65,7 +39,7 @@ def goroot(self):
     :returns: The Go distribution $GOROOT.
     :rtype: string
     """
-    go_distribution = self._binary_util.select_binary(self._relpath, self.version, 'go.tar.gz')
+    go_distribution = self.select()
     distribution_workdir = os.path.dirname(go_distribution)
     outdir = os.path.join(distribution_workdir, 'unpacked')
     if not os.path.exists(outdir):
@@ -89,14 +63,14 @@ class GoCommand(namedtuple('GoCommand', ['cmdline', 'env'])):
     """Encapsulates a go command that can be executed."""
 
     @classmethod
-    def _create(cls, goroot, cmd, go_env, args=None):
+    def create(cls, goroot, cmd, go_env, args=None):
       return cls([os.path.join(goroot, 'bin', 'go'), cmd] + (args or []), env=go_env)
 
     def spawn(self, env=None, **kwargs):
       """
       :param dict env: A custom environment to launch the Go command in.  If `None` the current
                        environment is used.
-      :param **kwargs: Keyword arguments to pass through to `subprocess.Popen`.
+      :param kwargs: Keyword arguments to pass through to `subprocess.Popen`.
       :returns: A handle to the spawned go command subprocess.
       :rtype: :class:`subprocess.Popen`
       """
@@ -109,7 +83,7 @@ def check_output(self, env=None, **kwargs):
 
       :param dict env: A custom environment to launch the Go command in.  If `None` the current
                        environment is used.
-      :param **kwargs: Keyword arguments to pass through to `subprocess.check_output`.
+      :param kwargs: Keyword arguments to pass through to `subprocess.check_output`.
       :return str: Output of Go command.
       :raises subprocess.CalledProcessError: Raises if Go command fails.
       """
@@ -132,7 +106,7 @@ def create_go_cmd(self, cmd, gopath=None, args=None):
     :returns: A go command that can be executed later.
     :rtype: :class:`GoDistribution.GoCommand`
     """
-    return self.GoCommand._create(self.goroot, cmd, go_env=self.go_env(gopath=gopath), args=args)
+    return self.GoCommand.create(self.goroot, cmd, go_env=self.go_env(gopath=gopath), args=args)
 
   def execute_go_cmd(self, cmd, gopath=None, args=None, env=None,
                      workunit_factory=None, workunit_name=None, workunit_labels=None, **kwargs):
@@ -149,11 +123,11 @@ def execute_go_cmd(self, cmd, gopath=None, args=None, env=None,
     :param workunit_factory: An optional callable that can produce a `WorkUnit` context
     :param string workunit_name: An optional name for the work unit; defaults to the `cmd`
     :param list workunit_labels: An optional sequence of labels for the work unit.
-    :param **kwargs: Keyword arguments to pass through to `subprocess.Popen`.
+    :param kwargs: Keyword arguments to pass through to `subprocess.Popen`.
     :returns: A tuple of the exit code and the go command that was run.
     :rtype: (int, :class:`GoDistribution.GoCommand`)
     """
-    go_cmd = self.GoCommand._create(self.goroot, cmd, go_env=self.go_env(gopath=gopath), args=args)
+    go_cmd = self.GoCommand.create(self.goroot, cmd, go_env=self.go_env(gopath=gopath), args=args)
     if workunit_factory is None:
       return go_cmd.spawn(**kwargs).wait()
     else:

contrib/go/src/python/pants/contrib/go/tasks/go_task.py

@@ -27,7 +27,7 @@ class GoTask(Task):
 
   @classmethod
   def subsystem_dependencies(cls):
-    return super(GoTask, cls).subsystem_dependencies() + (GoDistribution.Factory,)
+    return super(GoTask, cls).subsystem_dependencies() + (GoDistribution.scoped(cls),)
 
   @staticmethod
   def is_binary(target):
@@ -51,7 +51,7 @@ def is_go(target):
 
   @memoized_property
   def go_dist(self):
-    return GoDistribution.Factory.global_instance().create()
+    return GoDistribution.scoped_instance(self)
 
   @memoized_property
   def import_oracle(self):

contrib/go/tests/python/pants_test/contrib/go/subsystems/test_go_distribution.py

@@ -18,8 +18,7 @@
 class GoDistributionTest(unittest.TestCase):
 
   def distribution(self):
-    factory = global_subsystem_instance(GoDistribution.Factory)
-    return factory.create()
+    return global_subsystem_instance(GoDistribution)
 
   def test_bootstrap(self):
     go_distribution = self.distribution()

contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_compile_integration.py

@@ -22,8 +22,7 @@ def test_go_compile_simple(self):
               'contrib/go/examples/src/go/libA']
       pants_run = self.run_pants_with_workdir(args, workdir)
       self.assert_success(pants_run)
-      factory = global_subsystem_instance(GoDistribution.Factory)
-      go_dist = factory.create()
+      go_dist = global_subsystem_instance(GoDistribution)
       goos = go_dist.create_go_cmd('env', args=['GOOS']).check_output().strip()
       goarch = go_dist.create_go_cmd('env', args=['GOARCH']).check_output().strip()
       expected_files = set('contrib.go.examples.src.go.{libname}.{libname}/'

src/python/pants/backend/python/subsystems/isort.py

@@ -0,0 +1,17 @@
+# coding=utf-8
+# Copyright 2018 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 pants.binaries.binary_tool import Script
+
+
+class Isort(Script):
+  options_scope = 'isort'
+  default_version = '4.2.5'
+  suffix = '.pex'
+
+  replaces_scope = 'fmt.isort'
+  replaces_name = 'version'

src/python/pants/backend/python/tasks/BUILD

@@ -16,7 +16,6 @@ python_library(
     'src/python/pants/base:fingerprint_strategy',
     'src/python/pants/base:hash_utils',
     'src/python/pants/base:specs',
-    'src/python/pants/binaries:thrift_util',
     'src/python/pants/build_graph',
     'src/python/pants/invalidation',
     'src/python/pants/python',

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

@@ -8,17 +8,18 @@
 import logging
 import os
 
+from pants.backend.python.subsystems.isort import Isort
 from pants.backend.python.targets.python_binary import PythonBinary
 from pants.backend.python.targets.python_library import PythonLibrary
 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.binaries.binary_util import BinaryUtil
 from pants.task.fmt_task_mixin import FmtTaskMixin
 from pants.task.task import Task
 from pants.util.process_handler import subprocess
 
 
+# TODO: Should be named IsortRun, for consistency.
 class IsortPythonTask(FmtTaskMixin, Task):
   """Autoformats Python source files with isort.
 
@@ -40,7 +41,7 @@ class IsortPythonTask(FmtTaskMixin, Task):
 
   @classmethod
   def subsystem_dependencies(cls):
-    return super(IsortPythonTask, cls).subsystem_dependencies() + (BinaryUtil.Factory.scoped(cls), )
+    return super(IsortPythonTask, cls).subsystem_dependencies() + (Isort, )
 
   def __init__(self, *args, **kwargs):
     super(IsortPythonTask, self).__init__(*args, **kwargs)
@@ -50,6 +51,7 @@ def __init__(self, *args, **kwargs):
   def register_options(cls, register):
     super(IsortPythonTask, cls).register_options(register)
     register('--version', advanced=True, fingerprint=True, default='4.2.5',
+             removal_version='1.7.0.dev0', removal_hint='Use --version in scope isort',
              help='Version of isort.')
 
   def execute(self, test_output_file=None):
@@ -60,8 +62,7 @@ def execute(self, test_output_file=None):
       logging.debug(self.NOOP_MSG_HAS_TARGET_BUT_NO_SOURCE)
       return
 
-    isort_script = BinaryUtil.Factory.create().select_script('scripts/isort',
-                                                             self.options.version, 'isort.pex')
+    isort_script = Isort.global_instance().select(context=self.context)
     cmd = [isort_script] + self.get_passthru_args() + sources
     logging.debug(' '.join(cmd))
 

src/python/pants/binaries/binary_tool.py

@@ -20,13 +20,17 @@ class BinaryToolBase(Subsystem):
   # Subclasses must set these to appropriate values for the tool they define.
   # They must also set options_scope appropriately.
   platform_dependent = None
-  archive_type = None
+  archive_type = None  # See pants.fs.archive.archive for valid string values.
   default_version = None
 
   # Subclasses may set this to the tool name as understood by BinaryUtil.
   # If unset, it defaults to the value of options_scope.
   name = None
 
+  # Subclasses may set this to a suffix (e.g., '.pex') to add to the computed remote path.
+  # Note that setting archive_type will add an appropriate archive suffix after this suffix.
+  suffix = ''
+
   # Subclasses may set these to effect migration from an old --version option to this one.
   # TODO(benjy): Remove these after migration to the mixin is complete.
   replaces_scope = None
@@ -103,7 +107,7 @@ def _select_for_version(self, version):
     return self._binary_util.select(
       supportdir=self.get_support_dir(),
       version=version,
-      name=self._get_name(),
+      name='{}{}'.format(self._get_name(), self.suffix),
       platform_dependent=self.platform_dependent,
       archive_type=self.archive_type)