From 7c7b9ce6e940a6dfdb7c74145e0e8c61be205fd1 Mon Sep 17 00:00:00 2001 From: Yujie Chen Date: Tue, 28 Mar 2017 13:42:51 -0400 Subject: [PATCH 1/6] use jar_library transitive deps --- .../pants/backend/jvm/tasks/jvm_compile/compile_context.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py index 132bb0b0da6..e8819e89a2a 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py @@ -8,6 +8,7 @@ import zipfile from contextlib import contextmanager +from pants.backend.jvm.targets.jar_library import JarLibrary from pants.build_graph.aliased_target import AliasTarget from pants.build_graph.target import Target from pants.util.contextutil import open_zip @@ -15,11 +16,12 @@ def _resolve_strict_dependencies(target): for declared in target.dependencies: - if type(declared) in (AliasTarget, Target): + if type(declared) in (AliasTarget, JarLibrary, Target): # Is an alias. Recurse to expand. for r in _resolve_strict_dependencies(declared): yield r - else: + + if type(declared) not in (AliasTarget, Target): yield declared for export in _resolve_exports(declared): From 87cd814cefb44d55c6913e77bd238fbe69244e67 Mon Sep 17 00:00:00 2001 From: Yujie Chen Date: Tue, 28 Mar 2017 16:22:29 -0400 Subject: [PATCH 2/6] add comments --- src/python/pants/backend/jvm/tasks/jvm_compile/BUILD | 1 + .../pants/backend/jvm/tasks/jvm_compile/compile_context.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD b/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD index d167d950e74..b287c4612bf 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD @@ -40,6 +40,7 @@ python_library( name = 'compile_context', sources = ['compile_context.py'], dependencies = [ + 'src/python/pants/backend/jvm/targets:jvm', 'src/python/pants/build_graph', 'src/python/pants/util:contextutil', ] diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py index e8819e89a2a..2f8b801667d 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py @@ -16,6 +16,9 @@ def _resolve_strict_dependencies(target): for declared in target.dependencies: + # The transitive dependencies of JarLibrary targets have + # to be included regardless of strict_deps setting, + # because they are always required for compilation. if type(declared) in (AliasTarget, JarLibrary, Target): # Is an alias. Recurse to expand. for r in _resolve_strict_dependencies(declared): From a7ae0ee8673110660c9b705a45e61a3f57cd1ebd Mon Sep 17 00:00:00 2001 From: Yujie Chen Date: Tue, 28 Mar 2017 19:47:37 -0400 Subject: [PATCH 3/6] add exports property to jar_library that aliases dependencies --- src/python/pants/backend/jvm/targets/jar_library.py | 10 ++++++++++ src/python/pants/backend/jvm/tasks/jvm_compile/BUILD | 1 - .../backend/jvm/tasks/jvm_compile/compile_context.py | 6 ++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/python/pants/backend/jvm/targets/jar_library.py b/src/python/pants/backend/jvm/targets/jar_library.py index c8151105b01..2c517cdefae 100644 --- a/src/python/pants/backend/jvm/targets/jar_library.py +++ b/src/python/pants/backend/jvm/targets/jar_library.py @@ -76,6 +76,16 @@ def excludes(self): """ return self.payload.excludes + @property + def exports(self): + """ + :API: public + """ + + # It is currently aliased to dependencies. For future work see + # https://github.com/pantsbuild/pants/issues/4398 + return self.dependencies + @staticmethod def to_jar_dependencies(relative_to, jar_library_specs, build_graph): """Convenience method to resolve a list of specs to JarLibraries and return its jars attributes. diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD b/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD index b287c4612bf..d167d950e74 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD @@ -40,7 +40,6 @@ python_library( name = 'compile_context', sources = ['compile_context.py'], dependencies = [ - 'src/python/pants/backend/jvm/targets:jvm', 'src/python/pants/build_graph', 'src/python/pants/util:contextutil', ] diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py index 2f8b801667d..e8cba52f855 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py @@ -8,7 +8,6 @@ import zipfile from contextlib import contextmanager -from pants.backend.jvm.targets.jar_library import JarLibrary from pants.build_graph.aliased_target import AliasTarget from pants.build_graph.target import Target from pants.util.contextutil import open_zip @@ -19,12 +18,11 @@ def _resolve_strict_dependencies(target): # The transitive dependencies of JarLibrary targets have # to be included regardless of strict_deps setting, # because they are always required for compilation. - if type(declared) in (AliasTarget, JarLibrary, Target): + if type(declared) in (AliasTarget, Target): # Is an alias. Recurse to expand. for r in _resolve_strict_dependencies(declared): yield r - - if type(declared) not in (AliasTarget, Target): + else: yield declared for export in _resolve_exports(declared): From 1c784ddd27299a951d90aa62ebcfd5a4426e6f94 Mon Sep 17 00:00:00 2001 From: Yujie Chen Date: Tue, 28 Mar 2017 23:09:06 -0400 Subject: [PATCH 4/6] add case for exports target alias --- .../jvm/tasks/jvm_compile/compile_context.py | 14 +++++++------ .../org/pantsbuild/testproject/exports/BUILD | 20 +++++++++++++++++++ .../test_dep_exports_integration.py | 6 ++++++ 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py index e8cba52f855..7a9d68103e7 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/compile_context.py @@ -15,9 +15,6 @@ def _resolve_strict_dependencies(target): for declared in target.dependencies: - # The transitive dependencies of JarLibrary targets have - # to be included regardless of strict_deps setting, - # because they are always required for compilation. if type(declared) in (AliasTarget, Target): # Is an alias. Recurse to expand. for r in _resolve_strict_dependencies(declared): @@ -31,9 +28,14 @@ def _resolve_strict_dependencies(target): def _resolve_exports(target): for export in getattr(target, 'exports', []): - for exp in _resolve_exports(export): - yield exp - yield export + if type(export) in (AliasTarget, Target): + # If exported target is an alias, expand its dependencies. + for dep in _resolve_strict_dependencies(export): + yield dep + else: + for exp in _resolve_exports(export): + yield exp + yield export def strict_dependencies(target, dep_context): diff --git a/testprojects/tests/scala/org/pantsbuild/testproject/exports/BUILD b/testprojects/tests/scala/org/pantsbuild/testproject/exports/BUILD index 63a6adcee4f..6cbbb0e8905 100644 --- a/testprojects/tests/scala/org/pantsbuild/testproject/exports/BUILD +++ b/testprojects/tests/scala/org/pantsbuild/testproject/exports/BUILD @@ -25,3 +25,23 @@ scala_library( dependencies=[':C'], strict_deps=True, ) + +target( + name='TB', + dependencies=[':B'], +) + +scala_library( + name='TC', + sources = ['C.scala'], + dependencies=[':TB'], + exports=[':TB'], + strict_deps=True, +) + +scala_library( + name='TD', + sources = ['D.scala'], + dependencies=[':TC'], + strict_deps=True, +) diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py index 7ed78953ddc..37950d5edf2 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py @@ -32,6 +32,12 @@ def test_compilation(self): pants_run = self.run_pants(['compile', '--compile-scalafmt-skip', target]) self.assert_success(pants_run) + def test_exports_target_alias(self): + path = os.path.join(self.SRC_PREFIX, 'scala', self.SRC_PACKAGE) + target = '{}:TD'.format(path) + pants_run = self.run_pants(['compile', '--compile-scalafmt-skip', target]) + self.assert_success(pants_run) + def modify_exports_and_compile(self, target, modify_file): with self.temporary_sourcedir() as tmp_src: src_dir = os.path.relpath(os.path.join(tmp_src, os.path.basename(self.SRC_PACKAGE)), get_buildroot()) From 3641367ed9a54e09c17dff23b683b125d992f11b Mon Sep 17 00:00:00 2001 From: Yujie Chen Date: Wed, 29 Mar 2017 07:48:06 -0400 Subject: [PATCH 5/6] remove a redundant test case --- .../jvm/tasks/jvm_compile/test_dep_exports_integration.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py index 37950d5edf2..7ed78953ddc 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py @@ -32,12 +32,6 @@ def test_compilation(self): pants_run = self.run_pants(['compile', '--compile-scalafmt-skip', target]) self.assert_success(pants_run) - def test_exports_target_alias(self): - path = os.path.join(self.SRC_PREFIX, 'scala', self.SRC_PACKAGE) - target = '{}:TD'.format(path) - pants_run = self.run_pants(['compile', '--compile-scalafmt-skip', target]) - self.assert_success(pants_run) - def modify_exports_and_compile(self, target, modify_file): with self.temporary_sourcedir() as tmp_src: src_dir = os.path.relpath(os.path.join(tmp_src, os.path.basename(self.SRC_PACKAGE)), get_buildroot()) From 6c3791e0e8ba4e2046b495685e6d2b8061fa7689 Mon Sep 17 00:00:00 2001 From: Yujie Chen Date: Wed, 29 Mar 2017 13:12:40 -0400 Subject: [PATCH 6/6] add unit tests for resolve logic --- .../backend/jvm/tasks/jvm_compile/BUILD | 11 +++ .../tasks/jvm_compile/test_compile_context.py | 73 +++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_compile_context.py 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 fb4f2947605..b98afc35d1d 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/BUILD +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/BUILD @@ -1,6 +1,17 @@ # Copyright 2014 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +python_tests( + name = 'compile_context', + sources = ['test_compile_context.py'], + dependencies = [ + '3rdparty/python:mock', + 'src/python/pants/backend/jvm/targets:java', + 'src/python/pants/backend/jvm/tasks/jvm_compile:compile_context', + 'tests/python/pants_test:base_test', + ], +) + python_tests( name = 'jvm_classpath_published', sources = ['test_jvm_classpath_published.py'], diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_compile_context.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_compile_context.py new file mode 100644 index 00000000000..b8f99c4d0f2 --- /dev/null +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_compile_context.py @@ -0,0 +1,73 @@ +# coding=utf-8 +# Copyright 2017 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) + +import mock + +from pants.backend.jvm.targets.java_library import JavaLibrary +from pants.backend.jvm.tasks.jvm_compile.compile_context import strict_dependencies +from pants_test.base_test import BaseTest + + +class CompileContextTest(BaseTest): + def generate_targets(self): + self.lib_aa = self.make_target( + 'com/foo:AA', + target_type=JavaLibrary, + sources=['com/foo/AA.scala'], + ) + + self.lib_a = self.make_target( + 'com/foo:A', + target_type=JavaLibrary, + sources=['com/foo/A.scala'], + ) + + self.lib_b = self.make_target( + 'com/foo:B', + target_type=JavaLibrary, + sources=['com/foo/B.scala'], + dependencies=[self.lib_a, self.lib_aa], + exports=[':A'], + ) + + self.lib_c = self.make_target( + 'com/foo:C', + target_type=JavaLibrary, + sources=['com/foo/C.scala'], + dependencies=[self.lib_b], + exports=[':B'], + ) + + self.lib_c_alias = self.make_target( + 'com/foo:C_alias', + dependencies=[self.lib_c], + ) + + self.lib_d = self.make_target( + 'com/foo:D', + target_type=JavaLibrary, + sources=['com/foo/D.scala'], + dependencies=[self.lib_c_alias], + exports=[':C_alias'], + ) + + self.lib_e = self.make_target( + 'com/foo:E', + target_type=JavaLibrary, + sources=['com/foo/E.scala'], + dependencies=[self.lib_d], + ) + + def test_resolve_logic(self): + self.generate_targets() + dep_context = mock.Mock() + dep_context.compiler_plugin_types = () + self.assertEqual(set(strict_dependencies(self.lib_b, dep_context)), {self.lib_a, self.lib_aa}) + self.assertEqual(set(strict_dependencies(self.lib_c, dep_context)), {self.lib_b, self.lib_a}) + self.assertEqual(set(strict_dependencies(self.lib_c_alias, dep_context)), {self.lib_c, self.lib_b, self.lib_a}) + self.assertEqual(set(strict_dependencies(self.lib_d, dep_context)), {self.lib_c, self.lib_b, self.lib_a}) + self.assertEqual(set(strict_dependencies(self.lib_e, dep_context)), {self.lib_d, self.lib_c, self.lib_b, self.lib_a})