Skip to content
Open
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
30 changes: 30 additions & 0 deletions java/common/rules/java_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,22 @@ def _java_toolchain_impl(ctx):
else:
header_compiler_direct_data = []
header_compiler_direct_jvm_opts = []

turbine_direct_jvm_opts = get_internal_java_common().expand_java_opts(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets not complicate things unnecessarily. Just use the opts as is without expansion for now.

ctx,
"turbine_direct_jvm_opts",
tokenize = False,
exec_paths = True,
)
if turbine_direct_jvm_opts and not _is_deploy_jar_tool(ctx.attr.header_compiler_direct):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we're going with a dedicated attribute, I don't think we need to perform all this validation. Lets just pass the user-specified opts as is.

Maybe just a warning note in the attribute docs about graal vs deploy jar.

fail(
"turbine_direct_jvm_opts requires header_compiler_direct to be a " +
"deploy jar (.jar), such as TurbineDirect's deploy jar. Native " +
"direct header compilers such as turbine_direct_graal parse these " +
"values as Turbine command-line options, not JVM options.",
)
header_compiler_direct_jvm_opts = header_compiler_direct_jvm_opts + turbine_direct_jvm_opts

if ctx.attr.oneversion_allowlist and ctx.attr.oneversion_whitelist:
fail("oneversion_allowlist and oneversion_whitelist are mutually exclusive")
oneversion_allowlist = ctx.file.oneversion_allowlist if ctx.file.oneversion_allowlist else ctx.file.oneversion_whitelist
Expand Down Expand Up @@ -156,6 +172,12 @@ def _get_java_runtime(ctx):
return None
return ctx.attr.java_runtime[ToolchainInfo].java_runtime

def _is_deploy_jar_tool(tool):
if not tool:
return False
executable = tool[DefaultInfo].files_to_run.executable
return executable and executable.extension == "jar"

def _get_javac_opts(ctx):
opts = []
if ctx.attr.source_version:
Expand Down Expand Up @@ -588,6 +610,14 @@ Labels of data available for label-expansion in turbine_jvm_opts.
The list of arguments for the JVM when invoking turbine.
""",
),
"turbine_direct_jvm_opts": attr.string_list(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For consistency lets call this header_compiler_direct_opts

doc = """
The list of arguments for the JVM when invoking the direct header compiler.
These options are only valid when the direct header compiler is a deploy jar
(.jar), such as TurbineDirect's deploy jar. Do not set this for native direct
header compilers such as turbine_direct_graal.
""",
),
"xlint": attr.string_list(
default = [],
doc = """
Expand Down
122 changes: 122 additions & 0 deletions test/java/toolchains/java_toolchain_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ load("//java:java_binary.bzl", "java_binary")
load("//java:java_library.bzl", "java_library")
load("//java:java_plugin.bzl", "java_plugin")
load("//java/common:java_common.bzl", "java_common")
load("//java/toolchains:java_runtime.bzl", "java_runtime")
load("//test/java/testutil:java_info_subject.bzl", "java_info_subject")
load("//test/java/testutil:java_toolchain_info_subject.bzl", "java_toolchain_info_subject")
load("//test/java/testutil:javac_action_subject.bzl", "javac_action_subject")
Expand Down Expand Up @@ -560,6 +561,123 @@ def _test_java_compile_action_uses_tool_specific_jvm_opts_impl(env, target):
header_action = env.expect.that_target(target).action_generating("{package}/lib{name}-hjar.jar")
header_action.argv().contains("-DturbineFlag=1")

_TURBINE_DIRECT_JVM_OPT = "-DdirectTurbineFlag=1"

def _test_turbine_direct_jvm_opts_for_direct_header_compiler(name):
util.helper_target(
mock_java_toolchain,
name = name + "/java_toolchain",
header_compiler_direct = name + "/turbine_direct.jar",
turbine_direct_jvm_opts = [_TURBINE_DIRECT_JVM_OPT],
)
util.helper_target(
java_library,
name = name + "/java_lib",
srcs = ["a.java"],
)

analysis_test(
name = name,
impl = _test_turbine_direct_jvm_opts_for_direct_header_compiler_impl,
target = name + "/java_lib",
config_settings = {
"//command_line_option:java_header_compilation": "true",
"//command_line_option:extra_toolchains": [Label(name + "/java_toolchain")],
},
)

def _test_turbine_direct_jvm_opts_for_direct_header_compiler_impl(env, target):
header_action = env.expect.that_target(target).action_generating(
"{package}/lib{name}-hjar.jar",
)

header_action.argv().contains("{package}/{test_name}/turbine_direct.jar")
header_action.argv().contains(_TURBINE_DIRECT_JVM_OPT)

def _test_no_turbine_direct_jvm_opts_by_default(name):
util.helper_target(
mock_java_toolchain,
name = name + "/toolchain",
header_compiler_direct = name + "/turbine_direct.jar",
)
util.helper_target(
java_library,
name = name + "/lib",
srcs = ["a.java"],
)

