Skip to content

Commit

Permalink
Make sure that plus-one-deps from exports of direct deps also propaga…
Browse files Browse the repository at this point in the history
…te (#764)

* PlusOne propagates PlusOne deps of exports

* rename of tests

* correct test dependency

* use scala toolchain to toggle on +1 collection
  • Loading branch information
ittaiz authored Jun 6, 2019
1 parent 7ffc700 commit 73e266b
Show file tree
Hide file tree
Showing 13 changed files with 97 additions and 26 deletions.
14 changes: 12 additions & 2 deletions scala/plusone.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,18 @@ PlusOneDeps = provider(
)

def _collect_plus_one_deps_aspect_impl(target, ctx):
return [PlusOneDeps(direct_deps = getattr(ctx.rule.attr,'deps',[]))]
if (ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].plus_one_deps_mode == "off"):
return []
export_plus_one_deps = []
for exported_dep in getattr(ctx.rule.attr,'exports',[]):
if PlusOneDeps in exported_dep:
export_plus_one_deps.extend(exported_dep[PlusOneDeps].direct_deps)
return [PlusOneDeps(direct_deps = export_plus_one_deps + getattr(ctx.rule.attr,'deps',[]))]

collect_plus_one_deps_aspect = aspect(implementation = _collect_plus_one_deps_aspect_impl,
attr_aspects = ['deps'],
attr_aspects = ['deps','exports'],
toolchains = [
"@io_bazel_rules_scala//scala:toolchain_type"
],

)
38 changes: 38 additions & 0 deletions test_expect_failure/plus_one_deps/deps_of_exports/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
load("//scala:scala.bzl", "scala_library")
#Make sure that plus-one-deps from exports of direct deps also propagate

#example with target only in exports
scala_library(
name = "test_target_using_facade",
srcs = ["A.scala"],
deps = [":facade"],
)
scala_library(
name = "facade",
exports = [":direct_dep"]
)

#example with target in deps & exports
scala_library(
name = "test_target",
srcs = ["A.scala"],
deps = [":direct_dep"],
)
scala_library(
name = "direct_dep",
srcs = ["B.scala"],
deps = [":exported_dep"],
exports = [":exported_dep"],
)

#common
scala_library(
name = "exported_dep",
srcs = ["C.scala"],
deps = [":plus_one_dep_of_exported"],
)
scala_library(
name = "plus_one_dep_of_exported",
srcs = ["D.scala"],
)

21 changes: 0 additions & 21 deletions test_expect_failure/plus_one_deps/exports_deps/BUILD.bazel

This file was deleted.

5 changes: 5 additions & 0 deletions test_expect_failure/plus_one_deps/exports_of_deps/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package scalarules.test_expect_failure.plus_one_deps.internal_deps

class A {
println(new B().hi)
}
5 changes: 5 additions & 0 deletions test_expect_failure/plus_one_deps/exports_of_deps/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package scalarules.test_expect_failure.plus_one_deps.internal_deps

class B extends C {
def hi: String = "hi"
}
22 changes: 22 additions & 0 deletions test_expect_failure/plus_one_deps/exports_of_deps/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
load("//scala:scala.bzl", "scala_library")
#Make sure that plus-one-deps exported targets are also propagated (in addition to the plus-one-dep outputs)
scala_library(
name = "test_target",
srcs = ["A.scala"],
deps = [":direct_dep"],
)
scala_library(
name = "direct_dep",
srcs = ["B.scala"],
deps = [":plus_one_dep"],
)
scala_library(
name = "plus_one_dep",
srcs = ["C.scala"],
deps = [":exported_dep"],
exports = ["exported_dep"],
)
scala_library(
name = "exported_dep",
srcs = ["D.scala"],
)
3 changes: 3 additions & 0 deletions test_expect_failure/plus_one_deps/exports_of_deps/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package scalarules.test_expect_failure.plus_one_deps.internal_deps

class C extends D
5 changes: 5 additions & 0 deletions test_expect_failure/plus_one_deps/exports_of_deps/D.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package scalarules.test_expect_failure.plus_one_deps.internal_deps

class D {

}
10 changes: 7 additions & 3 deletions test_rules_scala.sh
Original file line number Diff line number Diff line change
Expand Up @@ -929,8 +929,11 @@ test_compilation_fails_with_plus_one_deps_undefined() {
test_compilation_succeeds_with_plus_one_deps_on_for_external_deps() {
bazel build --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" //test_expect_failure/plus_one_deps/external_deps:a
}
test_compilation_succeeds_with_plus_one_deps_on_also_for_exports() {
bazel build --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" //test_expect_failure/plus_one_deps/exports_deps:a
test_compilation_succeeds_with_plus_one_deps_on_also_for_exports_of_deps() {
bazel build --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" //test_expect_failure/plus_one_deps/exports_of_deps/...
}
test_compilation_succeeds_with_plus_one_deps_on_also_for_deps_of_exports() {
bazel build --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" //test_expect_failure/plus_one_deps/deps_of_exports/...
}
test_plus_one_deps_only_works_for_java_info_targets() {
#for example doesn't break scala proto which depends on proto_library
Expand Down Expand Up @@ -1044,7 +1047,8 @@ $runner test_override_javabin
$runner test_compilation_succeeds_with_plus_one_deps_on
$runner test_compilation_fails_with_plus_one_deps_undefined
$runner test_compilation_succeeds_with_plus_one_deps_on_for_external_deps
$runner test_compilation_succeeds_with_plus_one_deps_on_also_for_exports
$runner test_compilation_succeeds_with_plus_one_deps_on_also_for_exports_of_deps
$runner test_compilation_succeeds_with_plus_one_deps_on_also_for_deps_of_exports
$runner test_plus_one_deps_only_works_for_java_info_targets
$runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps"
$runner test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps
Expand Down

0 comments on commit 73e266b

Please sign in to comment.