Skip to content

Add scalac_jvm_flags option to toolchain #803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 5, 2019
Merged

Add scalac_jvm_flags option to toolchain #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a4e3a47
Add scalac_jvm_flags to scala_toolchain
beala-stripe Aug 2, 2019
6805122
Add docs
beala-stripe Aug 2, 2019
60c1ecc
Fix comment
beala-stripe Aug 2, 2019
c2a7783
Add enable_code_coverage_aspect to the docs
beala-stripe Aug 2, 2019
4950f3e
Flags on target should override flags on toolchain.
beala-stripe Aug 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 54 additions & 2 deletions docs/scala_toolchain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`:
Expand Down Expand Up @@ -46,3 +44,57 @@ scala_register_toolchains()
# WORKSPACE
register_toolchains("//toolchains:my_scala_toolchain")
```

<table class="table table-condensed table-bordered table-params">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can do a table in github flavored markdown https://help.github.com/en/articles/organizing-information-with-tables

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried converting this to MD, but it's not clear I got any readability wins. In order to insert newlines into cells, I needed to mix MD and HTML. I also cannot indent the HTML properly because a newline starts a new cell.

<colgroup>
<col class="col-param" />
<col class="param-description" />
</colgroup>
<thead>
<tr>
<th colspan="2">Attributes</th>
</tr>
</thead>
<tbody>
<tr>
<td><code>scalacopts</code></td>
<td>
<p><code>List of strings; optional</code></p>
<p>
Extra compiler options for this binary to be passed to scalac.
</p>
<p>
This is overridden by the `scalac_jvm_flags` attribute on individual targets.
</p>
</td>
</tr>
<tr>
<td><code>scalac_jvm_flags</code></td>
<td>
<p><code>List of strings; optional</code></p>
<p>
List of JVM flags to be passed to scalac. For example <code>["-Xmx5G"]</code> could be passed to control memory usage of Scalac.
</p>
</td>
</tr>
<tr>
<td><code>unused_dependency_checker_mode</code></td>
<td>
<p><code>String; optional</code></p>
<p>
Enable unused dependency checking (see <a href="https://github.com/bazelbuild/rules_scala#experimental-unused-dependency-checking">Unused dependency checking</a>).
Possible values are: <code>off</code>, <code>warn</code> and <code>error</code>.
</p>
</td>
</tr>
<tr>
<td><code>enable_code_coverage_aspect</code></td>
<td>
<p><code>"on" or "off"; optional; defaults to "off"</code></p>
<p>
This enables instrumenting tests with jacoco code coverage.
</p>
</td>
</tr>
</tbody>
</table>
9 changes: 8 additions & 1 deletion scala/private/rule_impls.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
for f in final_scalac_jvm_flags
] + ["@" + argfile.path],
)

Expand Down
4 changes: 3 additions & 1 deletion scala/scala_toolchain.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand All @@ -32,6 +33,7 @@ scala_toolchain = rule(
"enable_code_coverage_aspect": attr.string(
default = "off",
values = ["off", "on"],
)
),
"scalac_jvm_flags": attr.string_list(),
},
)
43 changes: 43 additions & 0 deletions test_expect_failure/scalac_jvm_opts/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
load("//scala:scala_toolchain.bzl", "scala_toolchain")
load("//scala:scala.bzl", "scala_library")

scala_toolchain(
name = "failing_toolchain_impl",
# This will fail because 1M isn't enough
scalac_jvm_flags = ["-Xmx1M"],
visibility = ["//visibility:public"],
)

scala_toolchain(
name = "passing_toolchain_impl",
# This will pass because 1G is enough
scalac_jvm_flags = ["-Xmx1G"],
Copy link
Contributor

@ptarjan ptarjan Aug 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/1/5 ? (or change the previous line)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

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"],
)

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"],
)
3 changes: 3 additions & 0 deletions test_expect_failure/scalac_jvm_opts/Empty.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package test_expect_failure.scalac_jvm_opts

class Empty
15 changes: 15 additions & 0 deletions test_rules_scala.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,18 @@ 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_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
}
Expand Down Expand Up @@ -1104,3 +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