Convert two existing concepts (RunTracker and the jar tool) to be sub…

…systems.

- The RunTracker change is straightforward.

- There is now a JarTool subsystem.  But implementing this neatly
  required some changes to the JVM tool machinery:

  + The old JvmToolTaskMixin is now JvmToolMixin. I.e., you don't have
    to be a task to use JVM tools. Indeed, subsystems can mix this in
    in order to register and bootstrap JVM tools.

  + JvmToolTaskMixin is now a thin subclass of JvmToolMixin, adapting
    it for ease of use by tasks. In the future we might make all
    JVM tools (compilers etc.) into subsystems and then not need
    JvmToolTaskMixin at all.

  + The new JarTool subsystem does two things:
      1) Bootstraps the jar tool and handles its options.
      2) Actually runs the jar tool with given args in a given runner.
         This interface is slightly awkward perhaps, but very reusable.
         And we already pass runners around as arguments elsewhere anyway.

- Removed some clunky code in JvmToolTaskTestBase that registers options
  for BootstrapJvmTools, which is not the task under test but is needed
  to bootstrap tools for the task under test.  Instead overrides context() to
  add BootstrapJvmTools to the list of tasks the base class already clunkily
  registers options for (in addition to the task under test)...

- TODO: JarTool is now tested indirectly in test_jar_task.py,
  but we might want to also test the now-standalone subsystem directly.

Testing Done:
CI passes: https://travis-ci.org/pantsbuild/pants/builds/58631095

Reviewed at https://rbcommons.com/s/twitter/r/2081/

src/python/pants/backend/core/tasks/reflect.py

@@ -438,6 +438,7 @@ def gen_glopts_reference_data():
   def register(*args, **kwargs):
     option_parser.register(*args, **kwargs)
   register.bootstrap = bootstrap_option_values()
+  register.scope = ''
   register_bootstrap_options(register, buildroot='<buildroot>')
   register_global_options(register)
   argparser = option_parser._help_argparser

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

@@ -0,0 +1,22 @@
+# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+python_library(
+  name = 'jvm_tool_mixin',
+  sources = ['jvm_tool_mixin.py'],
+  dependencies = [
+    'src/python/pants/base:exceptions',
+    'src/python/pants/option',
+    ],
+  )
+
+python_library(
+  name = 'jar_tool',
+  sources = ['jar_tool.py'],
+  dependencies = [
+    ':jvm_tool_mixin',
+    'src/python/pants/base:workunit',
+    'src/python/pants/option',
+    'src/python/pants/subsystem',
+    ],
+  )

src/python/pants/backend/jvm/subsystems/jar_tool.py

@@ -0,0 +1,34 @@
+# coding=utf-8
+# Copyright 2015 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.backend.jvm.subsystems.jvm_tool_mixin import JvmToolMixin
+from pants.base.workunit import WorkUnit
+from pants.option.options import Options
+from pants.subsystem.subsystem import Subsystem
+
+
+class JarTool(JvmToolMixin, Subsystem):
+  @classmethod
+  def scope_qualifier(cls):
+    return 'jar-tool'
+
+  @classmethod
+  def register_options(cls, register):
+    super(JarTool, cls).register_options(register)
+    # TODO: All jvm tools will need this option, so might as well have register_jvm_tool add it?
+    register('--jvm-options', advanced=True, type=Options.list, default=['-Xmx64M'],
+             help='Run the jar tool with these JVM options.')
+    cls.register_jvm_tool(register, 'jar-tool')
+
+  def run(self, context, runjava, args):
+    return runjava(self.tool_classpath_from_products(context.products, 'jar-tool',
+                                                     scope=self.options_scope),
+                   'com.twitter.common.jar.tool.Main',
+                   jvm_options=self.get_options().jvm_options,
+                   args=args,
+                   workunit_name='jar-tool',
+                   workunit_labels=[WorkUnit.TOOL, WorkUnit.JVM, WorkUnit.NAILGUN])

src/python/pants/backend/jvm/subsystems/jvm_tool_mixin.py

