Skip to content

Commit

Permalink
Add add_exports & add_opens parameters to JavaInfo constructor
Browse files Browse the repository at this point in the history
There is currently no non-hacky way for third-party rule implementations to add `add_exports` and `add_opens` to a `JavaInfo`, though hacky ways exist (like using a macro to generate a `java_library` with `add_exports` / `add_opens` and add it as a dependency).

Having an official way to create `JavaInfo`s with `add_exports` and `add_opens` helps third-party JVM rules better support JDK 9+ (and especially 17+, which requires `--add-opens` flags to access JDK internals through reflection).

Addresses half of #20033.

Closes #20036.

PiperOrigin-RevId: 580472097
Change-Id: I159e3410c5480ac683fd9af85bfd1d83ac0e6d8a
  • Loading branch information
timothyg-stripe authored and iancha1992 committed Nov 17, 2023
1 parent 2d61c9e commit f921577
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 9 deletions.
17 changes: 12 additions & 5 deletions src/main/starlark/builtins_bzl/common/java/java_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Definition of JavaInfo and JavaPluginInfo provider.

load(":common/cc/cc_common.bzl", "CcNativeLibraryInfo", "cc_common")
load(":common/cc/cc_info.bzl", "CcInfo")
load(":common/java/java_semantics.bzl", "semantics")

