From a4e3a47005e19c8c0ea374684df9e5e898bdcb98 Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Fri, 2 Aug 2019 17:21:55 -0600 Subject: [PATCH 1/5] Add scalac_jvm_flags to scala_toolchain This allows things like setting the max heap on all Scalac workers. --- scala/private/rule_impls.bzl | 2 +- scala/scala_toolchain.bzl | 4 ++- test_expect_failure/scalac_jvm_opts/BUILD | 35 +++++++++++++++++++ .../scalac_jvm_opts/Empty.scala | 3 ++ test_rules_scala.sh | 10 ++++++ 5 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 test_expect_failure/scalac_jvm_opts/BUILD create mode 100644 test_expect_failure/scalac_jvm_opts/Empty.scala diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 8a40f55fc..6744c94eb 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -312,7 +312,7 @@ StatsfileOutput: {statsfile_output} # consume the flags on startup. arguments = [ "--jvm_flag=%s" % f - for f in _expand_location(ctx, scalac_jvm_flags) + for f in _expand_location(ctx, scalac_jvm_flags) + ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scalac_jvm_flags ] + ["@" + argfile.path], ) diff --git a/scala/scala_toolchain.bzl b/scala/scala_toolchain.bzl index 157eae715..0faab9151 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -10,6 +10,7 @@ def _scala_toolchain_impl(ctx): unused_dependency_checker_mode = ctx.attr.unused_dependency_checker_mode, 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, ) return [toolchain] @@ -32,6 +33,7 @@ scala_toolchain = rule( "enable_code_coverage_aspect": attr.string( default = "off", values = ["off", "on"], - ) + ), + "scalac_jvm_flags": attr.string_list(), }, ) diff --git a/test_expect_failure/scalac_jvm_opts/BUILD b/test_expect_failure/scalac_jvm_opts/BUILD new file mode 100644 index 000000000..672119d65 --- /dev/null +++ b/test_expect_failure/scalac_jvm_opts/BUILD @@ -0,0 +1,35 @@ +load("//scala:scala_toolchain.bzl", "scala_toolchain") +load("//scala:scala.bzl", "scala_library") + +scala_toolchain( + name = "failing_toolchain_impl", + # This will fail because 1 meg isn't enough + scalac_jvm_flags = ["-Xmx1M"], + visibility = ["//visibility:public"], +) + +scala_toolchain( + name = "passing_toolchain_impl", + # This will fail because 1 meg isn't enough + scalac_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_library( + name = "empty_build", + srcs = ["Empty.scala"], +) diff --git a/test_expect_failure/scalac_jvm_opts/Empty.scala b/test_expect_failure/scalac_jvm_opts/Empty.scala new file mode 100644 index 000000000..691dbdd9b --- /dev/null +++ b/test_expect_failure/scalac_jvm_opts/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 ff65051bb..01c9f46f6 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -825,6 +825,14 @@ test_scalaopts_from_scala_toolchain() { action_should_fail build --extra_toolchains="//test_expect_failure/scalacopts_from_toolchain:failing_scala_toolchain" //test_expect_failure/scalacopts_from_toolchain:failing_build } +test_scalac_jvm_flags_from_scala_toolchain_fails() { + action_should_fail build --extra_toolchains="//test_expect_failure/scalac_jvm_opts:failing_scala_toolchain" //test_expect_failure/scalac_jvm_opts:empty_build +} + +test_scalac_jvm_flags_from_scala_toolchain_passes() { + bazel build --extra_toolchains="//test_expect_failure/scalac_jvm_opts:passing_scala_toolchain" //test_expect_failure/scalac_jvm_opts:empty_build +} + test_unused_dependency_checker_mode_set_in_rule() { action_should_fail build //test_expect_failure/unused_dependency_checker:failing_build } @@ -1104,3 +1112,5 @@ $runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one $runner test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps $runner test_coverage_on $runner scala_pb_library_targets_do_not_have_host_deps +$runner test_scalac_jvm_flags_from_scala_toolchain_passes +$runner test_scalac_jvm_flags_from_scala_toolchain_fails From 68051228399e2ff53e3f81979e81961614cff18b Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Fri, 2 Aug 2019 17:29:14 -0600 Subject: [PATCH 2/5] Add docs --- docs/scala_toolchain.md | 44 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/docs/scala_toolchain.md b/docs/scala_toolchain.md index bb9796d43..22e656b50 100644 --- a/docs/scala_toolchain.md +++ b/docs/scala_toolchain.md @@ -2,8 +2,6 @@ `scala_toolchain` allows you to define global configuration to all Scala targets. -Currently, the only option that can be set is `scalacopts` but the plan is to expand it to other options as well. - **Some scala_toolchain must be registered!** ### Several options to configure `scala_toolchain`: @@ -46,3 +44,45 @@ scala_register_toolchains() # WORKSPACE register_toolchains("//toolchains:my_scala_toolchain") ``` + + + + + + + + + + + + + + + + + + + + + + + + + +
Attributes
scalacopts +

