Skip to content

Commit

Permalink
[export-dep-as-jar] Respect strict_deps in libraries field (#9145)
Browse files Browse the repository at this point in the history
### Problem
The libraries field of a target didn't respect the strict_deps of the target, making it not correct.

### Solution
Precalculate a set of "things we want in the classpath", which takes strict_deps into account.
Predicate including something in the libraries field, so that we only include things from this set.

### Result
The test introduced in the third commit failed before the change, succeeds after.
  • Loading branch information
blorente authored Feb 19, 2020
1 parent 617e9a1 commit 15a36df
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 8 deletions.
3 changes: 3 additions & 0 deletions examples/src/scala/org/pantsbuild/example/strict_deps/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package a

class Foo
3 changes: 3 additions & 0 deletions examples/src/scala/org/pantsbuild/example/strict_deps/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package b

class Foo
32 changes: 32 additions & 0 deletions examples/src/scala/org/pantsbuild/example/strict_deps/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

scala_library(
name = "a",
sources = ["A.scala"],
)

scala_library(
name = "b",
sources = ["B.scala"],
)

scala_library(
name = "c",
sources = ["C.scala"],
dependencies = [":a"],
)

scala_library(
name = "d",
sources = ["D.scala"],
dependencies = [":b", ":c"],
strict_deps = True,
)

scala_library(
name = "e",
sources = ["E.scala"],
dependencies = [":b", ":c"],
strict_deps = False,
)
5 changes: 5 additions & 0 deletions examples/src/scala/org/pantsbuild/example/strict_deps/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package a

class C {
val implementationDetail: Any = new a.Foo()
}
8 changes: 8 additions & 0 deletions examples/src/scala/org/pantsbuild/example/strict_deps/D.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package d

import a._
import b._

class App {
val foo: Foo = new Foo()
}
7 changes: 7 additions & 0 deletions examples/src/scala/org/pantsbuild/example/strict_deps/E.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package e

import a.Foo

class App {
val foo: Foo = new Foo()
}
29 changes: 21 additions & 8 deletions src/python/pants/backend/project_info/tasks/export_dep_as_jar.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,19 @@ def _zip_sources(target, location, suffix='.jar'):
zip_file.write(os.path.join(get_buildroot(), src_from_build_root), src_from_source_root)
return f

def _dependencies_to_include_in_libraries(self, t, modulizable_target_set):
dependencies_to_include = set([])
def _dependencies_to_include_in_libraries(self, t, modulizable_target_set, dependencies_needed_in_classpath):
"""
NB: We need to pass dependencies_needed_in_classpath here to make sure we're being strict_deps-aware
when computing the dependencies.
"""

dependencies_to_include = []
self.context.build_graph.walk_transitive_dependency_graph(
[direct_dep.address for direct_dep in t.dependencies],
# NB: Dependency graph between modulizable targets is represented with modules,
# so we don't need to expand those branches of the dep graph.
predicate=lambda dep: dep not in modulizable_target_set,
work=lambda dep: dependencies_to_include.add(dep),
predicate=lambda dep: (dep not in modulizable_target_set) and (dep in dependencies_needed_in_classpath),
work=lambda dep: dependencies_to_include.append(dep),
)
return list(sorted(dependencies_to_include))

Expand Down Expand Up @@ -206,9 +211,6 @@ def _process_target(
'extra_jvm_options': current_target.payload.get_field_value('extra_jvm_options', [])
}

if not current_target.is_synthetic:
info['globs'] = current_target.globs_relative_to_buildroot()

def iter_transitive_jars(jar_lib):
"""
:type jar_lib: :class:`pants.backend.jvm.targets.jar_library.JarLibrary`
Expand Down Expand Up @@ -239,8 +241,19 @@ def _full_library_set_for_target(target):
libraries.add(target.id)
return libraries

if not current_target.is_synthetic:
info['globs'] = current_target.globs_relative_to_buildroot()

def _dependencies_needed_in_classpath(target):
if isinstance(target, JvmTarget):
return [dep for dep in DependencyContext.global_instance().dependencies_respecting_strict_deps(target)]
else:
return [dep for dep in target.closure()]

dependencies_needed_in_classpath = _dependencies_needed_in_classpath(current_target)

libraries_for_target = set([self._jar_id(jar) for jar in iter_transitive_jars(current_target)])
for dep in self._dependencies_to_include_in_libraries(current_target, modulizable_target_set):
for dep in self._dependencies_to_include_in_libraries(current_target, modulizable_target_set, dependencies_needed_in_classpath):
libraries_for_target.update(_full_library_set_for_target(dep))
info['libraries'].extend(libraries_for_target)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,20 @@ def setUp(self):

self.linear_build_graph = self.make_linear_graph(['a', 'b', 'c', 'd', 'e'], target_type=ScalaLibrary)

self.strict_deps_enabled = self.make_target(
'strict_deps:enabled',
target_type=JvmTarget,
dependencies=[self.scala_with_source_dep],
strict_deps=True
)

self.strict_deps_disabled = self.make_target(
'strict_deps:disabled',
target_type=JvmTarget,
dependencies=[self.scala_with_source_dep],
strict_deps=False
)

def create_runtime_classpath_for_targets(self, target):
def path_to_zjar_with_workdir(address: Address):
return os.path.join(self.pants_workdir, address.path_safe_spec, "z.jar")
Expand Down Expand Up @@ -613,3 +627,16 @@ def test_imports_3rdparty_jars_from_transitive_dependencies(self):
'org.apache:apache-jar:12.12.2012',
result['targets'][spec]['libraries']
)

def test_libraries_respect_strict_deps(self):
enabled_spec = self.strict_deps_enabled.address.spec
enabled_result = self.execute_export_json(enabled_spec)['targets'][enabled_spec]
disabled_spec = self.strict_deps_disabled.address.spec
disabled_result = self.execute_export_json(disabled_spec)['targets'][disabled_spec]

# Both the targets under test transitively depend on this target
# but it shouldn't be included in the strict deps case.
transitive_dependency_library_entry = self.jvm_target_with_sources.id

assert transitive_dependency_library_entry in disabled_result['libraries']
assert transitive_dependency_library_entry not in enabled_result['libraries']

0 comments on commit 15a36df

Please sign in to comment.