Skip to content

Commit b841e14

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
Fix and verify Java version compatibility of Java tools
Precompiled Java tools meant to run with 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 versions 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. Related to #26100 Closes #26126. PiperOrigin-RevId: 763699406 Change-Id: Id992546ca6302da11c7280a29417cba893411b51
1 parent 14ac826 commit b841e14

13 files changed

Lines changed: 117 additions & 197 deletions

File tree

‎MODULE.bazel.lock‎

Lines changed: 6 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎repositories.bzl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ DIST_ARCHIVE_REPOS = [get_canonical_repo_name(repo) for repo in [
5151
"rules_proto",
5252
"rules_python",
5353
"rules_shell",
54+
"with_cfg.bzl",
5455
"zlib",
5556
"zstd-jni",
5657
]] + [(get_canonical_repo_name("com_github_grpc_grpc") + "+grpc_repo_deps_ext+" + suffix) for suffix in [

‎src/BUILD‎

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
# Packaging
22

33
load("@bazel_pip_dev_deps//:requirements.bzl", "requirement")
4-
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
54
load("@rules_cc//cc:cc_binary.bzl", "cc_binary")
65
load("@rules_java//java:java_binary.bzl", "java_binary")
76
load("@rules_python//python:defs.bzl", "py_binary", "py_library")
87
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")
8+
load("//src:release_archive.bzl", "minimum_java_compilation_runtime_filegroup", "minimum_java_runtime_filegroup", "release_archive")
119
load(":embedded_tools.bzl", "srcsfile")
1210
load(":rule_size_test.bzl", "rule_size_test")
1311

@@ -513,19 +511,33 @@ genrule(
513511
)
514512

515513
# Following targets build java_tools.zip - platform independent part of java_tools
516-
JAVA_TOOLS_DEPLOY_JARS = [
517-
"//src/java_tools/buildjar:JavaBuilder_deploy.jar",
518-
"//src/java_tools/buildjar:VanillaJavaBuilder_deploy.jar",
519-
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass:GenClass_deploy.jar",
520-
"//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_direct_binary_deploy.jar",
521-
"//src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps:ImportDepsChecker_deploy.jar",
522-
"//src/java_tools/junitrunner/java/com/google/testing/coverage:JacocoCoverage_jarjar_deploy.jar",
523-
"//src/java_tools/junitrunner/java/com/google/testing/junit/runner:Runner_deploy.jar",
524-
]
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+
)
525534

526535
release_archive(
527536
name = "jars_java_tools_zip",
528-
srcs = JAVA_TOOLS_DEPLOY_JARS,
537+
srcs = [
538+
":minimum_java_compilation_runtime_tools",
539+
":minimum_java_runtime_tools",
540+
],
529541
package_dir = "java_tools",
530542
visibility = ["//visibility:private"],
531543
)
@@ -546,12 +558,6 @@ release_archive(
546558
],
547559
)
548560

549-
transition_java_language_8_archive(
550-
name = "java_tools_java8.zip",
551-
archive_zip = ":java_tools.zip",
552-
visibility = ["//src/test/shell/bazel:__pkg__"],
553-
)
554-
555561
release_archive(
556562
name = "turbine_direct_graal_zip",
557563
srcs = ["//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_direct_graal"],
@@ -574,12 +580,6 @@ release_archive(
574580
],
575581
)
576582

577-
transition_java_language_8_archive(
578-
name = "java_tools_prebuilt_java8.zip",
579-
archive_zip = ":java_tools_prebuilt.zip",
580-
visibility = ["//src/test/shell/bazel:__pkg__"],
581-
)
582-
583583
# Following targets used by the java_tools_binaries Buildkite pipeline to upload
584584
# the java_tools_*.zip to either tmp/sources or tmp/build directories in GCS.
585585
sh_binary(
@@ -665,11 +665,6 @@ filegroup(
665665
],
666666
)
667667

668-
bzl_library(
669-
name = "build_defs_bzl",
670-
srcs = ["build_defs.bzl"],
671-
)
672-
673668
java_binary(
674669
name = "CheckSunJnuEncoding",
675670
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 & 4 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()));
@@ -325,8 +324,7 @@ private static void setLocations(JavacFileManager fileManager, BlazeJavacArgumen
325324

326325
Path system = arguments.system();
327326
if (system != null) {
328-
fileManager.setLocationFromPaths(
329-
StandardLocation.locationFor("SYSTEM_MODULES"), ImmutableList.of(system));
327+
fileManager.setLocationFromPaths(StandardLocation.SYSTEM_MODULES, ImmutableList.of(system));
330328
}
331329
// The bootclasspath may legitimately be empty if --release is being used.
332330
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 = 17
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

0 commit comments

Comments
 (0)