Skip to content

Commit

Permalink
Add --incompatible_java_info_merge_runtime_module_flags
Browse files Browse the repository at this point in the history
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
  • Loading branch information
timothyg-stripe authored and copybara-github committed Nov 16, 2023
1 parent be8fdca commit eafa300
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -757,6 +768,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_NO_ATTR_LICENSE, incompatibleNoAttrLicense)
.setBool(INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT, incompatibleNoImplicitFileExport)
.setBool(INCOMPATIBLE_NO_PACKAGE_DISTRIBS, incompatibleNoPackageDistribs)
Expand Down Expand Up @@ -864,6 +878,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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
Expand Down
17 changes: 15 additions & 2 deletions src/main/starlark/builtins_bzl/common/java/java_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,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(
Expand All @@ -724,11 +737,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
]),
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit eafa300

Please sign in to comment.