From 521dbb08b0c703ba896684bafdc60805767f9d64 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 2 Nov 2023 14:40:57 -0700 Subject: [PATCH] Add --incompatible_java_info_merge_runtime_module_flags --- .../semantics/BuildLanguageOptions.java | 14 ++++ .../lib/rules/java/JavaStarlarkCommon.java | 9 +++ .../starlarkbuildapi/java/JavaCommonApi.java | 6 ++ .../builtins_bzl/common/java/java_info.bzl | 15 ++++- .../rules/java/JavaInfoStarlarkApiTest.java | 65 ++++++++++++++++++- 5 files changed, 106 insertions(+), 3 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 15872a959b3a54..dd92591d26bde7 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 @@ -573,6 +573,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", @@ -743,6 +754,7 @@ 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_NO_ATTR_LICENSE, incompatibleNoAttrLicense) .setBool(INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT, incompatibleNoImplicitFileExport) .setBool(INCOMPATIBLE_NO_PACKAGE_DISTRIBS, incompatibleNoPackageDistribs) @@ -849,6 +861,8 @@ public StarlarkSemantics toStarlarkSemantics() { "+incompatible_do_not_split_linking_cmdline"; public static final String INCOMPATIBLE_JAVA_COMMON_PARAMETERS = "+incompatible_java_common_parameters"; + public static final String INCOMPATIBLE_JAVA_INFO_MERGE_RUNTIME_MODULE_FLAGS = + "-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 2fa0b75e2f93a4..c3c9379653df64 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 @@ -397,6 +397,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 d31c2ed5b1d65b..5eb1e8f1073a66 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 @@ -691,6 +691,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 28623a93ee87f7..12025128701f95 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_info.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_info.bzl @@ -723,6 +723,17 @@ 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 JavaInfo constructor + # rather than runtimedeps_exports_deps (used by java_info_for_compilation). + # However, runtimedeps_exports_deps makes more sense. + # + # 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( @@ -736,11 +747,11 @@ def _javainfo_init( module_flags_info = _create_module_flags_info( add_exports = depset(transitive = [ dep.module_flags_info.add_exports - for dep in concatenated_deps.deps_exports + for dep in module_flags_deps ]), add_opens = depset(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 5e1b84da5b54aa..62668364fb1261 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", @@ -857,11 +858,24 @@ 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',", ")"); assertNoEvents(); @@ -870,7 +884,56 @@ public void buildHelperCreateJavaInfoWithModuleFlags() throws Exception { fetchJavaInfo().getProvider(JavaModuleFlagsProvider.class); assertThat(ruleOutputs.toFlags()) - .containsExactly("--add-opens=java.base/java.lang=ALL-UNNAMED"); + .containsExactly( + "--add-exports=java.base/java.lang=ALL-UNNAMED", + // no java.base/java.util under --noincompatible_java_info_merge_runtime_module_flags + "--add-opens=java.base/java.lang=ALL-UNNAMED", + "--add-opens=java.base/java.math=ALL-UNNAMED") + .inOrder(); + } + + @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',", + ")"); + assertNoEvents(); + + JavaModuleFlagsProvider ruleOutputs = + fetchJavaInfo().getProvider(JavaModuleFlagsProvider.class); + + 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