@@ -0,0 +1,80 @@
+# 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 pants.base.exceptions import TaskError
+from pants.option.options import Options
+
+
+class JvmToolMixin(object):
+  """A mixin for registering and accessing JVM-based tools."""
+
+  _tool_keys = []  # List of (scope, key, main, custom_rule) tuples.
+
+  @classmethod
+  def register_jvm_tool(cls, register, key, default=None, main=None, custom_rules=None):
+    """Registers a jvm tool under `key` for lazy classpath resolution.
+
+    Classpaths can be retrieved in `execute` scope via `tool_classpath`.
+
+    NB: If the tool's `main` class name is supplied the tool classpath will be shaded.
+
+    :param register: A function that can register options with the option system.
+    :param unicode key: The key the tool configuration should be registered under.
+    :param list default: The default tool classpath target address specs to use.
+    :param unicode main: The fully qualified class name of the tool's main class if shading of the
+                         tool classpath is desired.
+    :param list custom_rules: An optional list of `Shader.Rule`s to apply before the automatically
+                              generated binary jar shading rules.  This is useful for excluding
+                              classes shared between the tool and the code it runs over.  The
+                              canonical example is the `org.junit.Test` annotation read by junit
+                              runner tools from user code. In this sort of case the shared code must
+                              have a uniform name between the tool and the user code and so the
+                              shared code must be excluded from shading.
+    """
+    register('--{0}'.format(key),
+             advanced=True,
+             type=Options.list,
+             default=default or ['//:{0}'.format(key)],
+             help='Target specs for bootstrapping the {0} tool.'.format(key))
+
+    # TODO(John Sirois): Move towards requiring tool specs point to jvm_binary targets.
+    # These already have a main and are a natural place to house any custom shading rules.  That
+    # would eliminate the need to pass main and custom_rules here.
+    # It is awkward that jars can no longer be inlined as dependencies - this will require 2 targets
+    # for every tool - the jvm_binary, and a jar_library for its dependencies to point to.  It may
+    # be worth creating a JarLibrary subclass - say JarBinary, or else mixing in a Binary interface
+    # to JarLibrary to endow it with main and shade_rules attributes to allow for single-target
+    # definition of resolvable jvm binaries.
+    JvmToolMixin._tool_keys.append((register.scope, key, main, custom_rules))
+
+  @staticmethod
+  def get_registered_tools():
+    return JvmToolMixin._tool_keys
+
+  @staticmethod
+  def reset_registered_tools():
+    """Needed only for test isolation."""
+    JvmToolMixin._tool_keys = []
+
+  def tool_classpath_from_products(self, products, key, scope):
+    """Get a classpath for the tool previously registered under key in the given scope.
+
+    Returns a list of paths.
+    """
+    return self.lazy_tool_classpath_from_products(products, key, scope)()
+
+  def lazy_tool_classpath_from_products(self, products, key, scope):
+    """Get a lazy classpath for the tool previously registered under the key in the given scope.
+
+    Returns a no-arg callable. Invoking it returns a list of paths.
+    """
+    callback_product_map = products.get_data('jvm_build_tools_classpath_callbacks') or {}
+    callback = callback_product_map.get(scope, {}).get(key)
+    if not callback:
+      raise TaskError('No bootstrap callback registered for {key} in {scope}'.format(
+        key=key, scope=scope))
+    return callback

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

@@ -68,12 +68,12 @@ python_library(
   dependencies = [
     ':ivy_task_mixin',
     ':jar_task',
-    ':jvm_tool_task_mixin',
     'src/python/pants/base:exceptions',
     'src/python/pants/base:workunit',
     'src/python/pants/java:executor',
     'src/python/pants/java:util',
     'src/python/pants/java/jar:shader',
+    'src/python/pants/subsystem',
     'src/python/pants/util:dirutil',
   ],
 )
