Skip to content

Commit 161cd83

Browse files
committed
Fix and ensure Java 8/11 compatibility of prebuilt Java tools
Prebuilt Java tools meant to run on the user-specified `--java_{tool_,}runtime_version` must only use Java 8 language features and APIs. Previously, these tools were compiled with `--java_language_version=8`, but had access to a JDK 21 as the bootclasspath. In fact, `junitrunner` used `Stream#dropWhile`, which isn't available in a JDK 8. Similarly, Java tools meant to run on the Java runtime provided by a Java (compilation) toolchain must only use Java 11 language features and APIs. This was the case, but they were in fact build with `--java_language_version=8` and a JDK 21, just like the other tools. This PR splits tools into two groups and enforces consistent Java language and bootclasspath runtime for them via `--@rules_java//toolchains:incompatible_language_version_bootclasspath`. Constants in `//src:release_archive.bzl` can be used to set the two minimum versions globally.
1 parent 8c756b3 commit 161cd83

File tree

10 files changed

+113
-193
lines changed

10 files changed

+113
-193
lines changed

.bazelrc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,13 @@ build:windows --per_file_copt=external/.*@/w
5252
build:windows --host_per_file_copt=external/.*@/w
5353

5454
# Enable Java 21 language features
55-
build --java_runtime_version=21
56-
build --java_language_version=21
57-
build --tool_java_language_version=21
58-
build --tool_java_runtime_version=21
55+
common --java_runtime_version=21
56+
common --java_language_version=21
57+
common --tool_java_language_version=21
58+
common --tool_java_runtime_version=21
59+
# Ensure that Java language level and bootclasspath version are the same.
60+
# This avoids accidental usage of more recent Java APIs.
61+
common --@rules_java//toolchains:incompatible_language_version_bootclasspath
5962

6063
# User-specific .bazelrc
6164
try-import %workspace%/user.bazelrc

src/BUILD

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ load("@rules_cc//cc:cc_binary.bzl", "cc_binary")
66
load("@rules_java//java:java_binary.bzl", "java_binary")
77
load("@rules_python//python:defs.bzl", "py_binary", "py_library")
88
load("@rules_shell//shell:sh_binary.bzl", "sh_binary")
9-
load("//src:build_defs.bzl", "transition_java_language_8_archive")
10-
load("//src:release_archive.bzl", "release_archive")
9+
load("//src:release_archive.bzl", "minimum_java_compilation_runtime_filegroup", "minimum_java_runtime_filegroup", "release_archive")
1110
load(":embedded_tools.bzl", "srcsfile")
1211
load(":rule_size_test.bzl", "rule_size_test")
1312

@@ -512,19 +511,33 @@ genrule(
512511
)
513512

514513
# Following targets build java_tools.zip - platform independent part of java_tools
515-
JAVA_TOOLS_DEPLOY_JARS = [
516-
"//src/java_tools/buildjar:JavaBuilder_deploy.jar",
517-
"//src/java_tools/buildjar:VanillaJavaBuilder_deploy.jar",
518-
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass:GenClass_deploy.jar",
519-
"//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_direct_binary_deploy.jar",
520-
"//src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps:ImportDepsChecker_deploy.jar",
521-
"//src/java_tools/junitrunner/java/com/google/testing/coverage:JacocoCoverage_jarjar_deploy.jar",
522-
"//src/java_tools/junitrunner/java/com/google/testing/junit/runner:Runner_deploy.jar",
523-
]
514+
minimum_java_runtime_filegroup(
515+
name = "minimum_java_runtime_tools",
516+
srcs = [
517+
"//src/java_tools/junitrunner/java/com/google/testing/coverage:JacocoCoverage_jarjar_deploy.jar",
518+
"//src/java_tools/junitrunner/java/com/google/testing/junit/runner:Runner_deploy.jar",
519+
],
520+
visibility = ["//visibility:private"],
521+
)
522+
523+
minimum_java_compilation_runtime_filegroup(
524+
name = "minimum_java_compilation_runtime_tools",
525+
srcs = [
526+
"//src/java_tools/buildjar:JavaBuilder_deploy.jar",
527+
"//src/java_tools/buildjar:VanillaJavaBuilder_deploy.jar",
528+
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass:GenClass_deploy.jar",
529+
"//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_direct_binary_deploy.jar",
530+
"//src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps:ImportDepsChecker_deploy.jar",
531+
],
532+
visibility = ["//visibility:private"],
533+
)
524534

