-
-
Notifications
You must be signed in to change notification settings - Fork 288
Add support for add_exports/add_opens #1551
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.
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
Add support for add_exports/add_opens #1551
Conversation
5fa31f2 to
98f2e00
Compare
98f2e00 to
7b1d93d
Compare
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.
- I'm currently using
bazel_featuresto detect Bazel version for support foradd_opens/add_exports. - The project currently recommends that folks run
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.bzlalso loads Bazel rules likescala_library, which requires@bazel_features. - For
load("@bazel_features//...")to work, the WORKSPACE file needs to runbazel_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:
-
Bite the bullet and force every user to install
bazel_features. (That's what this PR would do, in its current shape.) -
Abandon support for Bazel 6.x. (Bazel 7.0.0+ has proper support for this feature.)
-
Change the
.bzlfile 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, liftrules_scala_setupandrules_scala_toolchain_deps_repositoriesout 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 callbazel_features_deps()for them.
I think option 2 seems most straightforward, though I'll leave it up to the maintainers to decide.
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 bazelbuild/bazel#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_infoand include those--add-exportsand--add-opensJVM flags in the launcher script.add_exportsandadd_opensattributes to all rules (includingscala_import) and propagate the flags throughJavaInfo.JavaInfoconstructor only added this functionality in 7.0.0; see User-accessible JavaInfo constructor doesn't allow changing module_flags_info bazelbuild/bazel#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_binaryrule. This PR adds the same functionality to Scala rules.Fixes #1523