Skip to content

Commit

Permalink
Add version to JavaRuntimeInfo.
Browse files Browse the repository at this point in the history
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)
  • Loading branch information
benjaminp committed Mar 14, 2023
1 parent 844b5d2 commit 42e1a55
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnforcementLevel;
import com.google.devtools.build.lib.rules.java.JavaHelper;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
import com.google.devtools.build.lib.rules.java.JavaRuntimeInfo;
import com.google.devtools.build.lib.rules.java.JavaSemantics;
import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
import com.google.devtools.build.lib.rules.java.JavaTargetAttributes;
Expand Down Expand Up @@ -548,6 +549,9 @@ public Iterable<String> getJvmFlags(
if (testClass == null) {
ruleContext.ruleError("cannot determine test class");
} else {
if (JavaRuntimeInfo.from(ruleContext).version() >= 17) {
jvmFlags.add("-Djava.security.manager=allow");
}
// Always run junit tests with -ea (enable assertion)
jvmFlags.add("-ea");
// "suite" is a misnomer.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
# External dependencies for the java_* rules.
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
load("@bazel_tools//tools/jdk:jdk_build_file.bzl", "JDK_BUILD_TEMPLATE")
load("@bazel_tools//tools/jdk:local_java_repository.bzl", "local_java_repository")
load("@bazel_tools//tools/jdk:remote_java_repository.bzl", "remote_java_repository")

maybe(
local_java_repository,
name = "local_jdk",
java_home = DEFAULT_SYSTEM_JAVABASE,
build_file = "@bazel_tools//tools/jdk:jdk.BUILD",
build_file_content = JDK_BUILD_TEMPLATE,
)

# OpenJDK distributions that should only be downloaded on demand (e.g. when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.lib.rules.java;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.devtools.build.lib.packages.Type.INTEGER;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -129,7 +130,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
hermeticInputs,
libModules,
defaultCDS,
hermeticStaticLibs);
hermeticStaticLibs,
ruleContext.attributes().get("version", INTEGER).toIntUnchecked());

TemplateVariableInfo templateVariableInfo =
new TemplateVariableInfo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public static JavaRuntimeInfo create(
NestedSet<Artifact> hermeticInputs,
@Nullable Artifact libModules,
@Nullable Artifact defaultCDS,
ImmutableList<CcInfo> hermeticStaticLibs) {
ImmutableList<CcInfo> hermeticStaticLibs,
int version) {
return new JavaRuntimeInfo(
javaBaseInputs,
javaHome,
Expand All @@ -62,7 +63,8 @@ public static JavaRuntimeInfo create(
hermeticInputs,
libModules,
defaultCDS,
hermeticStaticLibs);
hermeticStaticLibs,
version);
}

@Override
Expand Down Expand Up @@ -125,6 +127,7 @@ private static JavaRuntimeInfo from(RuleContext ruleContext, ToolchainInfo toolc
@Nullable private final Artifact libModules;
@Nullable private final Artifact defaultCDS;
private final ImmutableList<CcInfo> hermeticStaticLibs;
private final int version;

private JavaRuntimeInfo(
NestedSet<Artifact> javaBaseInputs,
Expand All @@ -135,7 +138,8 @@ private JavaRuntimeInfo(
NestedSet<Artifact> hermeticInputs,
@Nullable Artifact libModules,
@Nullable Artifact defaultCDS,
ImmutableList<CcInfo> hermeticStaticLibs) {
ImmutableList<CcInfo> hermeticStaticLibs,
int version) {
this.javaBaseInputs = javaBaseInputs;
this.javaHome = javaHome;
this.javaBinaryExecPath = javaBinaryExecPath;
Expand All @@ -145,6 +149,7 @@ private JavaRuntimeInfo(
this.libModules = libModules;
this.defaultCDS = defaultCDS;
this.hermeticStaticLibs = hermeticStaticLibs;
this.version = version;
}

/** All input artifacts in the javabase. */
Expand Down Expand Up @@ -232,6 +237,11 @@ public Depset starlarkJavaBaseInputs() {
return Depset.of(Artifact.class, javaBaseInputs());
}

@Override
public int version() {
return version;
}

@Override
public com.google.devtools.build.lib.packages.Provider getProvider() {
return PROVIDER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
import static com.google.devtools.build.lib.packages.BuildType.LICENSE;
import static com.google.devtools.build.lib.packages.Type.INTEGER;
import static com.google.devtools.build.lib.packages.Type.STRING;

import com.google.devtools.build.lib.analysis.BaseRuleClasses;
Expand Down Expand Up @@ -83,6 +84,10 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("java_home", STRING))
.add(attr("output_licenses", LICENSE))
/* <!-- #BLAZE_RULE(java_runtime).ATTRIBUTE(version) -->
The major version of the Java runtime.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("version", INTEGER))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,11 @@ public interface JavaRuntimeInfoApi extends StructApi {
doc = "Returns the JDK static libraries.",
structField = true)
Sequence<CcInfo> starlarkHermeticStaticLibs();

/** The Java version of the runtime. This may be 0 if the version is unknown. */
@StarlarkMethod(
name = "version",
doc = "The Java version of the runtime. This may be 0 if the version is unknown.",
structField = true)
int version();
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ def _bazel_java_binary_impl(ctx):
)

if ctx.attr.use_testrunner:
if semantics.find_java_runtime_toolchain(ctx).version >= 17:
jvm_flags.append("-Djava.security.manager=allow");
test_class = ctx.attr.test_class if hasattr(ctx.attr, "test_class") else ""
if test_class == "":
test_class = helper.primary_class(ctx)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public void equalityIsObjectIdentity() {
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
null,
null,
ImmutableList.of());
ImmutableList.of(),
17);
JavaRuntimeInfo b =
JavaRuntimeInfo.create(
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
Expand All @@ -49,7 +50,8 @@ public void equalityIsObjectIdentity() {
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
null,
null,
ImmutableList.of());
ImmutableList.of(),
17);

new EqualsTester().addEqualityGroup(a).addEqualityGroup(b).testEquals();
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/py/bazel/query_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def testSimpleQuery(self):
def testQueryFilesUsedByRepositoryRules(self):
self.ScratchFile('WORKSPACE')
self._AssertQueryOutputContains("kind('source file', deps(//external:*))",
'@bazel_tools//tools/jdk:jdk.BUILD')
'@bazel_tools//tools/genrule:genrule-setup.sh')

