From 77eacce3f56ce6b5b466337c123a5cdfd671f2ee Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Wed, 15 Nov 2023 03:59:34 -0800 Subject: [PATCH] Add add_exports & add_opens attributes to java_import This reduces the difference between `java_import` and other Java rules, like `java_library` and `java_binary`. * Propagate `add_exports` and `add_opens` through `java_import` * Add tests Fixes #19556. Closes #20035. PiperOrigin-RevId: 582617247 Change-Id: I513536acd4994de36190a5d79c2e92ac3f3ccc66 --- .../builtins_bzl/common/java/java_import.bzl | 26 ++++++++---- .../google/devtools/build/lib/view/java/BUILD | 1 + .../java/JavaImportConfiguredTargetTest.java | 42 +++++++++++++++++++ 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/java/java_import.bzl b/src/main/starlark/builtins_bzl/common/java/java_import.bzl index 5a2ad06d6f068c..011107d39eb4c2 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_import.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_import.bzl @@ -16,14 +16,14 @@ Definition of java_import rule. """ +load(":common/cc/cc_info.bzl", "CcInfo") load(":common/java/basic_java_library.bzl", "construct_defaultinfo") -load(":common/java/java_semantics.bzl", "semantics") -load(":common/java/proguard_validation.bzl", "validate_proguard_specs") load(":common/java/import_deps_check.bzl", "import_deps_check") -load(":common/cc/cc_info.bzl", "CcInfo") -load(":common/java/java_info.bzl", "JavaInfo") load(":common/java/java_common.bzl", "java_common") load(":common/java/java_common_internal_for_builtins.bzl", _run_ijar_private_for_builtins = "run_ijar") +load(":common/java/java_info.bzl", "JavaInfo") +load(":common/java/java_semantics.bzl", "semantics") +load(":common/java/proguard_validation.bzl", "validate_proguard_specs") PackageSpecificationInfo = _builtins.toplevel.PackageSpecificationInfo @@ -78,7 +78,7 @@ def _check_empty_jars_error(ctx, jars): if len(jars) == 0 and disallow_java_import_empty_jars and not_in_allowlist: fail("empty java_import.jars is no longer supported " + ctx.label.package) -def _create_java_info_with_dummy_output_file(ctx, srcjar, all_deps, exports, runtime_deps_list, neverlink, cc_info_list): +def _create_java_info_with_dummy_output_file(ctx, srcjar, all_deps, exports, runtime_deps_list, neverlink, cc_info_list, add_exports, add_opens): dummy_jar = ctx.actions.declare_file(ctx.label.name + "_dummy.jar") dummy_src_jar = srcjar if dummy_src_jar == None: @@ -94,6 +94,8 @@ def _create_java_info_with_dummy_output_file(ctx, srcjar, all_deps, exports, run neverlink = neverlink, exports = [export[JavaInfo] for export in exports if JavaInfo in export], # Watchout, maybe you need to add them there manually. native_libraries = cc_info_list, + add_exports = add_exports, + add_opens = add_opens, ) def bazel_java_import_rule( @@ -104,7 +106,9 @@ def bazel_java_import_rule( runtime_deps = [], exports = [], neverlink = False, - proguard_specs = []): + proguard_specs = [], + add_exports = [], + add_opens = []): """Implements java_import. This rule allows the use of precompiled .jar files as libraries in other Java rules. @@ -119,6 +123,8 @@ def bazel_java_import_rule( neverlink: (bool) Whether this rule should only be used for compilation and not at runtime. constraints: (list[String]) Rule constraints. proguard_specs: (list[File]) Files to be used as Proguard specification. + add_exports: (list[str]) Allow this library to access the given /. + add_opens: (list[str]) Allow this library to reflectively access the given /. Returns: (list[provider]) A list containing DefaultInfo, JavaInfo, @@ -159,11 +165,13 @@ def bazel_java_import_rule( source_jar = srcjar, exports = [export[JavaInfo] for export in exports if JavaInfo in export], # Watchout, maybe you need to add them there manually. native_libraries = cc_info_list, + add_exports = add_exports, + add_opens = add_opens, )) java_info = java_common.merge(java_infos) else: # TODO(kotlaja): Remove next line once all java_import targets with empty jars attribute are cleaned from depot (b/246559727). - java_info = _create_java_info_with_dummy_output_file(ctx, srcjar, all_deps, exports, runtime_deps_list, neverlink, cc_info_list) + java_info = _create_java_info_with_dummy_output_file(ctx, srcjar, all_deps, exports, runtime_deps_list, neverlink, cc_info_list, add_exports, add_opens) target = {"JavaInfo": java_info} @@ -207,6 +215,8 @@ def _proxy(ctx): ctx.attr.exports, ctx.attr.neverlink, ctx.files.proguard_specs, + ctx.attr.add_exports, + ctx.attr.add_opens, ).values() _ALLOWED_RULES_IN_DEPS_FOR_JAVA_IMPORT = [ @@ -253,6 +263,8 @@ JAVA_IMPORT_ATTRS = { allow_files = True, ), # Additional attrs + "add_exports": attr.string_list(), + "add_opens": attr.string_list(), "licenses": attr.license() if hasattr(attr, "license") else attr.string_list(), "_java_toolchain_type": attr.label(default = semantics.JAVA_TOOLCHAIN_TYPE), } diff --git a/src/test/java/com/google/devtools/build/lib/view/java/BUILD b/src/test/java/com/google/devtools/build/lib/view/java/BUILD index 529965dee744cf..cc4663c91c0a08 100644 --- a/src/test/java/com/google/devtools/build/lib/view/java/BUILD +++ b/src/test/java/com/google/devtools/build/lib/view/java/BUILD @@ -79,6 +79,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/rules/java:java-compilation", "//src/main/java/com/google/devtools/build/lib/rules/java:java-rules", + "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java", "//src/test/java/com/google/devtools/build/lib/actions/util", "//src/test/java/com/google/devtools/build/lib/analysis/util", "//src/test/java/com/google/devtools/build/lib/rules/java:java_compile_action_test_helper", diff --git a/src/test/java/com/google/devtools/build/lib/view/java/JavaImportConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/view/java/JavaImportConfiguredTargetTest.java index 1ee6e20c389161..4c61412c221e75 100644 --- a/src/test/java/com/google/devtools/build/lib/view/java/JavaImportConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/view/java/JavaImportConfiguredTargetTest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; import com.google.devtools.build.lib.rules.java.ProguardSpecProvider; +import com.google.devtools.build.lib.starlarkbuildapi.java.JavaModuleFlagsProviderApi; import com.google.devtools.build.lib.testutil.MoreAsserts; import java.util.Arrays; import java.util.List; @@ -220,6 +221,47 @@ public void testDeps() throws Exception { "java/jarlib2/depjar.jar"); } + @Test + public void testModuleFlags() throws Exception { + if (!analysisMock.isThisBazel()) { + return; + } + + scratch.file( + "java/jarlib2/BUILD", + "java_library(name = 'lib',", + " srcs = ['Main.java'],", + " deps = [':import-jar'])", + "java_import(name = 'import-jar',", + " jars = ['import.jar'],", + " deps = ['//java/jarlib2:depjar'],", + " exports = ['//java/jarlib2:exportjar'],", + ")", + "java_import(name = 'depjar',", + " jars = ['depjar.jar'],", + " add_exports = ['java.base/java.lang'])", + "java_import(name = 'exportjar',", + " jars = ['exportjar.jar'],", + " add_opens = ['java.base/java.util'])"); + + ConfiguredTarget importJar = getConfiguredTarget("//java/jarlib2:import-jar"); + JavaModuleFlagsProviderApi moduleFlagsProvider = + JavaInfo.getJavaInfo(importJar).getJavaModuleFlagsInfo(); + assertThat(moduleFlagsProvider.getAddExports().toList(String.class)) + .containsExactly("java.base/java.lang"); + assertThat(moduleFlagsProvider.getAddOpens().toList(String.class)) + .containsExactly("java.base/java.util"); + + // Check that module flags propagate to Java libraries properly. + ConfiguredTarget lib = getConfiguredTarget("//java/jarlib2:lib"); + JavaModuleFlagsProviderApi libModuleFlagsProvider = + JavaInfo.getJavaInfo(lib).getJavaModuleFlagsInfo(); + assertThat(libModuleFlagsProvider.getAddExports().toList(String.class)) + .containsExactly("java.base/java.lang"); + assertThat(libModuleFlagsProvider.getAddOpens().toList(String.class)) + .containsExactly("java.base/java.util"); + } + @Test public void testSrcJars() throws Exception { ConfiguredTarget jarLibWithSources =