Skip to content

Commit

Permalink
Only compile non-module exports for export-dep-as-jar goal (#8914)
Browse files Browse the repository at this point in the history
### 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.
  • Loading branch information
Henry Fuller authored and wisechengyi committed Jan 10, 2020
1 parent e3bd8ef commit 8546623
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 26 deletions.
22 changes: 19 additions & 3 deletions src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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()
Expand Down
13 changes: 13 additions & 0 deletions src/python/pants/testutil/task_test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions tests/python/pants_test/backend/jvm/tasks/jvm_compile/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 8546623

Please sign in to comment.