drastically simplify combined jvm tools and introduce get_zinc_compil…

…er_classpath()

also distinguish the identify of the JvmToolMixin Zinc *subsystem* and the JvmToolTaskMixin
RscCompile *task*

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

@@ -14,6 +14,7 @@
 from pants.java.distribution.distribution import DistributionLocator
 from pants.option.custom_types import target_option
 from pants.util.memo import memoized_property
+from pants.util.meta import classproperty
 from pants.util.objects import datatype
 
 
@@ -152,11 +153,11 @@ def formulate_help():
   # TODO: Deprecate .register_jvm_tool()?
   class JvmToolDeclaration(datatype([
     ('tool_name', text_type),
-    'main',
+    'main',                     # Optional
     ('classpath', tuple),
     ('custom_rules', tuple),
 ])):
-    """A specification for a JVM tool."""
+    """A mostly-typed specification for a JVM tool. Can be passed around across python modules."""
 
     def __new__(cls, tool_name, main=None, classpath=None, custom_rules=None):
       return super(JvmToolMixin.JvmToolDeclaration, cls).__new__(
@@ -183,42 +184,36 @@ def register_jvm_tool_decl(cls, register, decl, **kwargs):
 
   class CombinedJvmToolsError(JvmToolDeclError): pass
 
-  _combined_tools = []
-
-  @classmethod
-  def register_combined_jvm_tools(cls, register, tool_decls):
-    """Register `JvmToolDeclaration`s as JVM tools, which can be invoked as a single jar.
+  @classproperty
+  def combined_jvm_tool_names(cls):
+    """Return a list of tool names which can be invoked as a single jar.
 
     This allows tools to be invoked one at a time, but with the combined classpath of all of them
     using ensure_combined_jvm_tool_classpath(). This allows creating nailgun instances for the
     tools which have the same fingerprint, and allows a task to invoke multiple different JVM tools
-    from the same nailgun instances.
+    from the same nailgun instances. See #7089.
     """
-    for decl in tool_decls:
-      cls.register_jvm_tool_decl(register, decl)
-      cls._combined_tools.append(decl)
+    raise NotImplementedError(
+      'combined_jvm_tool_names must be implemented to use multiple JVM tools '
+      'with the nailgun execution strategy!')
 
   @memoized_property
   def _combined_jvm_tool_classpath(self):
     cp = []
-    for decl in self._combined_tools:
-      cp.extend(self.tool_classpath(decl.tool_name))
+    for tool_name in self.combined_jvm_tool_names:
+      cp.extend(self.tool_classpath(tool_name))
     return cp
 
-  @memoized_property
-  def _registered_combined_tool_keys(self):
-    return frozenset(decl.tool_name for decl in self._combined_tools)
-
   def ensure_combined_jvm_tool_classpath(self, tool_name):
-    """Get a single classpath for all tools registered with `register_combined_jvm_tools()`.
+    """Get a single classpath for all tools returned by `combined_jvm_tool_names()`.
 
     Also check to ensure the tool was registered for consumption as a combined JVM tool.
     """
-    if tool_name not in self._registered_combined_tool_keys:
+    if tool_name not in self.combined_jvm_tool_names:
       raise self.CombinedJvmToolsError(
-        "tool with name '{}' must be registered with register_combined_jvm_tools() "
+        "tool with name '{}' must be added to {}.combined_jvm_tool_names "
         "(known keys are: {})"
-        .format(tool_name, type(self).__name__, self._registered_combined_tool_keys))
+        .format(tool_name, type(self).__name__, self.combined_jvm_tool_names))
     return self._combined_jvm_tool_classpath
 
   @classmethod

src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py

@@ -16,7 +16,7 @@
 from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform
 from pants.backend.jvm.subsystems.jvm_tool_mixin import JvmToolMixin
 from pants.backend.jvm.subsystems.shader import Shader
