Skip to content

Add Java 24 compatibility #1719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions scala/private/phases/phase_coverage.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ load(
"//scala/private:coverage_replacements_provider.bzl",
_coverage_replacements_provider = "coverage_replacements_provider",
)
load(
"//scala/private:rule_impls.bzl",
_allow_security_manager = "allow_security_manager",
)

def phase_coverage_library(ctx, p):
args = struct(
Expand Down Expand Up @@ -64,7 +60,7 @@ def _phase_coverage(ctx, p, srcjars):
outputs = [output_jar],
executable = ctx.attr._code_coverage_instrumentation_worker.files_to_run,
execution_requirements = {"supports-workers": "1"},
arguments = ["--jvm_flag=%s" % f for f in _allow_security_manager(ctx)] + [args],
arguments = [args],
)

replacements = {input_jar: output_jar}
Expand Down
6 changes: 1 addition & 5 deletions scala/private/phases/phase_scalafmt.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
# Outputs to format the scala files when it is explicitly specified
#
load("//scala/private:paths.bzl", _scala_extension = "scala_extension")
load(
"//scala/private:rule_impls.bzl",
_allow_security_manager = "allow_security_manager",
)

def phase_scalafmt(ctx, p):
if ctx.attr.format:
Expand All @@ -26,7 +22,7 @@ def _build_format(ctx):
files = []
srcs = []
manifest_content = []
jvm_flags = ["-Dfile.encoding=UTF-8"] + _allow_security_manager(ctx)
jvm_flags = ["-Dfile.encoding=UTF-8"]
for src in ctx.files.srcs:
# only format scala source files, not generated files
if src.path.endswith(_scala_extension) and src.is_source:
Expand Down
19 changes: 4 additions & 15 deletions scala/private/phases/phase_write_executable.bzl
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
load("//scala/private:common.bzl", "rlocationpath_from_file")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buildifier moves this line up here. Should I leave these changes in?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if it's made by buildifier then keep it. We do not object to buildifier :)


#
# PHASE: write executable
#
# DOCUMENT THIS
#
load(
"//scala/private:rule_impls.bzl",
"allow_security_manager",
"expand_location",
"first_non_empty",
"is_windows",
"java_bin",
"java_bin_windows",
"runfiles_root",
"specified_java_runtime",
)
load("//scala/private:common.bzl", "rlocationpath_from_file")

def phase_write_executable_scalatest(ctx, p):
# jvm_flags passed in on the target override scala_test_jvm_flags passed in on the
Expand All @@ -29,7 +28,7 @@ def phase_write_executable_scalatest(ctx, p):
jvm_flags = [
"-DRULES_SCALA_MAIN_WS_NAME=%s" % ctx.workspace_name,
"-DRULES_SCALA_ARGS_FILE=%s" % rlocationpath_from_file(ctx, p.runfiles.args_file),
] + expand_location(ctx, final_jvm_flags) + _allow_security_manager_for_specified_java_runtime(ctx),
] + expand_location(ctx, final_jvm_flags),
use_jacoco = ctx.configuration.coverage_enabled,
)
return _phase_write_executable_default(ctx, p, args)
Expand All @@ -44,7 +43,7 @@ def phase_write_executable_repl(ctx, p):
def phase_write_executable_junit_test(ctx, p):
args = struct(
rjars = p.coverage_runfiles.rjars,
jvm_flags = p.jvm_flags + ctx.attr.jvm_flags + _allow_security_manager_for_specified_java_runtime(ctx),
jvm_flags = p.jvm_flags + ctx.attr.jvm_flags + ["-Dcom.google.testing.junit.runner.shouldInstallTestSecurityManager=false"],
main_class = "com.google.testing.junit.runner.BazelTestRunner",
use_jacoco = ctx.configuration.coverage_enabled,
)
Expand Down Expand Up @@ -186,13 +185,3 @@ def _jar_path_based_on_java_bin(ctx):
java_bin_var = java_bin(ctx)
jar_path = java_bin_var.rpartition("/")[0] + "/jar"
return jar_path

