diff --git a/src/python/pants/backend/jvm/subsystems/jvm_tool_mixin.py b/src/python/pants/backend/jvm/subsystems/jvm_tool_mixin.py index a3fe97fba89..4a307351007 100644 --- a/src/python/pants/backend/jvm/subsystems/jvm_tool_mixin.py +++ b/src/python/pants/backend/jvm/subsystems/jvm_tool_mixin.py @@ -13,8 +13,7 @@ from pants.base.exceptions import TaskError 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.memo import memoized_method from pants.util.objects import Exactly, datatype @@ -158,7 +157,11 @@ class JvmToolDeclaration(datatype([ ('classpath', tuple), ('custom_rules', tuple), ])): - """A mostly-typed specification for a JVM tool. Can be passed around across python modules.""" + """A mostly-typed specification for a JVM tool. + + This allows a specification for a JVM tool to be passed around verbatim across JvmToolMixin + subclasses. + """ def __new__(cls, tool_name, main=None, classpath=None, custom_rules=None): return super(JvmToolMixin.JvmToolDeclaration, cls).__new__( @@ -185,36 +188,25 @@ def register_jvm_tool_decl(cls, register, decl, **kwargs): class CombinedJvmToolsError(JvmToolDeclError): pass - @classproperty - def combined_jvm_tool_names(cls): - """Return a list of tool names which can be invoked as a single jar. + @memoized_method + def ensure_combined_jvm_tool_classpath(self, tool_name, combined_jvm_tool_names): + """Get a single classpath for all tools returned by `combined_jvm_tool_names`. - 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. See #7089. - """ - raise NotImplementedError('combined_jvm_tool_names is not implemented, see ' - 'https://github.com/pantsbuild/pants/pull/7092') + Also check to ensure `tool_name` is a member of `combined_jvm_tool_names`. - @memoized_property - def _combined_jvm_tool_classpath(self): - cp = [] - for tool_name in self.combined_jvm_tool_names: - cp.extend(self.tool_classpath(tool_name)) - return cp - - def ensure_combined_jvm_tool_classpath(self, tool_name): - """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. + This allows tools to be invoked one at a time, but with the combined classpath of all of + them. 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. See #7089. """ - if tool_name not in self.combined_jvm_tool_names: + if tool_name not in combined_jvm_tool_names: raise self.CombinedJvmToolsError( - "tool with name '{}' must be added to {}.combined_jvm_tool_names " - "(known keys are: {})" - .format(tool_name, type(self).__name__, self.combined_jvm_tool_names)) - return self._combined_jvm_tool_classpath + "tool with name '{}' must be added to `combined_jvm_tool_names` " + "(which was: {})" + .format(tool_name, combined_jvm_tool_names)) + cp = [] + for component_tool_name in combined_jvm_tool_names: + cp.extend(self.tool_classpath(component_tool_name)) + return cp @classmethod def prepare_tools(cls, round_manager): diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py b/src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py index b4092d6992d..450fda7edce 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py +++ b/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, Zinc +from pants.backend.jvm.subsystems.zinc import ZINC_COMPILER_DECL 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,7 +37,6 @@ 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 # @@ -191,18 +190,24 @@ def register_options(cls, register): 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): + def get_nailgunnable_combined_classpath(self, tool_name): """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'] + return self.ensure_combined_jvm_tool_classpath( + tool_name, + # TODO: allow @memoized_method to convert lists into tuples so they can be hashed! + ('rsc', 'metai', 'metacp', ZINC_COMPILER_DECL.tool_name)) # 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) + return self.do_for_execution_strategy_variant({ + self.HERMETIC: lambda: super(RscCompile, self).get_zinc_compiler_classpath(), + self.SUBPROCESS: lambda: super(RscCompile, self).get_zinc_compiler_classpath(), + self.NAILGUN: lambda: self.get_nailgunnable_combined_classpath(ZINC_COMPILER_DECL.tool_name), + }) def register_extra_products_from_contexts(self, targets, compile_contexts): super(RscCompile, self).register_extra_products_from_contexts(targets, compile_contexts) @@ -857,18 +862,17 @@ def _runtool_nonhermetic(self, parent_workunit, classpath, main, tool_name, args def _runtool(self, main, tool_name, args, distribution, tgt=None, input_files=tuple(), input_digest=None, output_dir=None): - def workunit_factory(*args, **kwargs): - return self.context.new_workunit(tool_name, *args, **kwargs) - return self.do_for_execution_strategy_variant(workunit_factory, { - self.HERMETIC: lambda _wu: self._runtool_hermetic( - main, tool_name, args, distribution, - tgt=tgt, input_files=input_files, input_digest=input_digest, output_dir=output_dir), - self.SUBPROCESS: lambda wu: self._runtool_nonhermetic( - wu, self.tool_classpath(tool_name), main, tool_name, args, distribution), - self.NAILGUN: lambda wu: self._runtool_nonhermetic( - wu, self.ensure_combined_jvm_tool_classpath(tool_name), - main, tool_name, args, distribution), - }) + with self.context.new_workunit(tool_name) as wu: + return self.do_for_execution_strategy_variant({ + self.HERMETIC: lambda: self._runtool_hermetic( + main, tool_name, args, distribution, + tgt=tgt, input_files=input_files, input_digest=input_digest, output_dir=output_dir), + self.SUBPROCESS: lambda: self._runtool_nonhermetic( + wu, self.tool_classpath(tool_name), main, tool_name, args, distribution), + self.NAILGUN: lambda: self._runtool_nonhermetic( + wu, self.get_nailgunnable_combined_classpath(tool_name), + main, tool_name, args, distribution), + }) def _run_metai_tool(self, distribution, diff --git a/src/python/pants/backend/jvm/tasks/nailgun_task.py b/src/python/pants/backend/jvm/tasks/nailgun_task.py index 52ce1ef7afa..30c2e51d882 100644 --- a/src/python/pants/backend/jvm/tasks/nailgun_task.py +++ b/src/python/pants/backend/jvm/tasks/nailgun_task.py @@ -28,19 +28,19 @@ class InvalidExecutionStrategyMapping(Exception): pass _all_execution_strategies = frozenset([NAILGUN, SUBPROCESS, HERMETIC]) - def do_for_execution_strategy_variant(self, workunit_factory, mapping): + def do_for_execution_strategy_variant(self, mapping): """Invoke the method in `mapping` with the key corresponding to the execution strategy. - `mapping` is a dict mapping execution strategy -> method accepting a workunit argument. + `mapping` is a dict mapping execution strategy -> zero-argument lambda. """ variants = frozenset(mapping.keys()) if variants != self._all_execution_strategies: raise self.InvalidExecutionStrategyMapping( - 'Must specify a mapping with exactly the keys {}'.format(self._all_execution_strategies)) + 'Must specify a mapping with exactly the keys {} (was: {})' + .format(self._all_execution_strategies, variants)) method_for_variant = mapping[self.execution_strategy] - with workunit_factory() as workunit: - # The methods need not return a value, but we pass it along if they do. - return method_for_variant(workunit) + # The methods need not return a value, but we pass it along if they do. + return method_for_variant() @classmethod def register_options(cls, register):