def testBuildFilesForExternalRepos_Simple(self):
self.ScratchFile('WORKSPACE', [
Expand Down
2 changes: 1 addition & 1 deletion tools/jdk/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ filegroup(
"default_java_toolchain.bzl",
"fail_rule.bzl",
"java_toolchain_alias.bzl",
"jdk.BUILD",
"jdk_build_file.bzl",
"local_java_repository.bzl",
"nosystemjdk/README",
"proguard_whitelister.py",
Expand Down
1 change: 0 additions & 1 deletion tools/jdk/BUILD.tools
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ alias(

exports_files([
"BUILD.java_tools",
"jdk.BUILD",
])

alias(
Expand Down
4 changes: 3 additions & 1 deletion tools/jdk/jdk.BUILD → tools/jdk/jdk_build_file.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@rules_java//java:defs.bzl", "java_runtime")
JDK_BUILD_TEMPLATE = """load("@rules_java//java:defs.bzl", "java_runtime")
package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -67,4 +67,6 @@ java_runtime(
":jdk-lib",
":jre",
],
version = ___VERSION___,
)
"""
11 changes: 9 additions & 2 deletions tools/jdk/local_java_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,13 @@ def _local_java_repository_impl(repository_ctx):

# Prepare BUILD file using "local_java_runtime" macro
build_file = ""
if repository_ctx.attr.build_file_content:
build_file = repository_ctx.attr.build_file_content
if repository_ctx.attr.build_file != None:
if build_file != "":
fail("build_file and build_file_content are exclusive")
build_file = repository_ctx.read(repository_ctx.path(repository_ctx.attr.build_file))
build_file = build_file.replace("___VERSION___", version if version else "0")

runtime_name = '"jdk"' if repository_ctx.attr.build_file else None
local_java_runtime_macro = """
Expand Down Expand Up @@ -201,10 +206,11 @@ _local_java_repository_rule = repository_rule(
"java_home": attr.string(),
"version": attr.string(),
"build_file": attr.label(),
"build_file_content": attr.string(),
},
)

def local_java_repository(name, java_home, version = "", build_file = None):
def local_java_repository(name, java_home, version = "", build_file = None, build_file_content = None):
"""Registers a runtime toolchain for local JDK and creates an unregistered compile toolchain.
Toolchain resolution is constrained with --java_runtime_version flag
Expand All @@ -220,7 +226,8 @@ def local_java_repository(name, java_home, version = "", build_file = None):
name: A unique name for this rule.
java_home: Location of the JDK imported.
build_file: optionally BUILD file template
build_file_content: optional BUILD file template as a string
version: optionally java version
"""
_local_java_repository_rule(name = name, java_home = java_home, version = version, build_file = build_file)
_local_java_repository_rule(name = name, java_home = java_home, version = version, build_file = build_file, build_file_content = build_file_content)
native.register_toolchains("@" + name + "//:runtime_toolchain_definition")
3 changes: 2 additions & 1 deletion tools/jdk/remote_java_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Rule remote_java_repository imports and registers JDK with the toolchain resolut
"""

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("@bazel_tools//tools/jdk:jdk_build_file.bzl", "JDK_BUILD_TEMPLATE")

def _toolchain_config_impl(ctx):
ctx.file("WORKSPACE", "workspace(name = \"{name}\")\n".format(name = ctx.name))
Expand Down Expand Up @@ -47,7 +48,7 @@ def remote_java_repository(name, version, target_compatible_with = None, prefix
"""
http_archive(
name = name,
build_file = "@bazel_tools//tools/jdk:jdk.BUILD",
build_file_content = JDK_BUILD_TEMPLATE.replace("___VERSION___", version),
**kwargs
)
_toolchain_config(
Expand Down

0 comments on commit 42e1a55

Please sign in to comment.