List of strings; optional

+

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

+
scalac_jvm_flags +

List of strings; optional

+

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

+
unused_dependency_checker_mode +

String; optional

+

+ Enable unused dependency checking (see Unused dependency checking). + Possible values are: off, warn and error. +

+
\ No newline at end of file From 60c1ecc30cc2fc7a861ba5897e4bfa3cfb0fa0c1 Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Fri, 2 Aug 2019 17:30:37 -0600 Subject: [PATCH 3/5] Fix comment --- test_expect_failure/scalac_jvm_opts/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_expect_failure/scalac_jvm_opts/BUILD b/test_expect_failure/scalac_jvm_opts/BUILD index 672119d65..74749a760 100644 --- a/test_expect_failure/scalac_jvm_opts/BUILD +++ b/test_expect_failure/scalac_jvm_opts/BUILD @@ -3,14 +3,14 @@ load("//scala:scala.bzl", "scala_library") scala_toolchain( name = "failing_toolchain_impl", - # This will fail because 1 meg isn't enough + # This will fail because 1M isn't enough scalac_jvm_flags = ["-Xmx1M"], visibility = ["//visibility:public"], ) scala_toolchain( name = "passing_toolchain_impl", - # This will fail because 1 meg isn't enough + # This will pass because 5G is enough scalac_jvm_flags = ["-Xmx1G"], visibility = ["//visibility:public"], ) From c2a778309685ac310b7d4060bcef1080ec331d13 Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Fri, 2 Aug 2019 17:38:40 -0600 Subject: [PATCH 4/5] Add enable_code_coverage_aspect to the docs --- docs/scala_toolchain.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/scala_toolchain.md b/docs/scala_toolchain.md index 22e656b50..ba135ef21 100644 --- a/docs/scala_toolchain.md +++ b/docs/scala_toolchain.md @@ -84,5 +84,14 @@ scala_register_toolchains()

+ + enable_code_coverage_aspect + +

"on" or "off"; optional; defaults to "off"

+

+ This enables instrumenting tests with jacoco code coverage. +

+ + \ No newline at end of file From 4950f3e8bf07a10d3f69954e1a739c574b80daef Mon Sep 17 00:00:00 2001 From: Alex Beal Date: Mon, 5 Aug 2019 11:59:03 -0600 Subject: [PATCH 5/5] Flags on target should override flags on toolchain. Also fix comment. --- docs/scala_toolchain.md | 3 +++ scala/private/rule_impls.bzl | 9 ++++++++- test_expect_failure/scalac_jvm_opts/BUILD | 10 +++++++++- test_rules_scala.sh | 7 ++++++- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/scala_toolchain.md b/docs/scala_toolchain.md index ba135ef21..d0e2d75b3 100644 --- a/docs/scala_toolchain.md +++ b/docs/scala_toolchain.md @@ -63,6 +63,9 @@ 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. +

diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 6744c94eb..96a96bae4 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -294,6 +294,13 @@ StatsfileOutput: {statsfile_output} resource_jars + [manifest, argfile] + scalac_inputs ) + # 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 + ctx.actions.run( inputs = ins, outputs = outs, @@ -312,7 +319,7 @@ StatsfileOutput: {statsfile_output} # consume the flags on startup. arguments = [ "--jvm_flag=%s" % f - for f in _expand_location(ctx, scalac_jvm_flags) + ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scalac_jvm_flags + for f in final_scalac_jvm_flags ] + ["@" + argfile.path], ) diff --git a/test_expect_failure/scalac_jvm_opts/BUILD b/test_expect_failure/scalac_jvm_opts/BUILD index 74749a760..1b7772ec0 100644 --- a/test_expect_failure/scalac_jvm_opts/BUILD +++ b/test_expect_failure/scalac_jvm_opts/BUILD @@ -10,7 +10,7 @@ scala_toolchain( scala_toolchain( name = "passing_toolchain_impl", - # This will pass because 5G is enough + # This will pass because 1G is enough scalac_jvm_flags = ["-Xmx1G"], visibility = ["//visibility:public"], ) @@ -33,3 +33,11 @@ scala_library( name = "empty_build", srcs = ["Empty.scala"], ) + +scala_library( + 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. + scalac_jvm_flags = ["-Xmx1G"], +) diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 01c9f46f6..f76542e91 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -833,6 +833,10 @@ test_scalac_jvm_flags_from_scala_toolchain_passes() { bazel build --extra_toolchains="//test_expect_failure/scalac_jvm_opts:passing_scala_toolchain" //test_expect_failure/scalac_jvm_opts:empty_build } +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_unused_dependency_checker_mode_set_in_rule() { action_should_fail build //test_expect_failure/unused_dependency_checker:failing_build } @@ -1112,5 +1116,6 @@ $runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one $runner test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps $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 +$runner test_scalac_jvm_flags_from_scala_toolchain_fails \ No newline at end of file