bazel
bazel copied to clipboard
Turbine mishandles --release flag in some cases
Description of the bug:
Turbine contains code that inspects the current JDK to determine where to look for class file signatures. I believe this is incorrect, since as I understand it, Turbine runs as a native Graal image which doesn't have these signatures.
The two places I noticed where Turbine looks at the running current JDK are here and here.
The second of those is only important if you're targeting pre-Java-12, but the former will cause Turbine to use JimageClassBinder.bindDefault to look up signatures if you pass it --release N where N is the version of the JDK running Turbine (21 in my case), and that will get you an error since jrt is not available on the native image.
I think the reason this code exists at all is that ct.sym doesn't contain signatures for the current JDK. As far as I can tell, that has changed as of Java 22, and ct.sym now contains signatures for the latest JDK version as well.
I think the jrt lookup doesn't work at all, so maybe it should be removed? From a user perspective, the thing that doesn't work is passing --release N if Turbine is running on Java N. For ct.sym files from Java 22+, it should work to look in ct.sym no matter what --release is set to, so that would at least fix this on the newest JDK.
In order to fix this for ct.sym files for older JDKs as well, I think it would be necessary to hand Turbine a reference to the module files, in addition to the reference to ct.sym it's already getting, since those ct.sym files don't contain signatures for the JDK they're from.
Which category does this issue belong to?
Java Rules
What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
Try to build a Java target with --release 21.
Which operating system are you running Bazel on?
MacOS x64
What is the output of bazel info release?
7.1.1
If bazel info release returns development version or (@non-git), tell us how you built Bazel.
No response
What's the output of git remote get-url origin; git rev-parse HEAD ?
No response
Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.
No response
Have you found anything relevant by searching the web?
Other work recently took place in this area in https://github.com/bazelbuild/bazel/pull/21610 and in https://github.com/bazelbuild/bazel/issues/20323
Any other information, logs, or outputs that you want to share?
No response
Thanks for the report, I agree we shouldn't be relying on those paths for the turbine native-image when --release is being used.
It's been a while since I looked at the status of jrt and Graal, but it looks like it has a AllowJRTFileSystem flag and some jrt support now, at least for reading from a separate JDK (but not the host runtime).
I think it would be necessary to hand Turbine a reference to the module files, in addition to the reference to ct.sym it's already getting, since those ct.sym files don't contain signatures for the JDK they're from.
I think the invocations are already passing both the 'bootclasspath' jar we generate and ct.sym, I wonder if it makes sense for it to recognize the case where ct.sym doesn't contain the desired --release and call back to using the 'bootclasspath' jar.
Thanks for responding.
Since the purpose of the --release flag is to allow building on a new JDK while targeting an old one, I think even an incomplete fix that only works for ct.sym from Java 22 would be useful. Fixing the issue when the runtime JDK is 21 or below could come later, when someone actually has that need?
I think a partial fix for Java 22 would be to simply swap the two lookups here https://github.com/google/turbine/blob/ff3b0f75b4df04b1dd04c2bd4f6934934049738c/java/com/google/turbine/main/Main.java#L304-L309, so we check ct.sym first, and only then check jrt if we're targeting the current JDK and didn't find anything in the sym file.
Would you accept a PR to that effect, or do you think another solution is better?
Looks like https://github.com/google/turbine/pull/320 made it into Turbine 0.6.0, which landed in Bazel in https://github.com/bazelbuild/bazel/commit/81117aabe22f615743a2fe01548f9764dcad8a63
Does that resolve this issue, or is there anything else we're waiting for?
@bazel-io flag
@hvadehra The issue I described should have been fixed, so IMO feel free to close.
@bazel-io fork 7.2.0
Looks like google/turbine#320 made it into Turbine 0.6.0, which landed in Bazel in 81117aa
Does that resolve this issue, or is there anything else we're waiting for?
Looks like that commit was accidentally reverted. I've sent #22301 to roll forward.
A fix for this issue has been included in Bazel 7.2.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.2.0rc1. Thanks!
Hi, this does not appear to be fixed in 7.2.0rc1 if the runtime JDK is 21 and the --release 21 javacopt is passed to a java_library.
In that case we get an error like:
ERROR: /Users/mattbrown/turbine-issue/BUILD.bazel:18:13: Compiling Java headers turbine-issue/libhello_lib-hjar.jar (1 source file) failed: (Exit 1): turbine_direct_graal failed: error executing Turbine command (from target //turbine-issue:hello_lib) external/remote_java_tools_darwin_arm64/java_tools/turbine_direct_graal '-Dturbine.ctSymPath=external/companyname_remotejdk_21_macos_aarch64/lib/ct.sym' --output ... (remaining 29 arguments skipped)
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
not a supported release: OptionalInt[21]
Usage: turbine [options]
...
The fix referenced above seems to only be for a runtime JDK of 22+:
Since the purpose of the
--releaseflag is to allow building on a new JDK while targeting an old one, I think even an incomplete fix that only works forct.symfrom Java 22 would be useful. Fixing the issue when the runtime JDK is 21 or below could come later, when someone actually has that need?
here is a minimal repro case, minus any WORKSPACE or toolchain setup: https://gist.github.com/mattnworb/b3043807d9da9bd9d6953938002c579d. There is no error when the --release flag in the java_library is --release 11 or --release 17, or when omitted.
@mattnworb Yes, before Java 22, ct.sym doesn't contain symbols for the current JDK, so you can't pass --release 21 to Java 21, because Turbine won't know where to find the signatures, it only knows how to look in ct.sym. The fix we implemented should be effective as of Java 22, so as of that version you should be able to use --release N on JDK N.
Fixing this for Java 21 requires a more complicated fix, since Turbine can't just look in ct.sym for that version.
If you don't want to work on that fix, consider upgrading the JDK you use for building to Java 22 instead. As long as you continue passing --release 21, the emitted code should stay compatible with Java 21.
I see, thanks. We are wrapping most of our usages of java_library in a macro where it is pretty simple for us to avoid setting a --release flag if it is the same value as our (default) toolchain version of 21, so the fix isn't too bad.