-from pants.backend.jvm.subsystems.zinc import ZINC_COMPILER_DECL
+from pants.backend.jvm.subsystems.zinc import ZINC_COMPILER_DECL, Zinc
 from pants.backend.jvm.targets.jar_library import JarLibrary
 from pants.backend.jvm.targets.java_library import JavaLibrary
 from pants.backend.jvm.targets.jvm_target import JvmTarget
@@ -37,6 +37,7 @@
 from pants.util.contextutil import Timer
 from pants.util.dirutil import (fast_relpath, fast_relpath_optional, maybe_read_file,
                                 safe_file_dump, safe_mkdir)
+from pants.util.meta import classproperty
 
 
 #
@@ -182,9 +183,24 @@ def register_options(cls, register):
 
     # Register all of these as "combined" JVM tools so that we can invoke their combined classpath
     # in a single nailgun instance. We still invoke their classpaths separately when not using
-    # nailgun, however.
-    cls.register_combined_jvm_tools(
-      register, [rsc_decl, metacp_decl, metai_decl, ZINC_COMPILER_DECL])
+    # nailgun, however. Note that we have to register zinc again as in ZincCompile it is being
+    # accessed through the Zinc subsystem, but we want to invoke all of these as a combined tool
+    # through *this task's* implementation of JvmToolMixin.
+    for decl in [rsc_decl, metacp_decl, metai_decl, ZINC_COMPILER_DECL]:
+      cls.register_jvm_tool_decl(register, decl)
+
+  @classproperty
+  def combined_jvm_tool_names(cls):
+    """Register all of the component tools of the rsc compile task as a "combined" jvm tool.
+
+    This allows us to invoke their combined classpath in a single nailgun instance. We still invoke
+    their classpaths separately when not using nailgun, however.
+    """
+    return ['rsc', 'metai', 'metacp', 'zinc']
+
+  # Overrides the normal zinc compiler classpath, which only contains zinc.
+  def get_zinc_compiler_classpath(self):
+    return self.ensure_combined_jvm_tool_classpath(Zinc.ZINC_COMPILER_TOOL_NAME)
 
   def register_extra_products_from_contexts(self, targets, compile_contexts):
     super(RscCompile, self).register_extra_products_from_contexts(targets, compile_contexts)

src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py

@@ -443,12 +443,7 @@ def relative_to_exec_root(path):
       # TODO: This should probably return a ClasspathEntry rather than a Digest
       return res.output_directory_digest
     else:
-      # This will just be the zinc compiler tool classpath normally, but tasks which invoke zinc
-      # along with other JVM tools with nailgun (such as RscCompile) require zinc to be invoked with
-      # this method to ensure a single classpath is used for all the tools they need to invoke so
-      # that the nailgun instance (which is keyed by classpath and JVM options) isn't invalidated.
-      zinc_combined_cp = self.ensure_combined_jvm_tool_classpath(Zinc.ZINC_COMPILER_TOOL_NAME)
-      if self.runjava(classpath=zinc_combined_cp,
+      if self.runjava(classpath=self.get_zinc_compiler_classpath(),
                       main=Zinc.ZINC_COMPILE_MAIN,
                       jvm_options=jvm_options,
                       args=zinc_args,
@@ -457,6 +452,16 @@ def relative_to_exec_root(path):
                       dist=self._zinc.dist):
         raise TaskError('Zinc compile failed.')
 
+  def get_zinc_compiler_classpath(self):
+    """Get the classpath for the zinc compiler JVM tool.
+
+    This will just be the zinc compiler tool classpath normally, but tasks which invoke zinc along
+    with other JVM tools with nailgun (such as RscCompile) require zinc to be invoked with this
+    method to ensure a single classpath is used for all the tools they need to invoke so that the
+    nailgun instance (which is keyed by classpath and JVM options) isn't invalidated.
+    """
+    return [self._zinc.zinc]
+
   def _verify_zinc_classpath(self, classpath, allow_dist=True):
     def is_outside(path, putative_parent):
       return os.path.relpath(path, putative_parent).startswith(os.pardir)