@@ -222,9 +222,9 @@ python_library(
     ':nailgun_task',
     '3rdparty/python:six',
     '3rdparty/python/twitter/commons:twitter.common.collections',
+    'src/python/pants/backend/jvm/subsystems:jar_tool',
     'src/python/pants/backend/jvm/targets:java',
     'src/python/pants/base:exceptions',
-    'src/python/pants/base:workunit',
     'src/python/pants/java/jar:manifest',
     'src/python/pants/util:contextutil',
     'src/python/pants/util:meta',
@@ -300,8 +300,7 @@ python_library(
   name = 'jvm_tool_task_mixin',
   sources = ['jvm_tool_task_mixin.py'],
   dependencies = [
-    'src/python/pants/base:exceptions',
-    'src/python/pants/option',
+    'src/python/pants/backend/jvm/subsystems:jvm_tool_mixin',
   ],
 )
 

src/python/pants/backend/jvm/tasks/bootstrap_jvm_tools.py

@@ -11,9 +11,9 @@
 import threading
 from collections import defaultdict
 
+from pants.backend.jvm.subsystems.jvm_tool_mixin import JvmToolMixin
 from pants.backend.jvm.tasks.ivy_task_mixin import IvyResolveFingerprintStrategy, IvyTaskMixin
 from pants.backend.jvm.tasks.jar_task import JarTask
-from pants.backend.jvm.tasks.jvm_tool_task_mixin import JvmToolTaskMixin
 from pants.base.address_lookup_error import AddressLookupError
 from pants.base.exceptions import TaskError
 from pants.java import util
@@ -90,7 +90,7 @@ def __init__(self, *args, **kwargs):
 
   def execute(self):
     context = self.context
-    if JvmToolTaskMixin.get_registered_tools():
+    if JvmToolMixin.get_registered_tools():
       # Map of scope -> (map of key -> callback).
       callback_product_map = (context.products.get_data('jvm_build_tools_classpath_callbacks') or
                               defaultdict(dict))
@@ -101,7 +101,7 @@ def execute(self):
       # the bootstrap tools.  It would be awkward and possibly incorrect to call
       # self.invalidated twice on a Task that does meaningful invalidation on its
       # targets. -pl
-      for scope, key, main, custom_rules in JvmToolTaskMixin.get_registered_tools():
+      for scope, key, main, custom_rules in JvmToolMixin.get_registered_tools():
         option = key.replace('-', '_')
         deplist = self.context.options.for_scope(scope)[option]
         callback_product_map[scope][key] = self.cached_bootstrap_classpath_callback(

src/python/pants/backend/jvm/tasks/jar_task.py

@@ -7,17 +7,17 @@
 
 import os
 import tempfile
-from abc import abstractmethod, abstractproperty
+from abc import abstractmethod
 from contextlib import contextmanager
 
 from six import binary_type, string_types
 from twitter.common.collections import maybe_list
 
+from pants.backend.jvm.subsystems.jar_tool import JarTool
 from pants.backend.jvm.targets.java_agent import JavaAgent
 from pants.backend.jvm.targets.jvm_binary import Duplicate, JarRules, Skip
 from pants.backend.jvm.tasks.nailgun_task import NailgunTask
 from pants.base.exceptions import TaskError
-from pants.base.workunit import WorkUnit
 from pants.binary_util import safe_args
 from pants.java.jar.manifest import Manifest
 from pants.util.contextutil import temporary_dir
@@ -203,8 +203,9 @@ class JarTask(NailgunTask):
   All subclasses will share the same underlying nailgunned jar tool and thus benefit from fast
   invocations.
   """
-
-  _CONFIG_SECTION = 'jar-tool'
+  @classmethod
+  def global_subsystems(cls):
+    return super(JarTask, cls).global_subsystems() + (JarTool, )
 
   @staticmethod
   def _flag(bool_value):
@@ -224,21 +225,12 @@ def _action_name(cls, action):
       raise ValueError('Unrecognized duplicate action: {}'.format(action))
     return name
 
-  @classmethod
-  def register_options(cls, register):
-    super(JarTask, cls).register_options(register)
-    cls.register_jvm_tool(register, 'jar-tool')
-
   def __init__(self, *args, **kwargs):
     super(JarTask, self).__init__(*args, **kwargs)
     self.set_distribution(jdk=True)
     # TODO(John Sirois): Consider poking a hole for custom jar-tool jvm args - namely for Xmx
     # control.
 
-  @property
-  def config_section(self):
-    return self._CONFIG_SECTION
-
   @contextmanager
   def open_jar(self, path, overwrite=False, compressed=True, jar_rules=None):
     """Yields a Jar that will be written when the context exits.
@@ -283,16 +275,7 @@ def open_jar(self, path, overwrite=False, compressed=True, jar_rules=None):
 
         args.append(path)
 
-        # TODO(Eric Ayers): This needs to be migrated with some thought behind it.  Consider
-        # that The jar-tool nailgun instance is shared between tasks and doesn't necessarily
-        # need the same JVM args as its parent.
-        jvm_options = self.context.config.getlist('jar-tool', 'jvm_args', default=['-Xmx64M'])
-        self.runjava(self.tool_classpath('jar-tool'),
-                     'com.twitter.common.jar.tool.Main',
-                     jvm_options=jvm_options,
-                     args=args,
-                     workunit_name='jar-tool',
-                     workunit_labels=[WorkUnit.TOOL, WorkUnit.JVM, WorkUnit.NAILGUN])
+        JarTool.global_instance().run(context=self.context, runjava=self.runjava, args=args)
 
   class JarBuilder(AbstractClass):
     """A utility to aid in adding the classes and resources associated with targets to a jar."""
@@ -334,7 +317,6 @@ def __init__(self, context, jar):
     def add_target(self, target, recursive=False):
       """Adds the classes and resources for a target to an open jar.
 
-      :param jar: An open jar to add to.
       :param target: The target to add generated classes and resources for.
       :param bool recursive: `True` to add classes and resources for the target's transitive
         internal dependency closure.

src/python/pants/backend/jvm/tasks/jvm_tool_task_mixin.py

@@ -5,69 +5,23 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-from pants.base.exceptions import TaskError
-from pants.option.options import Options
+from pants.backend.jvm.subsystems.jvm_tool_mixin import JvmToolMixin
 
 
-class JvmToolTaskMixin(object):
-
-  _tool_keys = []  # List of (scope, key) pairs.
-
-  @classmethod
-  def register_jvm_tool(cls, register, key, default=None, main=None, custom_rules=None):
-    """Registers a jvm tool under `key` for lazy classpath resolution.
-
-    Classpaths can be retrieved in `execute` scope via `tool_classpath`.
-
-    NB: If the tool's `main` class name is supplied the tool classpath will be shaded.
-
-    :param register: A function that can register options with the option system.
-    :param unicode key: The key the tool configuration should be registered under.
-    :param list default: The default tool classpath target address specs to use.
-    :param unicode main: The fully qualified class name of the tool's main class if shading of the
-                         tool classpath is desired.
-    :param list custom_rules: An optional list of `Shader.Rule`s to apply before the automatically
-                              generated binary jar shading rules.  This is useful for excluding
-                              classes shared between the tool and the code it runs over.  The
-                              canonical example is the `org.junit.Test` annotation read by junit
-                              runner tools from user code. In this sort of case the shared code must
-                              have a uniform name between the tool and the user code and so the
-                              shared code must be excluded from shading.
-    """
-    register('--{0}'.format(key),
-             type=Options.list,
-             default=default or ['//:{0}'.format(key)],
-             help='Target specs for bootstrapping the {0} tool.'.format(key))
-
-    # TODO(John Sirois): Move towards requiring tool specs point to jvm_binary targets.
-    # These already have a main and are a natural place to house any custom shading rules.  That
-    # would eliminate the need to pass main and custom_rules here.
-    # It is awkward that jars can no longer be inlined as dependencies - this will require 2 targets
-    # for every tool - the jvm_binary, and a jar_library for its dependencies to point to.  It may
-    # be worth creating a JarLibrary subclass - say JarBinary, or else mixing in a Binary interface
-    # to JarLibrary to endow it with main and shade_rules attributes to allow for single-target
-    # definition of resolvable jvm binaries.
-    JvmToolTaskMixin._tool_keys.append((cls.options_scope, key, main, custom_rules))
-
-  @staticmethod
-  def get_registered_tools():
-    return JvmToolTaskMixin._tool_keys
-
-  @staticmethod
-  def reset_registered_tools():
-    """Needed only for test isolation."""
-    JvmToolTaskMixin._tool_keys = []
+class JvmToolTaskMixin(JvmToolMixin):
+  """A JvmToolMixin specialized for mixing in to Tasks."""
 
   def tool_classpath(self, key, scope=None):
     """Get a classpath for the tool previously registered under key in the given scope.
 
     Returns a list of paths.
     """
-    scope = scope or self.options_scope
-    callback_product_map = (self.context.products.get_data('jvm_build_tools_classpath_callbacks')
-                            or {})
-    callback = callback_product_map.get(scope, {}).get(key)
-    if not callback:
-      raise TaskError('No bootstrap callback registered for {key} in {scope}'.format(
-          key=key, scope=scope))
-    return callback()
+    return self.lazy_tool_classpath(key, scope=scope)()
+
+  def lazy_tool_classpath(self, key, scope=None):
+    """Get a lazy classpath for the tool previously registered under the key in the given scope.
+
+    Returns a no-arg callable. Invoking it returns a list of paths.
+    """
+    return self.lazy_tool_classpath_from_products(self.context.products, key,
+                                                  scope=scope or self.options_scope)