# TODO(hvd): remove this when:
# - we have a general provider-type checking API
Expand Down Expand Up @@ -456,8 +457,7 @@ def java_info_for_compilation(
# only differs from the usual java_info.transitive_source_jars in the order of deps
transitive = [dep.transitive_source_jars for dep in concatenated_deps.runtimedeps_exports_deps],
),
# the JavaInfo constructor does not add flags from runtime_deps nor support
# adding this target's exports/opens
# the JavaInfo constructor does not add flags from runtime_deps
module_flags_info = _create_module_flags_info(
add_exports = depset(add_exports, transitive = [
dep.module_flags_info.add_exports
Expand Down Expand Up @@ -663,7 +663,9 @@ def _javainfo_init(
exports = [],
exported_plugins = [],
jdeps = None,
native_libraries = []):
native_libraries = [],
add_exports = [],
add_opens = []):
"""The JavaInfo constructor
Args:
Expand Down Expand Up @@ -693,10 +695,15 @@ def _javainfo_init(
is typically produced by a compiler. IDEs and other tools can use this information for
more efficient processing. Optional.
native_libraries: ([CcInfo]) Native library dependencies that are needed for this library.
add_exports: ([str]) The <module>/<package>s this library was given access to.
add_opens: ([str]) The <module>/<package>s this library was given reflective access to.
Returns:
(dict) arguments to the JavaInfo provider constructor
"""
if add_exports or add_opens:
semantics.check_java_info_opens_exports()

result, concatenated_deps = _javainfo_init_base(
output_jar,
compile_jar,
Expand Down Expand Up @@ -734,11 +741,11 @@ def _javainfo_init(
],
),
module_flags_info = _create_module_flags_info(
add_exports = depset(transitive = [
add_exports = depset(add_exports, transitive = [
dep.module_flags_info.add_exports
for dep in concatenated_deps.deps_exports
]),
add_opens = depset(transitive = [
add_opens = depset(add_opens, transitive = [
dep.module_flags_info.add_opens
for dep in concatenated_deps.deps_exports
]),
Expand Down
4 changes: 4 additions & 0 deletions src/main/starlark/builtins_bzl/common/java/java_semantics.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ def _get_default_resource_path(path, segment_extractor):
def _compatible_javac_options(*_args):
return depset()

def _check_java_info_opens_exports():
pass

semantics = struct(
JAVA_TOOLCHAIN_LABEL = "@bazel_tools//tools/jdk:current_java_toolchain",
JAVA_TOOLCHAIN_TYPE = "@bazel_tools//tools/jdk:toolchain_type",
Expand Down Expand Up @@ -76,4 +79,5 @@ semantics = struct(
JAVA_PROTO_TOOLCHAIN = "@rules_java//java/proto:toolchain_type",
JAVA_LITE_PROTO_TOOLCHAIN = "@rules_java//java/proto:lite_toolchain_type",
PROGUARD_ALLOWLISTER_LABEL = "@bazel_tools//tools/jdk:proguard_whitelister",
check_java_info_opens_exports = _check_java_info_opens_exports,
)
Original file line number Diff line number Diff line change
Expand Up @@ -857,20 +857,53 @@ public void buildHelperCreateJavaInfoWithModuleFlags() throws Exception {
"java_library(",
" name = 'my_java_lib_direct',",
" srcs = ['java/A.java'],",
" add_exports = ['java.base/java.lang'],",
" add_opens = ['java.base/java.lang'],",
")",
"java_library(",
" name = 'my_java_lib_runtime',",
" srcs = ['java/A.java'],",
" add_opens = ['java.base/java.util'],",
")",
"java_library(",
" name = 'my_java_lib_exports',",
" srcs = ['java/A.java'],",
" add_opens = ['java.base/java.math'],",
")",
"my_rule(",
" name = 'my_starlark_rule',",
" dep = [':my_java_lib_direct'],",
" dep_runtime = [':my_java_lib_runtime'],",
" dep_exports = [':my_java_lib_exports'],",
" output_jar = 'my_starlark_rule_lib.jar',",
" add_exports = ['java.base/java.lang.invoke'],",
")");
assertNoEvents();

JavaModuleFlagsProvider ruleOutputs =
fetchJavaInfo().getProvider(JavaModuleFlagsProvider.class);

assertThat(ruleOutputs.toFlags())
.containsExactly("--add-opens=java.base/java.lang=ALL-UNNAMED");
if (analysisMock.isThisBazel()) {
assertThat(ruleOutputs.toFlags())
.containsExactly(
"--add-exports=java.base/java.lang=ALL-UNNAMED",
"--add-exports=java.base/java.lang.invoke=ALL-UNNAMED",
// NB: no java.base/java.util as the JavaInfo constructor doesn't
// look at runtime_deps for module flags.
"--add-opens=java.base/java.lang=ALL-UNNAMED",
"--add-opens=java.base/java.math=ALL-UNNAMED")
.inOrder();
} else {
// add_exports/add_opens ignored in JavaInfo constructor in #newJavaInfo below
assertThat(ruleOutputs.toFlags())
.containsExactly(
"--add-exports=java.base/java.lang=ALL-UNNAMED",
// NB: no java.base/java.util as the JavaInfo constructor doesn't
// look at runtime_deps for module flags.
"--add-opens=java.base/java.lang=ALL-UNNAMED",
"--add-opens=java.base/java.math=ALL-UNNAMED")
.inOrder();
}
}

@Test
Expand Down Expand Up @@ -1239,8 +1272,14 @@ private String[] newJavaInfo() {
" generated_source_jar = ctx.file.generated_source_jar,",
" native_headers_jar = ctx.file.native_headers_jar,",
" manifest_proto = ctx.file.manifest_proto,",
" native_libraries = dp_libs,",
" )",
" native_libraries = dp_libs,");
if (analysisMock.isThisBazel()) {
lines.add(
" add_exports = ctx.attr.add_exports,", //
" add_opens = ctx.attr.add_opens,");
}
lines.add(
" )", //
" return [result(property = javaInfo)]");
return lines.build().toArray(new String[] {});
}
Expand Down Expand Up @@ -1271,6 +1310,8 @@ private void build() throws Exception {
" 'generated_source_jar' : attr.label(allow_single_file=True),",
" 'native_headers_jar' : attr.label(allow_single_file=True),",
" 'manifest_proto' : attr.label(allow_single_file=True),",
" 'add_exports' : attr.string_list(),",
" 'add_opens' : attr.string_list(),",
useIJar || stampJar || sourceFiles
? " '_toolchain': attr.label(default = Label('//java/com/google/test:toolchain')),"
: "",
Expand Down

0 comments on commit f921577

Please sign in to comment.