525535
release_archive(
526536
name = "jars_java_tools_zip",
527-
srcs = JAVA_TOOLS_DEPLOY_JARS,
537+
srcs = [
538+
":minimum_java_compilation_runtime_tools",
539+
":minimum_java_runtime_tools",
540+
],
528541
package_dir = "java_tools",
529542
visibility = ["//visibility:private"],
530543
)
@@ -545,12 +558,6 @@ release_archive(
545558
],
546559
)
547560

548-
transition_java_language_8_archive(
549-
name = "java_tools_java8.zip",
550-
archive_zip = ":java_tools.zip",
551-
visibility = ["//src/test/shell/bazel:__pkg__"],
552-
)
553-
554561
release_archive(
555562
name = "turbine_direct_graal_zip",
556563
srcs = ["//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_direct_graal"],
@@ -573,12 +580,6 @@ release_archive(
573580
],
574581
)
575582

576-
transition_java_language_8_archive(
577-
name = "java_tools_prebuilt_java8.zip",
578-
archive_zip = ":java_tools_prebuilt.zip",
579-
visibility = ["//src/test/shell/bazel:__pkg__"],
580-
)
581-
582583
# Following targets used by the java_tools_binaries Buildkite pipeline to upload
583584
# the java_tools_*.zip to either tmp/sources or tmp/build directories in GCS.
584585
sh_binary(
@@ -664,11 +665,6 @@ filegroup(
664665
],
665666
)
666667

