From 6a90db47e64febb032cb141f695985092d9f3053 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 16 Nov 2023 03:40:04 -0800 Subject: [PATCH] Add --incompatible_java_info_merge_runtime_module_flags Change the behavior of `JavaInfo` constructor to be more like `java_info_for_compilation`, by merging in the `add_exports` and `add_opens` flags for `runtime_deps` in addition to just `deps` and `exports`. Guard it under an `--incompatible_` flag which defaults to false, but I'm hoping to make it default to true in 8.x. Second half of #20033 Closes #20037. PiperOrigin-RevId: 582982387 Change-Id: Ibff680f71efed82f20da7d9ee83f0bfa7e5f5697 --- .../semantics/BuildLanguageOptions.java | 19 ++++++ .../lib/rules/java/JavaStarlarkCommon.java | 9 +++ .../starlarkbuildapi/java/JavaCommonApi.java | 6 ++ .../builtins_bzl/common/java/java_info.bzl | 17 +++++- .../rules/java/JavaInfoStarlarkApiTest.java | 58 +++++++++++++++++++ 5 files changed, 107 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 95dbed2aff7a4e..5b4de78ad6bcad 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -584,6 +584,17 @@ public final class BuildLanguageOptions extends OptionsBase { + "host_javabase in compile will all be removed.") public boolean incompatibleJavaCommonParameters; + @Option( + name = "incompatible_java_info_merge_runtime_module_flags", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.UNKNOWN}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "If set to true, the JavaInfo constructor will merge add_exports and " + + "add_opens of runtime_deps in addition to deps and exports.") + public boolean incompatibleJavaInfoMergeRuntimeModuleFlags; + @Option( name = "max_computation_steps", defaultValue = "0", @@ -754,6 +765,9 @@ public StarlarkSemantics toStarlarkSemantics() { INCOMPATIBLE_FIX_PACKAGE_GROUP_REPOROOT_SYNTAX, incompatibleFixPackageGroupReporootSyntax) .setBool(INCOMPATIBLE_JAVA_COMMON_PARAMETERS, incompatibleJavaCommonParameters) + .setBool( + INCOMPATIBLE_JAVA_INFO_MERGE_RUNTIME_MODULE_FLAGS, + incompatibleJavaInfoMergeRuntimeModuleFlags) .setBool(INCOMPATIBLE_NEW_ACTIONS_API, incompatibleNewActionsApi) .setBool(INCOMPATIBLE_NO_ATTR_LICENSE, incompatibleNoAttrLicense) .setBool(INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT, incompatibleNoImplicitFileExport) @@ -861,7 +875,12 @@ public StarlarkSemantics toStarlarkSemantics() { "+incompatible_do_not_split_linking_cmdline"; public static final String INCOMPATIBLE_JAVA_COMMON_PARAMETERS = "+incompatible_java_common_parameters"; +<<<<<<< HEAD public static final String INCOMPATIBLE_NEW_ACTIONS_API = "+incompatible_new_actions_api"; +======= + public static final String INCOMPATIBLE_JAVA_INFO_MERGE_RUNTIME_MODULE_FLAGS = + "-incompatible_java_info_merge_runtime_module_flags"; +>>>>>>> eafa300102 (Add --incompatible_java_info_merge_runtime_module_flags) public static final String INCOMPATIBLE_NO_ATTR_LICENSE = "+incompatible_no_attr_license"; public static final String INCOMPATIBLE_NO_PACKAGE_DISTRIBS = "-incompatible_no_package_distribs"; public static final String INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT = diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java index a16547768ac11c..1fc25d99f1fb66 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java @@ -410,6 +410,15 @@ public boolean isDepsetForJavaOutputSourceJarsEnabled(StarlarkThread thread) .getBool(BuildLanguageOptions.INCOMPATIBLE_DEPSET_FOR_JAVA_OUTPUT_SOURCE_JARS); } + @Override + public boolean isJavaInfoMergeRuntimeModuleFlagsEnabled(StarlarkThread thread) + throws EvalException { + checkPrivateAccess(thread); + return thread + .getSemantics() + .getBool(BuildLanguageOptions.INCOMPATIBLE_JAVA_INFO_MERGE_RUNTIME_MODULE_FLAGS); + } + @Override public JavaInfo wrapJavaInfo(Info javaInfo, StarlarkThread thread) throws EvalException, RuleErrorException { diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java index 93829778988927..4d2e0c90f3b1d7 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java @@ -700,6 +700,12 @@ void checkJavaToolchainIsDeclaredOnRuleForStarlark( useStarlarkThread = true) boolean isDepsetForJavaOutputSourceJarsEnabled(StarlarkThread thread) throws EvalException; + @StarlarkMethod( + name = "_incompatible_java_info_merge_runtime_module_flags", + documented = false, + useStarlarkThread = true) + boolean isJavaInfoMergeRuntimeModuleFlagsEnabled(StarlarkThread thread) throws EvalException; + @StarlarkMethod( name = "wrap_java_info", parameters = {@Param(name = "java_info")}, diff --git a/src/main/starlark/builtins_bzl/common/java/java_info.bzl b/src/main/starlark/builtins_bzl/common/java/java_info.bzl index c4de885f13b015..1befe6e8db7294 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_info.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_info.bzl @@ -730,6 +730,19 @@ def _javainfo_init( direct = [output_jar], transitive = [dep.transitive_runtime_jars for dep in concatenated_deps.exports_deps + runtime_deps], ) + + # For backward compatibility, we use deps_exports for add_exports/add_opens + # for the JavaInfo constructor rather than runtimedeps_exports_deps (used + # by java_info_for_compilation). However, runtimedeps_exports_deps makes + # more sense, since add_exports/add_opens from runtime_deps are needed at + # runtime anyway. + # + # TODO: When this flag is removed, move this logic into _javainfo_init_base + # and remove the special case from java_info_for_compilation. + module_flags_deps = concatenated_deps.deps_exports + if _java_common_internal._incompatible_java_info_merge_runtime_module_flags(): + module_flags_deps = concatenated_deps.runtimedeps_exports_deps + result.update( transitive_runtime_jars = transitive_runtime_jars, transitive_source_jars = depset( @@ -743,11 +756,11 @@ def _javainfo_init( module_flags_info = _create_module_flags_info( add_exports = depset(add_exports, transitive = [ dep.module_flags_info.add_exports - for dep in concatenated_deps.deps_exports + for dep in module_flags_deps ]), add_opens = depset(add_opens, transitive = [ dep.module_flags_info.add_opens - for dep in concatenated_deps.deps_exports + for dep in module_flags_deps ]), ), ) diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java index 2e5c09ca3c4bbd..5ec8324e8428be 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java @@ -850,6 +850,7 @@ public void buildHelperCreateJavaInfoWithManifestProto_javaRuleOutputJarsProvide @Test public void buildHelperCreateJavaInfoWithModuleFlags() throws Exception { + setBuildLanguageOptions("--noincompatible_java_info_merge_runtime_module_flags"); ruleBuilder().build(); scratch.file( "foo/BUILD", @@ -906,6 +907,63 @@ public void buildHelperCreateJavaInfoWithModuleFlags() throws Exception { } } + @Test + public void buildHelperCreateJavaInfoWithModuleFlagsIncompatibleMergeRuntime() throws Exception { + setBuildLanguageOptions("--incompatible_java_info_merge_runtime_module_flags"); + ruleBuilder().build(); + scratch.file( + "foo/BUILD", + "load(':extension.bzl', 'my_rule')", + "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); + + if (analysisMock.isThisBazel()) { + assertThat(ruleOutputs.toFlags()) + .containsExactly( + "--add-exports=java.base/java.lang=ALL-UNNAMED", + "--add-exports=java.base/java.lang.invoke=ALL-UNNAMED", + "--add-opens=java.base/java.util=ALL-UNNAMED", + "--add-opens=java.base/java.math=ALL-UNNAMED", + "--add-opens=java.base/java.lang=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", + "--add-opens=java.base/java.util=ALL-UNNAMED", + "--add-opens=java.base/java.math=ALL-UNNAMED", + "--add-opens=java.base/java.lang=ALL-UNNAMED") + .inOrder(); + } + } + @Test public void starlarkJavaOutputsCanBeAddedToJavaPluginInfo() throws Exception { Artifact classJar = createArtifact("foo.jar");