Skip to content

Commit 405fc89

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, thus lacking validation for this property. 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 405fc89

File tree

11 files changed

+115
-196
lines changed

11 files changed

+115
-196
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/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,7 @@ private static void setLocations(JavacFileManager fileManager, BlazeJavacArgumen
297297
try {
298298
fileManager.setLocationFromPaths(StandardLocation.CLASS_PATH, arguments.classPath());
299299
// modular dependencies must be on the module path, not the classpath
300-
fileManager.setLocationFromPaths(
301-
StandardLocation.locationFor("MODULE_PATH"), arguments.classPath());
300+
fileManager.setLocationFromPaths(StandardLocation.MODULE_PATH, arguments.classPath());
302301

303302
fileManager.setLocationFromPaths(
304303
StandardLocation.CLASS_OUTPUT, ImmutableList.of(arguments.classOutput()));
@@ -326,7 +325,7 @@ private static void setLocations(JavacFileManager fileManager, BlazeJavacArgumen
326325
Path system = arguments.system();
327326
if (system != null) {
328327
fileManager.setLocationFromPaths(
329-
StandardLocation.locationFor("SYSTEM_MODULES"), ImmutableList.of(system));
328+
StandardLocation.SYSTEM_MODULES, ImmutableList.of(system));
330329
}
331330
// The bootclasspath may legitimately be empty if --release is being used.
332331
Collection<Path> bootClassPath = arguments.bootClassPath();

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)