667-
bzl_library(
668-
name = "build_defs_bzl",
669-
srcs = ["build_defs.bzl"],
670-
)
671-
672668
java_binary(
673669
name = "CheckSunJnuEncoding",
674670
srcs = ["CheckSunJnuEncoding.java"],

src/build_defs.bzl

Lines changed: 0 additions & 58 deletions
This file was deleted.

src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal/SystemExitDetectingShutdownHook.java

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@
1414

1515
package com.google.testing.junit.runner.internal;
1616

17-
import static java.util.Arrays.stream;
18-
import static java.util.stream.Collectors.toList;
19-
2017
import java.io.PrintStream;
18+
import java.util.ArrayList;
2119
import java.util.List;
2220

2321
/**
@@ -28,40 +26,41 @@
2826
*/
2927
public class SystemExitDetectingShutdownHook {
3028
public static Thread newShutdownHook(PrintStream testRunnerOut) {
31-
Runnable hook = () -> {
32-
boolean foundRuntimeExit = false;
33-
for (StackTraceElement[] stack : Thread.getAllStackTraces().values()) {
34-
@SuppressWarnings("JdkCollectors") // can't use ImmutableList here
35-
List<String> framesStartingWithRuntimeExit =
36-
stream(stack)
37-
.dropWhile(
38-
frame ->
39-
!frame.getClassName().equals("java.lang.Runtime")
40-
|| !frame.getMethodName().equals("exit"))
41-
.map(SystemExitDetectingShutdownHook::frameString)
42-
.collect(toList());
43-
if (!framesStartingWithRuntimeExit.isEmpty()) {
44-
foundRuntimeExit = true;
45-
testRunnerOut.println("\nSystem.exit or Runtime.exit was called!");
46-
testRunnerOut.println(String.join("\n", framesStartingWithRuntimeExit));
47-
}
48-
}
49-
if (foundRuntimeExit) {
50-
// We must call halt rather than exit, because exit would lead to a deadlock. We use a
51-
// hopefully unique exit code to make it easier to identify this case.
52-
Runtime.getRuntime().halt(121);
53-
}
54-
};
29+
Runnable hook =
30+
() -> {
31+
boolean foundRuntimeExit = false;
32+
for (StackTraceElement[] stack : Thread.getAllStackTraces().values()) {
33+
List<String> framesStartingWithRuntimeExit = new ArrayList<>();
34+
boolean foundRuntimeExitInThisThread = false;
35+
for (StackTraceElement frame : stack) {
36+
if (!foundRuntimeExitInThisThread
37+
&& frame.getClassName().equals("java.lang.Runtime")
38+
&& frame.getMethodName().equals("exit")) {
39+
foundRuntimeExitInThisThread = true;
40+
}
41+
if (foundRuntimeExitInThisThread) {
42+
framesStartingWithRuntimeExit.add(frameString(frame));
43+
}
44+
}
45+
if (foundRuntimeExitInThisThread) {
46+
foundRuntimeExit = true;
47+
testRunnerOut.println("\nSystem.exit or Runtime.exit was called!");
48+
testRunnerOut.println(String.join("\n", framesStartingWithRuntimeExit));
49+
}
50+
}
51+
if (foundRuntimeExit) {
52+
// We must call halt rather than exit, because exit would lead to a deadlock. We use a
53+
// hopefully unique exit code to make it easier to identify this case.
54+
Runtime.getRuntime().halt(121);
55+
}
56+
};
5557
return new Thread(hook, "SystemExitDetectingShutdownHook");
5658
}
5759

5860
private static String frameString(StackTraceElement frame) {
5961
return String.format(
6062
" at %s.%s(%s:%d)",
61-
frame.getClassName(),
62-
frame.getMethodName(),
63-
frame.getFileName(),
64-
frame.getLineNumber());
63+
frame.getClassName(), frame.getMethodName(), frame.getFileName(), frame.getLineNumber());
6564
}
6665

6766
private SystemExitDetectingShutdownHook() {}

src/release_archive.bzl

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,37 @@
1414

1515
"""Rules to create a release archive"""
1616

17+
load("@rules_java//java:java_binary.bzl", "java_binary")
18+
load("@with_cfg.bzl", "with_cfg")
19+
20+
# The minimum --java_{tool_,}runtime_version supported by prebuilt Java tools.
21+
_MINIMUM_JAVA_RUNTIME_VERSION = 8
22+
23+
# The minimum version of a java_toolchain's java_runtime supported by prebuilt Java tools.
24+
_MINIMUM_JAVA_COMPILATION_RUNTIME_VERSION = 11
25+
26+
minimum_java_runtime_java_binary, _minimum_java_runtime_java_binary = (
27+
# Don't warn about targeting very old Java versions.
28+
with_cfg(java_binary)
29+
.set("java_language_version", str(_MINIMUM_JAVA_RUNTIME_VERSION))
30+
.extend("javacopt", ["-Xlint:-options"])
31+
.build()
32+
)
33+
34+
minimum_java_runtime_filegroup, _minimum_java_runtime_filegroup = (
35+
# Don't warn about targeting very old Java versions.
36+
with_cfg(native.filegroup)
37+
.set("java_language_version", str(_MINIMUM_JAVA_RUNTIME_VERSION))
38+
.extend("javacopt", ["-Xlint:-options"])
39+
.build()
40+
)
41+
42+
minimum_java_compilation_runtime_filegroup, _minimum_java_compilation_runtime_filegroup = (
43+
with_cfg(native.filegroup)
44+
.set("java_language_version", str(_MINIMUM_JAVA_COMPILATION_RUNTIME_VERSION))
45+
.build()
46+
)
47+
1748
def release_archive(name, srcs = [], src_map = {}, package_dir = "-", deps = [], **kwargs):
1849
""" Creates an zip of the srcs, and renamed label artifacts.
1950

src/test/shell/bazel/BUILD

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -233,18 +233,14 @@ sh_test(
233233
srcs = ["bazel_java17_test.sh"],
234234
args = [
235235
# java_tools zip to test
236-
"$(rlocationpath //src:java_tools_java8.zip)",
237-
"$(rlocationpath //src:java_tools_prebuilt_java8.zip)",
236+
"$(rlocationpath //src:java_tools.zip)",
237+
"$(rlocationpath //src:java_tools_prebuilt.zip)",
238238
],
239239
data = [
240240
":gen_rules_java_repo_name",
241241
":test-deps",
242-
# Use java_tools compiled at Java language 8 instead (aligned with the
243-
# default compilation level of java_tools). For the purpose
244-
# of this test, we need java_tools to be running with runtime >11 to
245-
# test the failure modes of incompatible system classpaths.
246-
"//src:java_tools_prebuilt_java8.zip",
247-
"//src:java_tools_java8.zip",
242+
"//src:java_tools.zip",
243+
"//src:java_tools_prebuilt.zip",
248244
"@bazel_tools//tools/bash/runfiles",
249245
],
250246
tags = ["local"],
@@ -294,8 +290,8 @@ JAVA_VERSIONS_COVERAGE = ("11", "17", "21")
294290
srcs = ["bazel_java_test.sh"],
295291
args = [
296292
# java_tools zips to test
297-
"$(rlocationpath %s)" % ("//src:java_tools_zip" if java_version == "21" else "//src:java_tools_java8.zip"),
298-
"$(rlocationpath %s)" % ("//src:java_tools_prebuilt_zip" if java_version == "21" else "//src:java_tools_prebuilt_java8.zip"),
293+
"$(rlocationpath //src:java_tools_zip)",
294+
"$(rlocationpath //src:java_tools_zip)",
299295
# --java_language_version value
300296
java_version,
301297
# --java_runtime_version value
@@ -304,8 +300,8 @@ JAVA_VERSIONS_COVERAGE = ("11", "17", "21")
304300
data = [
305301
":gen_rules_java_repo_name",
306302
":test-deps",
307-
"//src:java_tools_zip" if java_version == "21" else "//src:java_tools_java8.zip",
308-
"//src:java_tools_prebuilt_zip" if java_version == "21" else "//src:java_tools_prebuilt_java8.zip",
303+
"//src:java_tools_prebuilt_zip",
304+
"//src:java_tools_zip",
309305
"@bazel_tools//tools/bash/runfiles",
310306
],
311307
exec_compatible_with = ["//:highcpu_machine"],
@@ -560,8 +556,8 @@ sh_test(
560556
srcs = ["bazel_coverage_java_test.sh"],
561557
args = [
562558
# java_tools zips to test
563-
"$(rlocationpath %s)" % ("//src:java_tools_zip" if java_version == "21" else "//src:java_tools_java8.zip"),
564-
"$(rlocationpath %s)" % ("//src:java_tools_prebuilt_zip" if java_version == "21" else "//src:java_tools_prebuilt_java8.zip"),
559+
"$(rlocationpath //src:java_tools_zip)",
560+
"$(rlocationpath //src:java_tools_prebuilt_zip)",
565561
# WORKSPACE file of the coverage output generator repo to test
566562
"$(rlocationpath //tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:coverage_empty_workspace)",
567563
# --java_runtime_version value
@@ -570,10 +566,10 @@ sh_test(
570566
data = [
571567
":gen_rules_java_repo_name",
572568
":test-deps",
573-
"//src:java_tools_prebuilt_zip" if java_version == "21" else "//src:java_tools_prebuilt_java8.zip",
574-
"//src:java_tools_zip" if java_version == "21" else "//src:java_tools_java8.zip",
569+
"//src:java_tools_prebuilt_zip",
570+
"//src:java_tools_zip",
575571
"//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:coverage_empty_workspace",
576-
"//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:coverage_output_generator_repo%s" % ("" if java_version == "21" else "_java8"),
572+
"//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:coverage_output_generator_repo",
577573
],
578574
tags = ["no_windows"],
579575
)
@@ -628,7 +624,7 @@ sh_test(
628624
data = [
629625
":test-deps",
630626
"//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:coverage_empty_workspace",
631-
"//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:coverage_output_generator_repo_java8",
627+
"//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:coverage_output_generator_repo",
632628
],
633629
tags = [
634630
"no_windows",

src/upload_all_java_tools.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ commit_hash=$(git rev-parse HEAD)
5858
timestamp=$(date +%s)
5959
bazel_version=$(bazel info release | cut -d' ' -f2)
6060

61-
RELEASE_BUILD_OPTS="-c opt --tool_java_language_version=8 --java_language_version=8"
61+
RELEASE_BUILD_OPTS="-c opt"
6262

6363
# Check that the build machine is set up for Unicode.
6464
bazel run ${RELEASE_BUILD_OPTS} //src:CheckSunJnuEncoding

0 commit comments

Comments
 (0)