Skip to content

Commit

Permalink
[7.0.0] Backport add_exports/add_opens fixes (#20260)
Browse files Browse the repository at this point in the history
Backport #20036, #20035, and #20037.

Fixes #20258.
  • Loading branch information
timothyg-stripe authored Nov 20, 2023
1 parent 2d61c9e commit e2249f9
Show file tree
Hide file tree
Showing 9 changed files with 227 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -861,6 +875,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_NEW_ACTIONS_API = "+incompatible_new_actions_api";
public static final String INCOMPATIBLE_NO_ATTR_LICENSE = "+incompatible_no_attr_license";
public static final String INCOMPATIBLE_NO_PACKAGE_DISTRIBS = "-incompatible_no_package_distribs";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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
26 changes: 19 additions & 7 deletions src/main/starlark/builtins_bzl/common/java/java_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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(
Expand All @@ -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.
Expand All @@ -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 <module>/<package>.
add_opens: (list[str]) Allow this library to reflectively access the given <module>/<package>.
Returns:
(list[provider]) A list containing DefaultInfo, JavaInfo,
Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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),
}
Expand Down
34 changes: 27 additions & 7 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 All @@ -723,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(
Expand All @@ -734,13 +754,13 @@ 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
for dep in module_flags_deps
]),
add_opens = depset(transitive = [
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
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,
)
Loading

0 comments on commit e2249f9

Please sign in to comment.