Open timothyg-stripe opened 4 months ago
I managed to get a passing build with this PR, but it's a bit unfortunate since it would force users to add some additional lines to their WORKSPACE file.
bazel_features
to detect Bazel version for support for add_opens
/add_exports
.load("@io_bazel_rules_scala//scala:scala.bzl", "rules_scala_setup", "rules_scala_toolchain_deps_repositories")
in WORKSPACE. But loading @io_bazel_rules_scala//scala:scala.bzl
also loads Bazel rules like scala_library
, which requires @bazel_features
.load("@bazel_features//...")
to work, the WORKSPACE file needs to run bazel_features_deps()
before the attempted load.The sum effect of this is that users must have
http_archive(
name = "bazel_features",
sha256 = "d7787da289a7fb497352211ad200ec9f698822a9e0757a4976fd9f713ff372b3",
strip_prefix = "bazel_features-1.9.1",
url = "https://github.com/bazel-contrib/bazel_features/releases/download/v1.9.1/bazel_features-v1.9.1.tar.gz",
)
load("@bazel_features//:deps.bzl", "bazel_features_deps")
bazel_features_deps()
in the WORKSPACE file before loading anything from rules_scala
– and specifically, we cannot call bazel_features_deps()
for them in rules_scala_toolchain_deps_repositories()
.
I think there are three options:
bazel_features
. (That's what this PR would do, in its current shape.)Change the .bzl
file structure so that files that should be loaded from BUILD files are separate from those that should be loaded from WORKSPACE files. In other words, lift rules_scala_setup
and rules_scala_toolchain_deps_repositories
out of @io_bazel_rules_scala//scala:scala.bzl
, and into something like @io_bazel_rules_scala//scala:workspace.bzl
.
This way, users still have to load bazel_features
– but at least we can call bazel_features_deps()
for them.
I think option 2 seems most straightforward, though I'll leave it up to the maintainers to decide.
@liucijus @simuons
Hi, @timothyg-stripe, so this feature can be enabled only for bazel 7 and up. Could this be done with skylib versions?
@simuons unfortunately, skylib versions isn't sufficient since versions.get
does not work in a BUILD file. See https://github.com/bazelbuild/bazel/issues/4566.
I'm happy to write some tests for this, but existing tests appear to be broken in CI, and much of the functionality requires Bazel 7.0.0 to work anyway (while CI is still on Bazel 6.3).
Description
scala_binary
,scala_test
, and friends, readjava_info.module_flags_info
and include those--add-exports
and--add-opens
JVM flags in the launcher script.add_exports
andadd_opens
attributes to all rules (includingscala_import
) and propagate the flags throughJavaInfo
.JavaInfo
constructor only added this functionality in 7.0.0; see https://github.com/bazelbuild/bazel/issues/20033.Motivation
JDK 17 strongly encapsulates JDK internals, so many libraries need
--add-exports=
and--add-opens=
JVM flags to run. Bazel's Java rules already support adding those flags throughjava_library(add_opens = [...], add_exports = [...])
, and these fields are read in Bazel'sjava_binary
rule. This PR adds the same functionality to Scala rules.Fixes #1523