diff --git a/WORKSPACE b/WORKSPACE index 4706239d4..77757b70d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -171,4 +171,30 @@ git_repository( name = "bazel_skylib", remote = "https://github.com/bazelbuild/bazel-skylib.git", tag = "0.6.0", -) \ No newline at end of file +) + +## deps for tests of limited deps support +scala_maven_import_external( + name = "org_springframework_spring_core", + artifact = "org.springframework:spring-core:5.1.5.RELEASE", + jar_sha256 = "f771b605019eb9d2cf8f60c25c050233e39487ff54d74c93d687ea8de8b7285a", + licenses = ["notice"], # Apache 2.0 + server_urls = [ + "https://repo1.maven.org/maven2/", + "https://mirror.bazel.build/repo1.maven.org/maven2", + ], +) + +scala_maven_import_external( + name = "org_springframework_spring_tx", + artifact = "org.springframework:spring-tx:5.1.5.RELEASE", + jar_sha256 = "666f72b73c7e6b34e5bb92a0d77a14cdeef491c00fcb07a1e89eb62b08500135", + licenses = ["notice"], # Apache 2.0 + server_urls = [ + "https://repo1.maven.org/maven2/", + "https://mirror.bazel.build/repo1.maven.org/maven2", + ], + deps = [ + "@org_springframework_spring_core" + ] +) diff --git a/scala/plusone.bzl b/scala/plusone.bzl new file mode 100644 index 000000000..90b1c1f43 --- /dev/null +++ b/scala/plusone.bzl @@ -0,0 +1,17 @@ +""" +Keeps direct compile dependencies of targets. +This enables targets to pass the to compiler the plus one dependencies in addition to the direct ones. +For motivation of plus one see the e2e tests +""" +PlusOneDeps = provider( + fields = { + 'direct_deps' : 'list of direct compile dependencies of a target', + } +) + +def _collect_plus_one_deps_aspect_impl(target, ctx): + return [PlusOneDeps(direct_deps = getattr(ctx.rule.attr,'deps',[]))] + +collect_plus_one_deps_aspect = aspect(implementation = _collect_plus_one_deps_aspect_impl, + attr_aspects = ['deps'], +) diff --git a/scala/private/common.bzl b/scala/private/common.bzl index e54841a3b..2cea9ffd3 100644 --- a/scala/private/common.bzl +++ b/scala/private/common.bzl @@ -1,4 +1,5 @@ load("@io_bazel_rules_scala//scala:jars_to_labels.bzl", "JarsToLabelsInfo") +load("@io_bazel_rules_scala//scala:plusone.bzl", "PlusOneDeps") def write_manifest(ctx): main_class = getattr(ctx.attr, "main_class", None) @@ -22,21 +23,25 @@ def collect_srcjars(targets): def collect_jars( dep_targets, dependency_analyzer_is_off = True, - unused_dependency_checker_is_off = True): + unused_dependency_checker_is_off = True, + plus_one_deps_is_off = True): """Compute the runtime and compile-time dependencies from the given targets""" # noqa if dependency_analyzer_is_off: return _collect_jars_when_dependency_analyzer_is_off( dep_targets, unused_dependency_checker_is_off, + plus_one_deps_is_off, ) else: return _collect_jars_when_dependency_analyzer_is_on(dep_targets) def _collect_jars_when_dependency_analyzer_is_off( dep_targets, - unused_dependency_checker_is_off): + unused_dependency_checker_is_off, + plus_one_deps_is_off): compile_jars = [] + plus_one_deps_compile_jars = [] runtime_jars = [] jars2labels = {} @@ -58,11 +63,16 @@ def _collect_jars_when_dependency_analyzer_is_off( else: print("ignored dependency, has no JavaInfo: " + str(dep_target)) + if (not plus_one_deps_is_off) and (PlusOneDeps in dep_target): + plus_one_deps_compile_jars.append( + depset(transitive = [dep[JavaInfo].compile_jars for dep in dep_target[PlusOneDeps].direct_deps if JavaInfo in dep ]) + ) + return struct( compile_jars = depset(transitive = compile_jars), transitive_runtime_jars = depset(transitive = runtime_jars), jars2labels = JarsToLabelsInfo(jars_to_labels = jars2labels), - transitive_compile_jars = depset(), + transitive_compile_jars = depset(transitive = compile_jars + plus_one_deps_compile_jars), ) def _collect_jars_when_dependency_analyzer_is_on(dep_targets): diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 8ca5237eb..a5077817f 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -220,6 +220,8 @@ CurrentTarget: {current_target} ignored_targets = ignored_targets, current_target = current_target, ) + if is_dependency_analyzer_off(ctx) and not _is_plus_one_deps_off(ctx): + compiler_classpath_jars = transitive_compile_jars plugins_list = plugins.to_list() plugin_arg = _join_path(plugins_list) @@ -616,6 +618,9 @@ def is_dependency_analyzer_on(ctx): def is_dependency_analyzer_off(ctx): return not is_dependency_analyzer_on(ctx) +def _is_plus_one_deps_off(ctx): + return ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].plus_one_deps_mode == "off" + # Extract very common code out from dependency analysis into single place # automatically adds dependency on scala-library and scala-reflect # collects jars from deps, runtime jars from runtime_deps, and @@ -631,6 +636,7 @@ def _collect_jars_from_common_ctx( ctx.attr.deps + extra_deps + base_classpath, dependency_analyzer_is_off, unused_dependency_checker_is_off, + _is_plus_one_deps_off(ctx), ) ( @@ -985,9 +991,10 @@ def scala_test_impl(ctx): ] unused_dependency_checker_is_off = unused_dependency_checker_mode == "off" + scalatest_base_classpath = scalac_provider.default_classpath + [ctx.attr._scalatest] jars = _collect_jars_from_common_ctx( ctx, - scalac_provider.default_classpath, + scalatest_base_classpath, extra_runtime_deps = [ ctx.attr._scalatest_reporter, ctx.attr._scalatest_runner, @@ -1006,36 +1013,6 @@ def scala_test_impl(ctx): jars.jars2labels, ) - # _scalatest is an http_jar, so its compile jar is run through ijar - # however, contains macros, so need to handle separately - scalatest_jars = collect_jars([ctx.attr._scalatest]).transitive_runtime_jars - cjars = depset(transitive = [cjars, scalatest_jars]) - transitive_rjars = depset(transitive = [transitive_rjars, scalatest_jars]) - - if is_dependency_analyzer_on(ctx): - transitive_compile_jars = depset( - transitive = [scalatest_jars, transitive_compile_jars], - ) - scalatest_jars_list = scalatest_jars.to_list() - j2l = jars_to_labels.jars_to_labels - add_labels_of_jars_to( - j2l, - ctx.attr._scalatest, - scalatest_jars_list, - scalatest_jars_list, - ) - jars_to_labels = JarsToLabelsInfo(jars_to_labels = j2l) - - elif not unused_dependency_checker_is_off: - j2l = jars_to_labels.jars_to_labels - add_labels_of_jars_to( - j2l, - ctx.attr._scalatest, - [], - scalatest_jars.to_list(), - ) - jars_to_labels = JarsToLabelsInfo(jars_to_labels = j2l) - args = " ".join([ "-R \"{path}\"".format(path = ctx.outputs.jar.short_path), _scala_test_flags(ctx), diff --git a/scala/scala.bzl b/scala/scala.bzl index 03c874722..c4694cc31 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -24,6 +24,10 @@ load( "@io_bazel_rules_scala//specs2:specs2_junit.bzl", _specs2_junit_dependencies = "specs2_junit_dependencies", ) +load( + "@io_bazel_rules_scala//scala:plusone.bzl", + _collect_plus_one_deps_aspect = "collect_plus_one_deps_aspect", +) _launcher_template = { "_java_stub_template": attr.label( @@ -105,7 +109,7 @@ _junit_resolve_deps = { # Common attributes reused across multiple rules. _common_attrs_for_plugin_bootstrapping = { "srcs": attr.label_list(allow_files = [".scala", ".srcjar", ".java"]), - "deps": attr.label_list(), + "deps": attr.label_list(aspects = [_collect_plus_one_deps_aspect]), "plugins": attr.label_list(allow_files = [".jar"]), "runtime_deps": attr.label_list(providers = [[JavaInfo]]), "data": attr.label_list(allow_files = True), diff --git a/scala/scala_toolchain.bzl b/scala/scala_toolchain.bzl index cce20efac..c228bc4ba 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -8,6 +8,7 @@ def _scala_toolchain_impl(ctx): scalacopts = ctx.attr.scalacopts, scalac_provider_attr = ctx.attr.scalac_provider_attr, unused_dependency_checker_mode = ctx.attr.unused_dependency_checker_mode, + plus_one_deps_mode = ctx.attr.plus_one_deps_mode, ) return [toolchain] @@ -23,5 +24,9 @@ scala_toolchain = rule( default = "off", values = ["off", "warn", "error"], ), + "plus_one_deps_mode": attr.string( + default = "off", + values = ["off", "on"], + ), }, ) diff --git a/test_expect_failure/plus_one_deps/BUILD.bazel b/test_expect_failure/plus_one_deps/BUILD.bazel new file mode 100644 index 000000000..308a5193b --- /dev/null +++ b/test_expect_failure/plus_one_deps/BUILD.bazel @@ -0,0 +1,25 @@ +load("//scala:scala_toolchain.bzl", "scala_toolchain") +scala_toolchain( + name = "plus_one_deps_impl", + plus_one_deps_mode = "on", + visibility = ["//visibility:public"], +) +toolchain( + name = "plus_one_deps", + toolchain = "plus_one_deps_impl", + toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type", + visibility = ["//visibility:public"], +) +scala_toolchain( + name = "plus_one_deps_with_unused_error_impl", + unused_dependency_checker_mode = "error", + plus_one_deps_mode = "on", + visibility = ["//visibility:public"], +) +toolchain( + name = "plus_one_deps_with_unused_error", + toolchain = "plus_one_deps_with_unused_error_impl", + toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type", + visibility = ["//visibility:public"], +) + diff --git a/test_expect_failure/plus_one_deps/exports_deps/A.scala b/test_expect_failure/plus_one_deps/exports_deps/A.scala new file mode 100644 index 000000000..6886de430 --- /dev/null +++ b/test_expect_failure/plus_one_deps/exports_deps/A.scala @@ -0,0 +1,5 @@ +package scalarules.test_expect_failure.plus_one_deps.internal_deps + +class A { + println(new B().hi) +} diff --git a/test_expect_failure/plus_one_deps/exports_deps/B.scala b/test_expect_failure/plus_one_deps/exports_deps/B.scala new file mode 100644 index 000000000..dc3fd5165 --- /dev/null +++ b/test_expect_failure/plus_one_deps/exports_deps/B.scala @@ -0,0 +1,5 @@ +package scalarules.test_expect_failure.plus_one_deps.internal_deps + +class B extends C { + def hi: String = "hi" +} \ No newline at end of file diff --git a/test_expect_failure/plus_one_deps/exports_deps/BUILD.bazel b/test_expect_failure/plus_one_deps/exports_deps/BUILD.bazel new file mode 100644 index 000000000..d1628eb99 --- /dev/null +++ b/test_expect_failure/plus_one_deps/exports_deps/BUILD.bazel @@ -0,0 +1,21 @@ +load("//scala:scala.bzl", "scala_library") +scala_library( + name = "a", + srcs = ["A.scala"], + deps = [":b"], +) +scala_library( + name = "b", + srcs = ["B.scala"], + deps = [":c"], +) +scala_library( + name = "c", + srcs = ["C.scala"], + deps = [":d"], + exports = ["d"], +) +scala_library( + name = "d", + srcs = ["D.scala"], +) diff --git a/test_expect_failure/plus_one_deps/exports_deps/C.scala b/test_expect_failure/plus_one_deps/exports_deps/C.scala new file mode 100644 index 000000000..942f31db0 --- /dev/null +++ b/test_expect_failure/plus_one_deps/exports_deps/C.scala @@ -0,0 +1,3 @@ +package scalarules.test_expect_failure.plus_one_deps.internal_deps + +class C extends D \ No newline at end of file diff --git a/test_expect_failure/plus_one_deps/exports_deps/D.scala b/test_expect_failure/plus_one_deps/exports_deps/D.scala new file mode 100644 index 000000000..c74bf6530 --- /dev/null +++ b/test_expect_failure/plus_one_deps/exports_deps/D.scala @@ -0,0 +1,5 @@ +package scalarules.test_expect_failure.plus_one_deps.internal_deps + +class D { + +} diff --git a/test_expect_failure/plus_one_deps/external_deps/A.scala b/test_expect_failure/plus_one_deps/external_deps/A.scala new file mode 100644 index 000000000..2d21dc92b --- /dev/null +++ b/test_expect_failure/plus_one_deps/external_deps/A.scala @@ -0,0 +1,6 @@ +package scalarules.test_expect_failure.plus_one_deps.external_deps +import org.springframework.dao.DataAccessException + +class A { + println(classOf[DataAccessException]) +} \ No newline at end of file diff --git a/test_expect_failure/plus_one_deps/external_deps/BUILD.bazel b/test_expect_failure/plus_one_deps/external_deps/BUILD.bazel new file mode 100644 index 000000000..ebfebfe63 --- /dev/null +++ b/test_expect_failure/plus_one_deps/external_deps/BUILD.bazel @@ -0,0 +1,7 @@ +load("//scala:scala.bzl", "scala_library") +scala_library( + name = "a", + srcs = ["A.scala"], + deps = ["@org_springframework_spring_tx"] + +) \ No newline at end of file diff --git a/test_expect_failure/plus_one_deps/internal_deps/A.scala b/test_expect_failure/plus_one_deps/internal_deps/A.scala new file mode 100644 index 000000000..6886de430 --- /dev/null +++ b/test_expect_failure/plus_one_deps/internal_deps/A.scala @@ -0,0 +1,5 @@ +package scalarules.test_expect_failure.plus_one_deps.internal_deps + +class A { + println(new B().hi) +} diff --git a/test_expect_failure/plus_one_deps/internal_deps/B.scala b/test_expect_failure/plus_one_deps/internal_deps/B.scala new file mode 100644 index 000000000..dc3fd5165 --- /dev/null +++ b/test_expect_failure/plus_one_deps/internal_deps/B.scala @@ -0,0 +1,5 @@ +package scalarules.test_expect_failure.plus_one_deps.internal_deps + +class B extends C { + def hi: String = "hi" +} \ No newline at end of file diff --git a/test_expect_failure/plus_one_deps/internal_deps/BUILD.bazel b/test_expect_failure/plus_one_deps/internal_deps/BUILD.bazel new file mode 100644 index 000000000..a8a7e6026 --- /dev/null +++ b/test_expect_failure/plus_one_deps/internal_deps/BUILD.bazel @@ -0,0 +1,15 @@ +load("//scala:scala.bzl", "scala_library") +scala_library( + name = "a", + srcs = ["A.scala"], + deps = [":b"], +) +scala_library( + name = "b", + srcs = ["B.scala"], + deps = [":c"], +) +scala_library( + name = "c", + srcs = ["C.scala"], +) diff --git a/test_expect_failure/plus_one_deps/internal_deps/C.scala b/test_expect_failure/plus_one_deps/internal_deps/C.scala new file mode 100644 index 000000000..d5b862d55 --- /dev/null +++ b/test_expect_failure/plus_one_deps/internal_deps/C.scala @@ -0,0 +1,3 @@ +package scalarules.test_expect_failure.plus_one_deps.internal_deps + +class C \ No newline at end of file diff --git a/test_expect_failure/plus_one_deps/with_unused_deps/A.scala b/test_expect_failure/plus_one_deps/with_unused_deps/A.scala new file mode 100644 index 000000000..b2ea8d134 --- /dev/null +++ b/test_expect_failure/plus_one_deps/with_unused_deps/A.scala @@ -0,0 +1,5 @@ +package scalarules.test_expect_failure.plus_one_deps.with_unused_deps + +class A { + println(new B().hi) +} diff --git a/test_expect_failure/plus_one_deps/with_unused_deps/B.scala b/test_expect_failure/plus_one_deps/with_unused_deps/B.scala new file mode 100644 index 000000000..425daa6f5 --- /dev/null +++ b/test_expect_failure/plus_one_deps/with_unused_deps/B.scala @@ -0,0 +1,8 @@ +package scalarules.test_expect_failure.plus_one_deps.with_unused_deps + +class B { + def hi: String = { + println(classOf[C]) + "hi" + } +} \ No newline at end of file diff --git a/test_expect_failure/plus_one_deps/with_unused_deps/BUILD.bazel b/test_expect_failure/plus_one_deps/with_unused_deps/BUILD.bazel new file mode 100644 index 000000000..12eb9834f --- /dev/null +++ b/test_expect_failure/plus_one_deps/with_unused_deps/BUILD.bazel @@ -0,0 +1,15 @@ +load("//scala:scala.bzl", "scala_library") +scala_library( + name = "a", + srcs = ["A.scala"], + deps = [":b",":c"], +) +scala_library( + name = "b", + srcs = ["B.scala"], + deps = [":c"], +) +scala_library( + name = "c", + srcs = ["C.scala"], +) diff --git a/test_expect_failure/plus_one_deps/with_unused_deps/C.scala b/test_expect_failure/plus_one_deps/with_unused_deps/C.scala new file mode 100644 index 000000000..0f8fc2775 --- /dev/null +++ b/test_expect_failure/plus_one_deps/with_unused_deps/C.scala @@ -0,0 +1,3 @@ +package scalarules.test_expect_failure.plus_one_deps.with_unused_deps + +class C \ No newline at end of file diff --git a/test_expect_failure/scala_import/ScalaImportPropagatesCompileDepsTest.scala b/test_expect_failure/scala_import/ScalaImportPropagatesCompileDepsTest.scala index 8789ff952..ca7e6efaf 100644 --- a/test_expect_failure/scala_import/ScalaImportPropagatesCompileDepsTest.scala +++ b/test_expect_failure/scala_import/ScalaImportPropagatesCompileDepsTest.scala @@ -7,7 +7,7 @@ import org.specs2.mutable.SpecificationWithJUnit class ScalaImportPropagatesCompileDepsTest extends SpecificationWithJUnit { "scala_import" should { - "propagate runtime deps" in { + "propagate compile time deps" in { println(classOf[Cache[String, String]]) println(classOf[ArrayUtils]) println(classOf[cats.Applicative[Any]]) diff --git a/test_rules_scala.sh b/test_rules_scala.sh index cdd1293da..3ee51945b 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -904,6 +904,26 @@ test_scala_import_fetch_sources() { assert_file_exists $bazel_out_external_guava_21/$srcjar_name } +test_compilation_succeeds_with_plus_one_deps_on() { + bazel build --extra_toolchains=//test_expect_failure/plus_one_deps:plus_one_deps //test_expect_failure/plus_one_deps/internal_deps:a +} +test_compilation_fails_with_plus_one_deps_undefined() { + action_should_fail build //test_expect_failure/plus_one_deps/internal_deps:a +} +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_plus_one_deps_only_works_for_java_info_targets() { + #for example doesn't break scala proto which depends on proto_library + bazel build --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" //test/proto:test_proto +} +test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps() { + action_should_fail build --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps_with_unused_error" //test_expect_failure/plus_one_deps/with_unused_deps:a +} + assert_file_exists() { if [[ -f $1 ]]; then echo "File $1 exists." @@ -997,3 +1017,11 @@ $runner test_scala_import_source_jar_should_be_fetched_when_env_bazel_jvm_fetch_ $runner test_scala_import_source_jar_should_not_be_fetched_when_env_bazel_jvm_fetch_sources_is_set_to_non_true $runner test_unused_dependency_checker_mode_warn $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_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 +