From 8546623ebdba954facbe68c78816092f36ef3d44 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Fri, 10 Jan 2020 11:22:02 -0800 Subject: [PATCH] Only compile non-module exports for `export-dep-as-jar` goal (#8914) ### Problem When running the `export-dep-as-jar` Goal we compile all targets in the build context. This is unnecessary and can be problematic if any of the target roots have compile errors in them, because the export will fail. It is unnecessary because the target roots are being exported as source modules #8812 ### Solution We exclude target_roots, and their dependees from the compile context. ### Result Only targets in the build context that are not being exported as sources are compiled. --- .../jvm/tasks/jvm_compile/jvm_compile.py | 22 +++++- .../tasks/jvm_compile/zinc/zinc_compile.py | 2 +- .../project_info/tasks/export_dep_as_jar.py | 6 +- src/python/pants/testutil/task_test_base.py | 13 ++++ .../backend/jvm/tasks/jvm_compile/BUILD | 2 + .../jvm/tasks/jvm_compile/test_jvm_compile.py | 67 ++++++++++++++++++- .../tasks/test_export_dep_as_jar.py | 18 +---- 7 files changed, 104 insertions(+), 26 deletions(-) diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py b/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py index 7a9487c094d..43a3a6b6824 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py @@ -428,11 +428,21 @@ def execute(self): relevant_targets = list(self.context.targets(predicate=self.select)) + # If we are only exporting jars then we can omit some targets from the runtime_classpath. + if ( + self.context.products.is_required_data("export_dep_as_jar_classpath") and + not self.context.products.is_required_data("runtime_classpath") + ): + # Filter modulized targets from invalid targets list. + target_root_addresses = [t.address for t in set(self.context.target_roots)] + dependees_of_target_roots = self.context.build_graph.transitive_dependees_of_addresses(target_root_addresses) + relevant_targets = list(set(relevant_targets) - dependees_of_target_roots) + if not relevant_targets: return # Clone the compile_classpath to the runtime_classpath. - classpath_product = self.create_runtime_classpath() + classpath_product = self.create_classpath_product() fingerprint_strategy = DependencyContext.global_instance().create_fingerprint_strategy( classpath_product) @@ -462,14 +472,20 @@ def execute(self): classpath_product.remove_for_target(cc.target, [(conf, cc.classes_dir)]) classpath_product.add_for_target(cc.target, [(conf, cc.jar_file)]) + # The runtime classpath will always be a superset of the export_dep_as_jar_classpath + # so however we compute runtime_classpath it will contain all the nescessary jars for + # the export_dep_as_jar_classpath.export_dep_as_jar_classpath. + if self.context.products.is_required_data('export_dep_as_jar_classpath'): + self.context.products.get_data('export_dep_as_jar_classpath', classpath_product.copy) + def _classpath_for_context(self, context): if self.get_options().use_classpath_jars: return context.jar_file return context.classes_dir - def create_runtime_classpath(self): + def create_classpath_product(self): compile_classpath = self.context.products.get_data('compile_classpath') - classpath_product = self.context.products.get_data('runtime_classpath') + classpath_product = self.context.products.get_data("runtime_classpath") if not classpath_product: classpath_product = self.context.products.get_data('runtime_classpath', compile_classpath.copy) else: diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py index 7d1f00f3cde..55ae1ed30c3 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py @@ -717,7 +717,7 @@ class ZincCompile(BaseZincCompile): @classmethod def product_types(cls): - return ['runtime_classpath', 'zinc_analysis', 'zinc_args'] + return ['runtime_classpath', 'export_dep_as_jar_classpath', 'zinc_analysis', 'zinc_args'] def select(self, target): # Require that targets are marked for JVM compilation, to differentiate from diff --git a/src/python/pants/backend/project_info/tasks/export_dep_as_jar.py b/src/python/pants/backend/project_info/tasks/export_dep_as_jar.py index 0e0716b19b4..1060f2e7efa 100644 --- a/src/python/pants/backend/project_info/tasks/export_dep_as_jar.py +++ b/src/python/pants/backend/project_info/tasks/export_dep_as_jar.py @@ -47,7 +47,7 @@ def register_options(cls, register): @classmethod def prepare(cls, options, round_manager): super().prepare(options, round_manager) - round_manager.require_data('runtime_classpath') + round_manager.require_data('export_dep_as_jar_classpath') @property def _output_folder(self): @@ -354,9 +354,9 @@ def generate_targets_map(self, targets, runtime_classpath): return graph_info def console_output(self, targets): - runtime_classpath = self.context.products.get_data('runtime_classpath') + runtime_classpath = self.context.products.get_data('export_dep_as_jar_classpath') if runtime_classpath is None: - raise TaskError("There was an error compiling the targets - There is no runtime classpath") + raise TaskError("There was an error compiling the targets - There is no export_dep_as_jar classpath") graph_info = self.generate_targets_map(targets, runtime_classpath=runtime_classpath) if self.get_options().formatted: return json.dumps(graph_info, indent=4, separators=(',', ': ')).splitlines() diff --git a/src/python/pants/testutil/task_test_base.py b/src/python/pants/testutil/task_test_base.py index d9267acc92c..af8f3a113af 100644 --- a/src/python/pants/testutil/task_test_base.py +++ b/src/python/pants/testutil/task_test_base.py @@ -165,6 +165,19 @@ def cache_check(self, expected_num_artifacts=None): else: self.assertEqual(num_artifacts, expected_num_artifacts) + def make_linear_graph(self, names, **additional_target_args): + # A build graph where a -(depends on)-> b -> ... -> e + graph = {} + last_target = None + for name in reversed(names): + last_target = self.make_target( + f'project_info:{name}', + dependencies=[] if last_target is None else [last_target], + **additional_target_args, + ) + graph[name] = last_target + return graph + class ConsoleTaskTestBase(TaskTestBase): """A base class useful for testing ConsoleTasks. diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/BUILD b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/BUILD index 3b1d1c81710..bd70df5422a 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/BUILD +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/BUILD @@ -20,7 +20,9 @@ python_tests( dependencies = [ 'src/python/pants/backend/jvm/tasks/jvm_compile', 'src/python/pants/backend/jvm/tasks:classpath_products', + 'src/python/pants/backend/jvm/subsystems:jvm_platform', 'src/python/pants/testutil/jvm:nailgun_task_test_base', + 'src/python/pants/testutil/subsystem', ], tags = {"partially_type_checked"}, ) diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_jvm_compile.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_jvm_compile.py index 6b4125d01a4..2e6fd1a75cf 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_jvm_compile.py +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_jvm_compile.py @@ -3,6 +3,7 @@ import os +from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform from pants.backend.jvm.subsystems.zinc import Zinc from pants.backend.jvm.targets.java_library import JavaLibrary from pants.backend.jvm.tasks.classpath_products import ClasspathProducts @@ -11,10 +12,17 @@ from pants.backend.jvm.tasks.nailgun_task import NailgunTaskBase from pants.base.build_environment import get_buildroot from pants.testutil.jvm.nailgun_task_test_base import NailgunTaskTestBase +from pants.testutil.subsystem.util import init_subsystems class DummyJvmCompile(JvmCompile): - pass + compiler_name = 'dummy' + + def select(self, *args): + return True + + def do_compile(self, invalidation_check, compile_contexts, classpath_product): + pass class JvmCompileTest(NailgunTaskTestBase): @@ -42,9 +50,62 @@ def test_if_runtime_classpath_exists(self): runtime_classpath.add_for_targets([target], [('default', pre_init_runtime_entry)]) task = self.create_task(context) - resulting_classpath = task.create_runtime_classpath() + resulting_classpath = task.create_classpath_product() self.assertEqual([('default', pre_init_runtime_entry), ('default', compile_entry)], - resulting_classpath.get_for_target(target)) + resulting_classpath.get_for_target(target)) + + def create_and_return_classpath_products(self, targets, required_products): + """Executes our mocked out JvmCompile class, with certain required products + args: + required_products: list of str. The products to declare a dependency on.f + rtype: tuple(ClasspathProducts) + """ + init_subsystems([JvmPlatform]) + context = self.context(target_roots=[targets['a'], targets['c']], options={'jvm-platform': {'compiler': 'dummy'}}) + context.products.get_data('compile_classpath', ClasspathProducts.init_func(self.pants_workdir)) + # This should cause the jvm compile execution to exclude target roots and their + # dependess from the set of relevant targets. + for rp in required_products: + context.products.require_data(rp) + self.execute(context) + return (context.products.get_data('runtime_classpath'), context.products.get_data('export_dep_as_jar_classpath')) + + def test_modulized_targets_not_compiled_for_export_classpath(self): + targets = self.make_linear_graph(['a', 'b', 'c', 'd', 'e'], target_type=JavaLibrary) + runtime_classpath, export_dep_as_jar_classpath = self.create_and_return_classpath_products( + targets, ['export_dep_as_jar_classpath'] + ) + # assert none of the modulized targets have classpaths. + self.assertEqual( + len( + export_dep_as_jar_classpath.get_for_target(targets['a']) + + export_dep_as_jar_classpath.get_for_target(targets['b']) + + export_dep_as_jar_classpath.get_for_target(targets['c']) + ), + 0 + ) + self.assertEqual(len(export_dep_as_jar_classpath.get_for_target(targets['d'])), 1) + self.assertEqual(len(export_dep_as_jar_classpath.get_for_target(targets['e'])), 1) + + def test_modulized_targets_are_compiled_when_runtime_classpath_is_requested(self): + targets = self.make_linear_graph(['a', 'b', 'c', 'd', 'e'], target_type=JavaLibrary) + # This should cause the jvm compile execution to exclude target roots and their + # dependess from the set of relevant targets. + runtime_classpath, export_dep_as_jar_classpath = self.create_and_return_classpath_products( + targets, ['export_dep_as_jar_classpath', 'runtime_classpath'] + ) + self.assertEqual(runtime_classpath, export_dep_as_jar_classpath) + # assert all of the modulized targets have classpaths. + self.assertEqual(len(export_dep_as_jar_classpath.get_for_target(targets['a'])), 1) + self.assertEqual(len(export_dep_as_jar_classpath.get_for_target(targets['b'])), 1) + self.assertEqual(len(export_dep_as_jar_classpath.get_for_target(targets['c'])), 1) + + def test_export_dep_as_jar_classpath_not_created(self): + targets = self.make_linear_graph(['a', 'b', 'c', 'd', 'e'], target_type=JavaLibrary) + runtime_classpath, export_dep_as_jar_classpath = self.create_and_return_classpath_products( + targets, ['runtime_classpath'] + ) + self.assertIsNone(export_dep_as_jar_classpath) class BaseZincCompileJDKTest(NailgunTaskTestBase): diff --git a/tests/python/pants_test/backend/project_info/tasks/test_export_dep_as_jar.py b/tests/python/pants_test/backend/project_info/tasks/test_export_dep_as_jar.py index 41d56fd841d..7f6bad2be0b 100644 --- a/tests/python/pants_test/backend/project_info/tasks/test_export_dep_as_jar.py +++ b/tests/python/pants_test/backend/project_info/tasks/test_export_dep_as_jar.py @@ -247,21 +247,7 @@ def setUp(self): sources=[], ) - def _make_linear_graph(names, **additional_target_args): - # A build graph where a -(depends on)-> b -> ... -> e - graph = {} - last_target = None - for name in reversed(names): - last_target = self.make_target( - f'project_info:{name}', - target_type=ScalaLibrary, - dependencies=[] if last_target is None else [last_target], - **additional_target_args, - ) - graph[name] = last_target - return graph - - self.linear_build_graph = _make_linear_graph(['a', 'b', 'c', 'd', 'e']) + self.linear_build_graph = self.make_linear_graph(['a', 'b', 'c', 'd', 'e'], target_type=ScalaLibrary) def create_runtime_classpath_for_targets(self, target): def path_to_zjar_with_workdir(address: Address): @@ -295,7 +281,7 @@ def execute_export(self, *specs, **options_overrides): for_task_types=[BootstrapJvmTools]) runtime_classpath = self.create_runtime_classpath_for_targets(self.scala_with_source_dep) - context.products.safe_create_data('runtime_classpath', + context.products.safe_create_data('export_dep_as_jar_classpath', init_func=lambda: runtime_classpath) bootstrap_task = BootstrapJvmTools(context, self.pants_workdir)