analysis_test(
name = name,
impl = _test_no_turbine_direct_jvm_opts_by_default_impl,
target = name + "/lib",
config_settings = {
"//command_line_option:java_header_compilation": "true",
"//command_line_option:extra_toolchains": [Label(name + "/toolchain")],
},
)

def _test_no_turbine_direct_jvm_opts_by_default_impl(env, target):
header_action = env.expect.that_target(target).action_generating(
"{package}/lib{name}-hjar.jar",
)

header_action.argv().contains("{package}/{test_name}/turbine_direct.jar")
header_action.argv().not_contains(_TURBINE_DIRECT_JVM_OPT)

def _test_turbine_jvm_opts_do_not_apply_to_direct_header_compiler(name):
util.helper_target(
mock_java_toolchain,
name = name + "/toolchain",
header_compiler_direct = name + "/turbine_direct.jar",
turbine_jvm_opts = [_TURBINE_DIRECT_JVM_OPT],
)
util.helper_target(
java_library,
name = name + "/lib",
srcs = ["b.java"],
)

analysis_test(
name = name,
impl = _test_turbine_jvm_opts_do_not_apply_to_direct_header_compiler_impl,
target = name + "/lib",
config_settings = {
"//command_line_option:java_header_compilation": "true",
"//command_line_option:extra_toolchains": [Label(name + "/toolchain")],
},
)

def _test_turbine_jvm_opts_do_not_apply_to_direct_header_compiler_impl(env, target):
header_action = env.expect.that_target(target).action_generating(
"{package}/lib{name}-hjar.jar",
)

header_action.argv().contains("{package}/{test_name}/turbine_direct.jar")
header_action.argv().not_contains(_TURBINE_DIRECT_JVM_OPT)

def _test_turbine_direct_jvm_opts_reject_native_direct_header_compiler(name):
util.helper_target(
mock_java_toolchain,
name = name + "/toolchain",
header_compiler_direct = name + "/turbine_direct_graal",
turbine_direct_jvm_opts = [_TURBINE_DIRECT_JVM_OPT],
)

analysis_test(
name = name,
impl = _test_turbine_direct_jvm_opts_reject_native_direct_header_compiler_impl,
target = name + "/toolchain_java", # the underlying java_toolchain
expect_failure = True,
)

def _test_turbine_direct_jvm_opts_reject_native_direct_header_compiler_impl(env, target):
env.expect.that_target(target).failures().contains_predicate(
matching.contains(
"turbine_direct_jvm_opts requires header_compiler_direct to be a " +
"deploy jar (.jar)",
),
)

def _test_javabuilder_location_expansion_with_multiple_artifacts(name):
util.helper_target(
native.filegroup,
Expand Down Expand Up @@ -766,6 +884,10 @@ def java_toolchain_tests(name):
_test_java_compile_action_target_gets_javacopts_from_toolchain,
_test_java_compile_action_exec_gets_javacopts_from_toolchain,
_test_java_compile_action_uses_tool_specific_jvm_opts,
_test_turbine_direct_jvm_opts_for_direct_header_compiler,
_test_no_turbine_direct_jvm_opts_by_default,
_test_turbine_jvm_opts_do_not_apply_to_direct_header_compiler,
_test_turbine_direct_jvm_opts_reject_native_direct_header_compiler,
_test_javabuilder_location_expansion_with_multiple_artifacts,
_test_java_common_without_toolchain_type_fails,
_test_java_toolchain_flag_default,
Expand Down
17 changes: 17 additions & 0 deletions toolchains/default_java_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,22 @@ DEFAULT_JAVACOPTS = [
"-Xmaxwarns -1",
]

TURBINE_DIRECT_JAVA_FALLBACK_JVM_OPTS = select({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this makes sense - the header_compiler_direct is a public attribute and user-settable. Expecting the non-graal executable based on platform isn't correct.

"@bazel_tools//src/conditions:darwin_arm64": [],
"@bazel_tools//src/conditions:linux_x86_64": [],
"@bazel_tools//src/conditions:linux_aarch64": [],
"@bazel_tools//src/conditions:windows": [],
"//conditions:default": [
# TurbineDirect's Java fallback runs as java -jar and bundles
# protobuf, whose UnsafeUtil emits terminal-deprecation warnings on
# JDK 24+ unless this is set. Native turbine_direct_graal platforms
# intentionally get no JVM opts: they treat these values as Turbine
# command-line options, not JVM options.
# See https://github.com/protocolbuffers/protobuf/issues/20760.
"--sun-misc-unsafe-memory-access=allow",
],
})

# If this is changed, the docs for "{,tool_}java_language_version" also
# need to be updated in the Bazel user manual
_DEFAULT_JAVA_LANGUAGE_VERSION = "11"
Expand All @@ -89,6 +105,7 @@ _BASE_TOOLCHAIN_CONFIGURATION = dict(
# Turbine is not a worker and parallel GC is faster for short-lived programs.
"-XX:+UseParallelGC",
],
turbine_direct_jvm_opts = TURBINE_DIRECT_JAVA_FALLBACK_JVM_OPTS,
misc = DEFAULT_JAVACOPTS,
singlejar = Label("//toolchains:singlejar"),
# Code to enumerate target JVM boot classpath uses host JVM. Because
Expand Down