-
Notifications
You must be signed in to change notification settings - Fork 101
Silence protobuf sun.misc.Unsafe warnings from the direct header comp… #375
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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( | ||
| 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): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
@@ -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: | ||
|
|
@@ -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( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency lets call this |
||
| 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 = """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,22 @@ DEFAULT_JAVACOPTS = [ | |
| "-Xmaxwarns -1", | ||
| ] | ||
|
|
||
| TURBINE_DIRECT_JAVA_FALLBACK_JVM_OPTS = select({ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this makes sense - the |
||
| "@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" | ||
|
|
@@ -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 | ||
|
|
||
There was a problem hiding this comment.
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.