# Allow security manager for generated test executables if they will be run with jdk >= 17
def _allow_security_manager_for_specified_java_runtime(ctx):
return allow_security_manager(
ctx,
specified_java_runtime(
ctx,
default_runtime = ctx.attr._java_runtime[java_common.JavaRuntimeInfo],
),
)
11 changes: 1 addition & 10 deletions scala/private/rule_impls.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def compile_scala(
# TODO: scalac worker is run with @bazel_tools//tools/jdk:runtime_toolchain_type
# which is different from rules_java where compilation runtime is used from
# @bazel_tools//tools/jdk:toolchain_type
final_scalac_jvm_flags = first_non_empty(scalac_jvm_flags, toolchain.scalac_jvm_flags) + allow_security_manager(ctx)
final_scalac_jvm_flags = first_non_empty(scalac_jvm_flags, toolchain.scalac_jvm_flags)

ctx.actions.run(
inputs = ins,
Expand Down Expand Up @@ -221,12 +221,3 @@ def java_bin_windows(ctx):

def is_windows(ctx):
return ctx.configuration.host_path_separator == ";"

# Return a jvm flag allowing security manager for jdk runtime >= 17
# If no runtime is supplied then runtime is taken from ctx.attr._java_host_runtime
# This must be a runtime used in generated java_binary script (usually workers using SecurityManager)
def allow_security_manager(ctx, runtime = None):
java_runtime = runtime if runtime else ctx.attr._java_host_runtime[java_common.JavaRuntimeInfo]

# Bazel 5.x doesn't have java_runtime.version defined
return ["-Djava.security.manager=allow"] if hasattr(java_runtime, "version") and java_runtime.version >= 17 else []
7 changes: 3 additions & 4 deletions scala_proto/private/scala_proto_aspect.bzl
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("@rules_proto//proto:defs.bzl", "ProtoInfo")
load("//scala/private:common.bzl", "write_manifest_file")
load("//scala/private:dependency.bzl", "legacy_unclear_dependency_info_for_protobuf_scrooge")
load("//scala/private:phases/api.bzl", "extras_phases", "run_aspect_phases")
load(
"//scala/private:rule_impls.bzl",
"compile_scala",
"specified_java_compile_toolchain",
_allow_security_manager = "allow_security_manager",
)
load("//scala/private/toolchain_deps:toolchain_deps.bzl", "find_deps_info_on")
load(
"//scala_proto/private:scala_proto_aspect_provider.bzl",
"ScalaProtoAspectInfo",
)
load("//scala/private:phases/api.bzl", "extras_phases", "run_aspect_phases")
load("@bazel_skylib//lib:dicts.bzl", "dicts")

def _import_paths(proto, ctx):
# Under Bazel 7.x, direct_sources from generated protos may still contain
Expand Down Expand Up @@ -77,7 +76,7 @@ def _generate_sources(ctx, toolchain, proto):

ctx.actions.run(
executable = toolchain.worker,
arguments = ["--jvm_flag=%s" % f for f in _allow_security_manager(ctx)] + [toolchain.worker_flags, args],
arguments = [toolchain.worker_flags, args],
inputs = depset(transitive = [descriptors, toolchain.generators_jars]),
outputs = outputs.values(),
tools = [toolchain.protoc],
Expand Down
25 changes: 0 additions & 25 deletions src/java/io/bazel/rulesscala/worker/Worker.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.security.Permission;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* A base for JVM workers.
Expand Down Expand Up @@ -54,15 +51,6 @@ public static void workerMain(String workerArgs[], Interface workerInterface) th

/** The main loop for persistent worker processes */
private static void persistentWorkerMain(Interface workerInterface) {
System.setSecurityManager(
new SecurityManager() {
@Override
public void checkPermission(Permission permission) {
Matcher matcher = exitPattern.matcher(permission.getName());
if (matcher.find()) throw new ExitTrapped(Integer.parseInt(matcher.group(1)));
}
});

InputStream stdin = System.in;
PrintStream stdout = System.out;
PrintStream stderr = System.err;
Expand Down Expand Up @@ -94,8 +82,6 @@ public void checkPermission(Permission permission) {
String[] workerArgs = stringListToArray(request.getArgumentsList());
String[] args = expandArgsIfArgsfile(workerArgs);
workerInterface.work(args);
} catch (ExitTrapped e) {
code = e.code;
} catch (Exception e) {
if (e instanceof Interface.WorkerException) System.err.println(e.getMessage());
else e.printStackTrace();
Expand Down Expand Up @@ -177,17 +163,6 @@ public void reset() {
}
}

static class ExitTrapped extends RuntimeException {
final int code;

ExitTrapped(int code) {
super();
this.code = code;
}
}

private static Pattern exitPattern = Pattern.compile("exitVM\\.(-?\\d+)");

private static String[] stringListToArray(List<String> argList) {
int numArgs = argList.size();
String[] args = new String[numArgs];
Expand Down
3 changes: 0 additions & 3 deletions src/java/io/bazel/rulesscala/worker/WorkerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,6 @@ public void testBufferWriteReadAndReset() throws Exception {

@AfterClass
public static void teardown() {
// Persistent workers install a security manager. We need to
// reset it here so that our own process can exit!
System.setSecurityManager(null);
}

// Copied/modified from Bazel's MoreAsserts
Expand Down
6 changes: 6 additions & 0 deletions tools/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,19 @@ WARNINGS_CONFIG = [

buildifier(
name = "lint_check",
exclude_patterns = [
"./.ijwb/*",
],
lint_mode = "warn",
lint_warnings = WARNINGS_CONFIG,
mode = "check",
)

buildifier(
name = "lint_fix",
exclude_patterns = [
"./.ijwb/*",
],
lint_mode = "fix",
lint_warnings = WARNINGS_CONFIG,
mode = "fix",
Expand Down
7 changes: 3 additions & 4 deletions twitter_scrooge/twitter_scrooge.bzl
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_skylib//lib:dicts.bzl", "dicts")
load(
"//scala/private:common.bzl",
"write_manifest_file",
Expand All @@ -8,13 +9,11 @@ load(
)
load(
"//scala/private:rule_impls.bzl",
"allow_security_manager",
"compile_java",
"compile_scala",
)
load("//thrift:thrift_info.bzl", "ThriftInfo")
load("//thrift:thrift.bzl", "merge_thrift_infos")
load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("//thrift:thrift_info.bzl", "ThriftInfo")

def _colon_paths(data):
return ":".join([f.path for f in sorted(data)])
Expand Down Expand Up @@ -86,7 +85,7 @@ def _generate_jvm_code(ctx, label, compile_thrifts, include_thrifts, jar_output,
# be correctly handled since the executable is a jvm app that will
# consume the flags on startup.
#arguments = ["--jvm_flag=%s" % flag for flag in ctx.attr.jvm_flags] +
arguments = ["--jvm_flag=%s" % f for f in allow_security_manager(ctx)] + ["@" + argfile.path],
arguments = ["@" + argfile.path],
)

def _compiled_jar_file(actions, scrooge_jar):
Expand Down