diff --git a/README.md b/README.md index 799d44a84..76b27af75 100644 --- a/README.md +++ b/README.md @@ -104,21 +104,23 @@ In order to make a java rule use this jar file, use the `java_import` rule.

List of labels, required

List of Scala .scala source files used to build the - library

+ library. These may be .srcjar jar files that contain source code.

deps

List of labels, optional

-

List of other libraries to linked to this library target

+

List of other libraries to linked to this library target. + These must be jvm targets (scala_library, java_library, java_import, etc...)

runtime_deps

List of labels, optional

-

List of other libraries to put on the classpath only at runtime. This is rarely needed in Scala.

+

List of other libraries to put on the classpath only at runtime. This is rarely needed in Scala. + These must be jvm targets (scala_library, java_library, java_import, etc...)

@@ -127,7 +129,8 @@ In order to make a java rule use this jar file, use the `java_import` rule.

List of labels, optional

List of targets to add to the dependencies of those that depend on this target. Similar to the `java_library` parameter of the same name. Use this sparingly as it weakens the - precision of the build graph.

+ precision of the build graph. + These must be jvm targets (scala_library, java_library, java_import, etc...)

@@ -260,14 +263,16 @@ A `scala_binary` requires a `main_class` attribute. deps

List of labels, optional

-

List of other libraries to linked to this binary target

+

List of other libraries to linked to this binary target. + These must be jvm targets (scala_library, java_library, java_import, etc...)

runtime_deps

List of labels, optional

-

List of other libraries to put on the classpath only at runtime. This is rarely needed in Scala.

+

List of other libraries to put on the classpath only at runtime. This is rarely needed in Scala. + These must be jvm targets (scala_library, java_library, java_import, etc...)

