From e19f7e3f60fbfdf17f6adecd44f5dcbe8db8318c Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Mon, 5 Aug 2019 15:50:31 -0600 Subject: [PATCH 1/5] Add scala_test_jvm_flags to the toolchain --- docs/scala_toolchain.md | 18 ++++++-- scala/private/rule_impls.bzl | 24 ++++++++--- scala/scala_toolchain.bzl | 2 + .../scala_test_jvm_flags/BUILD | 43 +++++++++++++++++++ .../scala_test_jvm_flags/Empty.scala | 3 ++ test_rules_scala.sh | 17 +++++++- 6 files changed, 98 insertions(+), 9 deletions(-) create mode 100644 test_expect_failure/scala_test_jvm_flags/BUILD create mode 100644 test_expect_failure/scala_test_jvm_flags/Empty.scala diff --git a/docs/scala_toolchain.md b/docs/scala_toolchain.md index d0e2d75b3..9bcc2b82c 100644 --- a/docs/scala_toolchain.md +++ b/docs/scala_toolchain.md @@ -63,9 +63,6 @@ scala_register_toolchains()

Extra compiler options for this binary to be passed to scalac.

-

- This is overridden by the `scalac_jvm_flags` attribute on individual targets. -

@@ -75,6 +72,21 @@ scala_register_toolchains()

List of JVM flags to be passed to scalac. For example ["-Xmx5G"] could be passed to control memory usage of Scalac.

+

+ This is overridden by the scalac_jvm_flags attribute on individual targets. +

+ + + + scala_test_jvm_flags + +

List of strings; optional

+

+ List of JVM flags to be passed to the ScalaTest runner. For example ["-Xmx5G"] could be passed to control memory usage of the ScalaTest runner. +

+

+ This is overridden by the jvm_flags attribute on individual targets. +

diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 96a96bae4..25620c327 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -133,6 +133,13 @@ def _expand_location(ctx, flags): def _join_path(args, sep = ","): return sep.join([f.path for f in args]) +# Return the first non-empty arg. If all are empty, return the last. +def _first_non_empty(*args): + for arg in args: + if arg: + return arg + return args[-1] + def compile_scala( ctx, target_label, @@ -296,10 +303,10 @@ StatsfileOutput: {statsfile_output} # scalac_jvm_flags passed in on the target override scalac_jvm_flags passed in on the # toolchain - if scalac_jvm_flags: - final_scalac_jvm_flags = _expand_location(ctx, scalac_jvm_flags) - else: - final_scalac_jvm_flags = ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scalac_jvm_flags + final_scalac_jvm_flags = _first_non_empty( + _expand_location(ctx, scalac_jvm_flags), + ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scalac_jvm_flags + ) ctx.actions.run( inputs = ins, @@ -1202,13 +1209,20 @@ def scala_test_impl(ctx): ]) coverage_runfiles = ctx.files._jacocorunner + ctx.files._lcov_merger + coverage_replacements.values() + # jvm_flags passed in on the target override scala_test_jvm_flags passed in on the + # toolchain + final_jvm_flags = _first_non_empty( + ctx.attr.jvm_flags, + ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scala_test_jvm_flags + ) + coverage_runfiles.extend(_write_executable( ctx = ctx, executable = executable, jvm_flags = [ "-DRULES_SCALA_MAIN_WS_NAME=%s" % ctx.workspace_name, "-DRULES_SCALA_ARGS_FILE=%s" % argsFile.short_path, - ] + ctx.attr.jvm_flags, + ] + final_jvm_flags, main_class = ctx.attr.main_class, rjars = rjars, use_jacoco = ctx.configuration.coverage_enabled, diff --git a/scala/scala_toolchain.bzl b/scala/scala_toolchain.bzl index 0faab9151..f57e23302 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -11,6 +11,7 @@ def _scala_toolchain_impl(ctx): plus_one_deps_mode = ctx.attr.plus_one_deps_mode, enable_code_coverage_aspect = ctx.attr.enable_code_coverage_aspect, scalac_jvm_flags = ctx.attr.scalac_jvm_flags, + scala_test_jvm_flags = ctx.attr.scala_test_jvm_flags, ) return [toolchain] @@ -35,5 +36,6 @@ scala_toolchain = rule( values = ["off", "on"], ), "scalac_jvm_flags": attr.string_list(), + "scala_test_jvm_flags": attr.string_list(), }, ) diff --git a/test_expect_failure/scala_test_jvm_flags/BUILD b/test_expect_failure/scala_test_jvm_flags/BUILD new file mode 100644 index 000000000..a777dfaf0 --- /dev/null +++ b/test_expect_failure/scala_test_jvm_flags/BUILD @@ -0,0 +1,43 @@ +load("//scala:scala_toolchain.bzl", "scala_toolchain") +load("//scala:scala.bzl", "scala_test") + +scala_toolchain( + name = "failing_toolchain_impl", + # This will fail because 1M isn't enough + scala_test_jvm_flags = ["-Xmx1M"], + visibility = ["//visibility:public"], +) + +scala_toolchain( + name = "passing_toolchain_impl", + # This will pass because 1G is enough + scala_test_jvm_flags = ["-Xmx1G"], + visibility = ["//visibility:public"], +) + +toolchain( + name = "failing_scala_toolchain", + toolchain = "failing_toolchain_impl", + toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type", + visibility = ["//visibility:public"], +) + +toolchain( + name = "passing_scala_toolchain", + toolchain = "passing_toolchain_impl", + toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type", + visibility = ["//visibility:public"], +) + +scala_test( + name = "empty_build", + srcs = ["Empty.scala"], +) + +scala_test( + name = "empty_overriding_build", + srcs = ["Empty.scala"], + # This overrides the option passed in on the toolchain, and should BUILD, even if + # the `failing_scala_toolchain` is used. + jvm_flags = ["-Xmx1G"], +) diff --git a/test_expect_failure/scala_test_jvm_flags/Empty.scala b/test_expect_failure/scala_test_jvm_flags/Empty.scala new file mode 100644 index 000000000..691dbdd9b --- /dev/null +++ b/test_expect_failure/scala_test_jvm_flags/Empty.scala @@ -0,0 +1,3 @@ +package test_expect_failure.scalac_jvm_opts + +class Empty \ No newline at end of file diff --git a/test_rules_scala.sh b/test_rules_scala.sh index f76542e91..7bdba39cd 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -837,6 +837,18 @@ test_scalac_jvm_flags_on_target_overrides_toolchain_passes() { bazel build --extra_toolchains="//test_expect_failure/scalac_jvm_opts:failing_scala_toolchain" //test_expect_failure/scalac_jvm_opts:empty_overriding_build } +test_scala_test_jvm_flags_from_scala_toolchain_fails() { + action_should_fail test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_build +} + +test_scala_test_jvm_flags_from_scala_toolchain_passes() { + bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:passing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_build +} + +test_scala_test_jvm_flags_on_target_overrides_toolchain_passes() { + bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_overriding_build +} + test_unused_dependency_checker_mode_set_in_rule() { action_should_fail build //test_expect_failure/unused_dependency_checker:failing_build } @@ -1118,4 +1130,7 @@ $runner test_coverage_on $runner scala_pb_library_targets_do_not_have_host_deps $runner test_scalac_jvm_flags_on_target_overrides_toolchain_passes $runner test_scalac_jvm_flags_from_scala_toolchain_passes -$runner test_scalac_jvm_flags_from_scala_toolchain_fails \ No newline at end of file +$runner test_scalac_jvm_flags_from_scala_toolchain_fails +$runner test_scala_test_jvm_flags_on_target_overrides_toolchain_passes +$runner test_scala_test_jvm_flags_from_scala_toolchain_passes +$runner test_scala_test_jvm_flags_from_scala_toolchain_fails \ No newline at end of file From 1c3b6ed06b25265c13e74110e42d7ee2e06a89a4 Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Mon, 5 Aug 2019 15:52:29 -0600 Subject: [PATCH 2/5] Fix package name --- test_expect_failure/scala_test_jvm_flags/Empty.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_expect_failure/scala_test_jvm_flags/Empty.scala b/test_expect_failure/scala_test_jvm_flags/Empty.scala index 691dbdd9b..7c09b6c5d 100644 --- a/test_expect_failure/scala_test_jvm_flags/Empty.scala +++ b/test_expect_failure/scala_test_jvm_flags/Empty.scala @@ -1,3 +1,3 @@ -package test_expect_failure.scalac_jvm_opts +package test_expect_failure.scala_test_jvm_flags class Empty \ No newline at end of file From 647989a2e17141c3d2432b6258f58b0acb283b76 Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Mon, 5 Aug 2019 15:57:48 -0600 Subject: [PATCH 3/5] Fix target names --- test_expect_failure/scala_test_jvm_flags/BUILD | 4 ++-- test_rules_scala.sh | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test_expect_failure/scala_test_jvm_flags/BUILD b/test_expect_failure/scala_test_jvm_flags/BUILD index a777dfaf0..3d13cca75 100644 --- a/test_expect_failure/scala_test_jvm_flags/BUILD +++ b/test_expect_failure/scala_test_jvm_flags/BUILD @@ -30,12 +30,12 @@ toolchain( ) scala_test( - name = "empty_build", + name = "empty_test", srcs = ["Empty.scala"], ) scala_test( - name = "empty_overriding_build", + name = "empty_overriding_test", srcs = ["Empty.scala"], # This overrides the option passed in on the toolchain, and should BUILD, even if # the `failing_scala_toolchain` is used. diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 7bdba39cd..9304097cd 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -838,15 +838,15 @@ test_scalac_jvm_flags_on_target_overrides_toolchain_passes() { } test_scala_test_jvm_flags_from_scala_toolchain_fails() { - action_should_fail test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_build + action_should_fail test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_test } test_scala_test_jvm_flags_from_scala_toolchain_passes() { - bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:passing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_build + bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:passing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_test } test_scala_test_jvm_flags_on_target_overrides_toolchain_passes() { - bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_overriding_build + bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_overriding_test } test_unused_dependency_checker_mode_set_in_rule() { From 214de3c08f54452ad64e1f97ca092e789b05c79e Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Mon, 5 Aug 2019 16:52:00 -0600 Subject: [PATCH 4/5] Add trivial test suite and rename some things --- test_expect_failure/scala_test_jvm_flags/BUILD | 4 ++-- test_expect_failure/scala_test_jvm_flags/Empty.scala | 3 --- test_expect_failure/scala_test_jvm_flags/EmptyTest.scala | 9 +++++++++ 3 files changed, 11 insertions(+), 5 deletions(-) delete mode 100644 test_expect_failure/scala_test_jvm_flags/Empty.scala create mode 100644 test_expect_failure/scala_test_jvm_flags/EmptyTest.scala diff --git a/test_expect_failure/scala_test_jvm_flags/BUILD b/test_expect_failure/scala_test_jvm_flags/BUILD index 3d13cca75..f2bb261c2 100644 --- a/test_expect_failure/scala_test_jvm_flags/BUILD +++ b/test_expect_failure/scala_test_jvm_flags/BUILD @@ -31,12 +31,12 @@ toolchain( scala_test( name = "empty_test", - srcs = ["Empty.scala"], + srcs = ["EmptyTest.scala"], ) scala_test( name = "empty_overriding_test", - srcs = ["Empty.scala"], + srcs = ["EmptyTest.scala"], # This overrides the option passed in on the toolchain, and should BUILD, even if # the `failing_scala_toolchain` is used. jvm_flags = ["-Xmx1G"], diff --git a/test_expect_failure/scala_test_jvm_flags/Empty.scala b/test_expect_failure/scala_test_jvm_flags/Empty.scala deleted file mode 100644 index 7c09b6c5d..000000000 --- a/test_expect_failure/scala_test_jvm_flags/Empty.scala +++ /dev/null @@ -1,3 +0,0 @@ -package test_expect_failure.scala_test_jvm_flags - -class Empty \ No newline at end of file diff --git a/test_expect_failure/scala_test_jvm_flags/EmptyTest.scala b/test_expect_failure/scala_test_jvm_flags/EmptyTest.scala new file mode 100644 index 000000000..d1fbfc7a0 --- /dev/null +++ b/test_expect_failure/scala_test_jvm_flags/EmptyTest.scala @@ -0,0 +1,9 @@ +package test_expect_failure.scala_test_jvm_flags + +import org.scalatest.FunSuite + +class EmptyTest extends FunSuite { + test("empty test") { + assert(true) + } +} \ No newline at end of file From 863a394c0f5f5f9fcea5594c4b09a8b86a1a3ff5 Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Tue, 6 Aug 2019 09:40:27 -0600 Subject: [PATCH 5/5] Wrap all jvm_flags in _expand_location --- scala/private/rule_impls.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 25620c327..c92542482 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -304,7 +304,7 @@ StatsfileOutput: {statsfile_output} # scalac_jvm_flags passed in on the target override scalac_jvm_flags passed in on the # toolchain final_scalac_jvm_flags = _first_non_empty( - _expand_location(ctx, scalac_jvm_flags), + scalac_jvm_flags, ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scalac_jvm_flags ) @@ -326,7 +326,7 @@ StatsfileOutput: {statsfile_output} # consume the flags on startup. arguments = [ "--jvm_flag=%s" % f - for f in final_scalac_jvm_flags + for f in _expand_location(ctx, final_scalac_jvm_flags) ] + ["@" + argfile.path], ) @@ -1222,7 +1222,7 @@ def scala_test_impl(ctx): jvm_flags = [ "-DRULES_SCALA_MAIN_WS_NAME=%s" % ctx.workspace_name, "-DRULES_SCALA_ARGS_FILE=%s" % argsFile.short_path, - ] + final_jvm_flags, + ] + _expand_location(ctx, final_jvm_flags), main_class = ctx.attr.main_class, rjars = rjars, use_jacoco = ctx.configuration.coverage_enabled,