diff --git a/scala/BUILD b/scala/BUILD index 06dbacbbf..b2eefcd35 100644 --- a/scala/BUILD +++ b/scala/BUILD @@ -17,3 +17,9 @@ toolchain( toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type", visibility = ["//visibility:public"], ) + +java_import( + name = "bazel_test_runner_deploy", + jars = ["@bazel_tools//tools/jdk:TestRunner_deploy.jar"], + visibility = ["//visibility:public"], +) diff --git a/scala/private/common.bzl b/scala/private/common.bzl index c8049cb19..a97765579 100644 --- a/scala/private/common.bzl +++ b/scala/private/common.bzl @@ -26,15 +26,14 @@ def _collect_jars_when_dependency_analyzer_is_off(dep_targets): runtime_jars = [] for dep_target in dep_targets: + # we require a JavaInfo for dependencies + # must use java_import or scala_import if you have raw files if JavaInfo in dep_target: java_provider = dep_target[JavaInfo] compile_jars.append(java_provider.compile_jars) runtime_jars.append(java_provider.transitive_runtime_jars) else: - # support http_file pointed at a jar. http_jar uses ijar, - # which breaks scala macros - compile_jars.append(filter_not_sources(dep_target.files)) - runtime_jars.append(filter_not_sources(dep_target.files)) + print("ignored dependency, has no JavaInfo: " + str(dep_target)) return struct( compile_jars = depset(transitive = compile_jars), @@ -49,26 +48,21 @@ def _collect_jars_when_dependency_analyzer_is_on(dep_targets): runtime_jars = [] for dep_target in dep_targets: - current_dep_compile_jars = None - current_dep_transitive_compile_jars = None - + # we require a JavaInfo for dependencies + # must use java_import or scala_import if you have raw files if JavaInfo in dep_target: java_provider = dep_target[JavaInfo] current_dep_compile_jars = java_provider.compile_jars current_dep_transitive_compile_jars = java_provider.transitive_compile_time_jars runtime_jars.append(java_provider.transitive_runtime_jars) + + compile_jars.append(current_dep_compile_jars) + transitive_compile_jars.append(current_dep_transitive_compile_jars) + add_labels_of_jars_to(jars2labels, dep_target, + current_dep_transitive_compile_jars.to_list(), + current_dep_compile_jars.to_list()) else: - # support http_file pointed at a jar. http_jar uses ijar, - # which breaks scala macros - current_dep_compile_jars = filter_not_sources(dep_target.files) - current_dep_transitive_compile_jars = filter_not_sources(dep_target.files) - runtime_jars.append(filter_not_sources(dep_target.files)) - - compile_jars.append(current_dep_compile_jars) - transitive_compile_jars.append(current_dep_transitive_compile_jars) - add_labels_of_jars_to(jars2labels, dep_target, - current_dep_transitive_compile_jars.to_list(), - current_dep_compile_jars.to_list()) + print("ignored dependency, has no JavaInfo: " + str(dep_target)) return struct( compile_jars = depset(transitive = compile_jars), diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 81e823e5b..4163f6c76 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -457,12 +457,7 @@ def _collect_runtime_jars(dep_targets): runtime_jars = [] for dep_target in dep_targets: - if JavaInfo in dep_target: - runtime_jars.append(dep_target[JavaInfo].transitive_runtime_jars) - else: - # support http_file pointed at a jar. http_jar uses ijar, - # which breaks scala macros - runtime_jars.append(filter_not_sources(dep_target.files)) + runtime_jars.append(dep_target[JavaInfo].transitive_runtime_jars) return runtime_jars @@ -562,7 +557,9 @@ def _lib(ctx, non_macro_lib): # this information through. extra_information allows passing # this information through, and it is up to the new_targets # to filter and make sense of this information. - extra_information=_collect_extra_information(ctx.attr.deps), + # unfortunately, we need to see this for scrooge and protobuf to work, + # but those are generating srcjar, so they should really come in via srcs + extra_information=_collect_extra_information(ctx.attr.deps + ctx.attr.srcs), jars_to_labels = jars.jars2labels, ) diff --git a/scala/scala.bzl b/scala/scala.bzl index 2d99e1351..dd5873dd0 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -102,7 +102,7 @@ _common_attrs_for_plugin_bootstrapping = { "srcs": attr.label_list(allow_files = [".scala", ".srcjar", ".java"]), "deps": attr.label_list(), "plugins": attr.label_list(allow_files = [".jar"]), - "runtime_deps": attr.label_list(), + "runtime_deps": attr.label_list(providers = [[JavaInfo]]), "data": attr.label_list(allow_files = True, cfg = "data"), "resources": attr.label_list(allow_files = True), "resource_strip_prefix": attr.string(), @@ -219,13 +219,10 @@ _scala_test_attrs = { "full_stacktraces": attr.bool(default = True), "_scalatest": attr.label( default = Label( - "//external:io_bazel_rules_scala/dependency/scalatest/scalatest"), - allow_files = True), + "//external:io_bazel_rules_scala/dependency/scalatest/scalatest")), "_scalatest_runner": attr.label( - executable = True, cfg = "host", - default = Label("//src/java/io/bazel/rulesscala/scala_test:runner.jar"), - allow_files = True), + default = Label("//src/java/io/bazel/rulesscala/scala_test:runner")), "_scalatest_reporter": attr.label( default = Label("//scala/support:test_reporter")), } @@ -476,7 +473,8 @@ _scala_junit_test_attrs = { "//external:io_bazel_rules_scala/dependency/hamcrest/hamcrest_core") ), "_bazel_test_runner": attr.label( - default = Label("@bazel_tools//tools/jdk:TestRunner_deploy.jar"), + default = Label( + "@io_bazel_rules_scala//scala:bazel_test_runner_deploy"), allow_files = True), } _scala_junit_test_attrs.update(_launcher_template) diff --git a/src/java/io/bazel/rulesscala/scala_test/BUILD b/src/java/io/bazel/rulesscala/scala_test/BUILD index 4520a832e..34c233909 100644 --- a/src/java/io/bazel/rulesscala/scala_test/BUILD +++ b/src/java/io/bazel/rulesscala/scala_test/BUILD @@ -1,4 +1,4 @@ -java_binary( +java_library( name = "runner", srcs = ["Runner.java"], visibility = ["//visibility:public"], diff --git a/test/src/main/scala/scalarules/test/sources_jars_in_deps/BUILD b/test/src/main/scala/scalarules/test/sources_jars_in_deps/BUILD index 29f73c471..8a2d0d7af 100644 --- a/test/src/main/scala/scalarules/test/sources_jars_in_deps/BUILD +++ b/test/src/main/scala/scalarules/test/sources_jars_in_deps/BUILD @@ -4,6 +4,6 @@ scala_library( name = "source_jar_not_oncp", srcs = ["ReferCatsImplicits.scala"], deps = [ - "@org_typelevel__cats_core//jar:file", + "@org_typelevel__cats_core//jar", ], ) diff --git a/test/src/main/scala/scalarules/test/twitter_scrooge/BUILD b/test/src/main/scala/scalarules/test/twitter_scrooge/BUILD index 98450b08e..df4068b95 100644 --- a/test/src/main/scala/scalarules/test/twitter_scrooge/BUILD +++ b/test/src/main/scala/scalarules/test/twitter_scrooge/BUILD @@ -190,3 +190,13 @@ scala_binary( main_class = "scalarules.test.twitter_scrooge.BareThrifts", deps = [":bare_thrift_scrooge"], ) + +sh_test( + name = "libthrift2_a_not_on_classpath", + srcs = ["grep.sh"], + args = [ + "libthrift2_a", + "$(location :justscrooges)", + ], + data = [":justscrooges"], +) diff --git a/test/src/main/scala/scalarules/test/twitter_scrooge/grep.sh b/test/src/main/scala/scalarules/test/twitter_scrooge/grep.sh new file mode 100755 index 000000000..880b6e840 --- /dev/null +++ b/test/src/main/scala/scalarules/test/twitter_scrooge/grep.sh @@ -0,0 +1,13 @@ +#!/bin/sh + + +# this is used by a sh_test to test that a string is NOT +# found in a file. This is used for instance to check if +# stray jars are not making it onto the classpath + +if grep -q $1 $2 ; then + echo "ERROR: found $1" + exit 1 +else + exit 0 +fi diff --git a/test_expect_failure/missing_direct_deps/external_deps/BUILD b/test_expect_failure/missing_direct_deps/external_deps/BUILD index 363e40774..9585f74f2 100644 --- a/test_expect_failure/missing_direct_deps/external_deps/BUILD +++ b/test_expect_failure/missing_direct_deps/external_deps/BUILD @@ -31,5 +31,5 @@ scala_library( srcs = [ "B.scala", ], - deps = ["@com_google_guava_guava_21_0_with_file//jar:file"], + deps = ["@com_google_guava_guava_21_0_with_file//jar"], ) diff --git a/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl b/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl index d20515454..98fe7bdef 100644 --- a/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl +++ b/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl @@ -12,8 +12,7 @@ def _custom_jvm_impl(ctx): def _collect(deps): transitive_compile_jars = depset() for dep_target in deps: - transitive_compile_jars += dep_target[ - java_common.provider].transitive_compile_time_jars + transitive_compile_jars += dep_target[JavaInfo].transitive_compile_time_jars return transitive_compile_jars custom_jvm = rule( diff --git a/test_rules_scala.sh b/test_rules_scala.sh index e2507fdd6..f1ff88e28 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -159,7 +159,7 @@ test_scala_library_expect_failure_on_missing_direct_external_deps_jar() { } test_scala_library_expect_failure_on_missing_direct_external_deps_file_group() { - dependenecy_target='@com_google_guava_guava_21_0_with_file//jar:file' + dependenecy_target='@com_google_guava_guava_21_0_with_file//jar:jar' test_target='test_expect_failure/missing_direct_deps/external_deps:transitive_external_dependency_user_file_group' test_scala_library_expect_failure_on_missing_direct_deps $dependenecy_target $test_target diff --git a/twitter_scrooge/twitter_scrooge.bzl b/twitter_scrooge/twitter_scrooge.bzl index 6dda175d4..1b6904726 100644 --- a/twitter_scrooge/twitter_scrooge.bzl +++ b/twitter_scrooge/twitter_scrooge.bzl @@ -289,12 +289,11 @@ def scrooge_scala_library(name, with_finagle = with_finagle, ) - # deps from macro invocation would come via srcjar - # however, retained to make dependency analysis via aspects easier scala_library( name = name, + srcs = [srcjar], deps = deps + remote_jars + [ - srcjar, "//external:io_bazel_rules_scala/dependency/thrift/libthrift", + "//external:io_bazel_rules_scala/dependency/thrift/libthrift", "//external:io_bazel_rules_scala/dependency/thrift/scrooge_core" ], exports = deps + remote_jars + [