Skip to content
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

toolchain_vanilla works with locally defined jdk 11, but toolchain_java11 breaks the jdk compiler #14533

Closed
james-deee opened this issue Jan 9, 2022 · 16 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: support / not a bug (process)

Comments

@james-deee
Copy link

james-deee commented Jan 9, 2022

Description of the problem / feature request:

Building in Java 11 works using toolchain_vanilla, but changing to toolchain_java11 does not compile due to a JDK bug (that should be fixed). Does the toolchain_java11 not use a locally defined host_javabase and javabase?

I assume that the vanilla one is using the local, but does that change to remote if i use toolchain_java11? Is there any way to change this so that I can define the jdk to use in that toolchain?

We define things like this in our .bazelrc:

build --define=ABSOLUTE_JAVABASE=//tools:jdk11
build --javabase=//tools:jdk11
build --host_javabase=//tools:jdk11
build --java_toolchain=@bazel_tools//tools/jdk:toolchain_java11
build --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java11

The jdk11 we use is this (via an alias)

http_archive(
    name = "jdk11_linux",
    build_file_content = "java_runtime(name = 'runtime', srcs =  glob(['**']), visibility = ['//visibility:public'])",
    sha256 = "3b1c0c34be4c894e64135a454f2d5aaa4bd10aea04ec2fa0c0efe6bb26528e30",
    strip_prefix = "jdk-11.0.13+8",
    urls = ["https://github.com/adoptium/temurin11-binaries/releases/download/jdk-11.0.13%2B8/OpenJDK11U-jdk_x64_linux_hotspot_11.0.13_8.tar.gz"],
)

Feature requests: what underlying problem are you trying to solve with this feature?

Get our build to work.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I think that it might have to do with our own giant code base, so it's not easily reproducible that I've found.

What operating system are you running Bazel on?

Linux - Ubuntu 21.04

What's the output of bazel info release?

release 1.2.1

Have you found anything relevant by searching the web?

Yes, it looks like the bug we are hitting is a bug in the JDK: https://bugs.openjdk.java.net/browse/JDK-8210483

Any other information, logs, or outputs that you want to share?

(06:53:41) ERROR: XXX/BUILD:5:1: Building XXX-class.jar (2277 source files) and running annotation processors (AnnotationProcessorHider$AnnotationProcessor, AnnotationDependsOnProcessor) failed (Exit 1)
compiler message file broken: key=compiler.misc.msg.bug arguments=11.0.13, {1}, {2}, {3}, {4}, {5}, {6}, {7}
java.lang.AssertionError
	at jdk.compiler/com.sun.tools.javac.util.Assert.error(Assert.java:155)
	at jdk.compiler/com.sun.tools.javac.util.Assert.check(Assert.java:46)
	at jdk.compiler/com.sun.tools.javac.comp.DeferredAttr$2$1.setOverloadKind(
@james-deee james-deee changed the title 'toolchain_vanilla' works with locally defined jdk 11, but toolchain_java11 breaks the jdk compiler toolchain_vanilla works with locally defined jdk 11, but toolchain_java11 breaks the jdk compiler Jan 10, 2022
@cushon
Copy link
Contributor

cushon commented Jan 12, 2022

toolchain_java11 uses the JDK in the java_toolchain.java_runtime attribute, so I think it's ignoring the JDK configured with --javabase and fetching a remote one.

Bazel was still using JDK 11.0.6 until relatively recently: 698c17a

I think you might be encountering this JDK bug, which has the same stack trace, but wasn't fixed until 11.0.7: https://bugs.openjdk.java.net/browse/JDK-8213908

@comius
Copy link
Contributor

comius commented Jan 12, 2022

What's the output of bazel info release?
release 1.2.1

Have you considered upgrading to LTS release? It would be much easier to support those.

toolchain_java11 uses the JDK in the java_toolchain.java_runtime attribute, so I think it's ignoring the JDK configured with --javabase and fetching a remote one.

I think java_toolchain.java_runtime was added in Bazel release > 4.0.0. Can you provide output of bazel query @bazel_tools//tools/jdk:toolchain_java11 --output=build?

Does the toolchain_java11 not use a locally defined host_javabase and javabase?

I think Bazel 1.2.1 is using host_javabase during compilation. But the toolchain_java11 also defines overrides jdk.compile and java.compile modules, which might play a role here (and toolchain_vanilla doesn't do this).

Perhaps you can use the output of the query command above to define your own Java toolchain, that's between vanilla and java11? (On the other hand, this is going to be something you'll need to fix again on Bazel upgrade).

@james-deee
Copy link
Author

james-deee commented Jan 12, 2022

@comius and @cushon thank y'all for responding. And yes, I dont think we have java_toolchain.java_runtime available to us in our current version. We do plan on upgrading, but there's a lot of things we'd have to change along the way for that upgrade at the moment, so am trying to first move to Java11 on our current 1.2.1 version.

@comius here's the output of that command

java_toolchain(
    name = "toolchain_java11",
    bootclasspath = ["@bazel_tools//tools/jdk:platformclasspath"],
    forcibly_disable_header_compilation = False,
    genclass = ["@bazel_tools//tools/jdk:genclass"],
    generator_function = "default_java_toolchain",
    generator_location = "tools/jdk/BUILD:379",
    generator_name = "toolchain_java11",
    header_compiler = ["@bazel_tools//tools/jdk:turbine"],
    header_compiler_direct = ["@bazel_tools//tools/jdk:turbine_direct"],
    ijar = ["@bazel_tools//tools/jdk:ijar"],
    javabuilder = ["@bazel_tools//tools/jdk:javabuilder"],
    javac = ["@bazel_tools//tools/jdk:javac_jar"],
    javac_supports_workers = True,
    jvm_opts = [
        "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
        "--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
        "--patch-module=java.compiler=$(location @bazel_tools//tools/jdk:java_compiler_jar)",
        "--patch-module=jdk.compiler=$(location @bazel_tools//tools/jdk:jdk_compiler_jar)",
        "--add-opens=java.base/java.nio=ALL-UNNAMED",
        "--add-opens=java.base/java.lang=ALL-UNNAMED",
    ],
    misc = [
        "-XDskipDuplicateBridges=true",
        "-g",
        "-parameters",
    ],
    singlejar = ["@bazel_tools//tools/jdk:singlejar"],
    source_version = "11",
    tags = ["__JAVA_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__"],
    target_version = "11",
    tools = [
        "@bazel_tools//tools/jdk:java_compiler_jar",
        "@bazel_tools//tools/jdk:jdk_compiler_jar",
    ],
)

Here's the difference between the 2 toolchains:
image

What's interesting is that the vanilla one still appears to use those same tools. Is it the patch-module the thing you're referring to in the java11 one? I tried removing those, but then I get a weird dependency failure for an error-prone library we have.....and we already have the dependency defined for the java_library.

tools/plugins/helpers/GuiceInjectionFinder.java:21: error: [strict] Using type com.google.errorprone.matchers.Matcher from an indirect dependency (TOOL_INFO: "@com_google_errorprone_error_prone_check_api//:com_google_errorprone_error_prone_check_api"). See command below **
	private static final Matcher<VariableTree> IS_ASSISTED_PARAMETER =
	                     ^
 ** Please add the following dependencies: 
  @com_google_errorprone_error_prone_check_api//:com_google_errorprone_error_prone_check_api to //tools/plugins:helpers 
 ** You can use the following buildozer command: 
buildozer 'add deps @com_google_errorprone_error_prone_check_api//:com_google_errorprone_error_prone_check_api' //tools/plugins:helpers 

@james-deee
Copy link
Author

Or do I need to build up our own java_tools zip that we build on our own that uses the JDK we want?
https://github.com/bazelbuild/bazel/blob/1.2.1/src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE#L230

@cushon
Copy link
Contributor

cushon commented Jan 13, 2022

The non-vanilla toolchain depends on a custom version of javac, those --patch-module= flags are overriding the javac implementation in the JDK with the custom one from @bazel_tools//tools/jdk:jdk_compiler_jar.

The customized javac is relatively old, and doesn't have a fix for the crash in setOverloadKind.

One of the customizations in that javac version is a bug fix that is needed for Strict Java Deps to work, which is why you get the dependency failure with those flags removed.

The 'vanilla' toolchain works with a stock JDK 11 by disabling some of the features that depend on the modified version of javac, including Strict Java Deps.

As of JDK 13, Bazel no longer needs any customizations to javac, so we're not using the --patch-module= approach with newer JDK versions.

(I'm not sure what the best solution is for you here, I'm just trying to provide some context.)

@james-deee
Copy link
Author

Thanks @cushon for continuing to look.

One of the customizations in that javac version is a bug fix that is needed for Strict Java Deps to work, which is why you get the dependency failure with those flags removed.

I'm wondering if we could build our own version of this thing: https://github.com/bazelbuild/bazel/blob/1.2.1/src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE#L230

Using BOTH an updated JDK that we are using and the javac bug fix you mention here. Although, can you give me more insight into how that javac was "patched" with the bug fix? Is that something easily done?

I'd love a simpler answer than this, but just seeing if this is even an option to fix it.

@cushon
Copy link
Contributor

cushon commented Jan 13, 2022

I think the approach you describe is possible but not easy or well documented :(

Here are some breadcrumbs:

I'm wondering if we could build our own version of this thing: https://github.com/bazelbuild/bazel/blob/1.2.1/src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE#L230

There's some context about how to build java_tools.zip in bazelbuild/java_tools#43

can you give me more insight into how that javac was "patched" with the bug fix? Is that something easily done?

From #11262 (comment)

the Strict Java Deps implementation depends on a couple of javac bug fixes that are present in the vendored javac, but not in a stock OpenJDK 11 javac.

I think JDK-8214026 and JDK-8193277 are sufficient, and those are fixed in JDK 12 and 13 respectively. I tried using JDK 13 with your example and it allows those targets to build:

So you could apply those patches to JDK 11 and build a custom JDK, or apply them to javac and use --patch-module= to override the javac version.

The sources for the custom JDK 11 javac Bazel is using are here: https://github.com/google/error-prone-javac/commits/jdk-11.0.3-1

The incantation to build those sources is something like

$ ant \
-buildfile make/langtools/build.xml \
-Dboot.java.home=<path to JDK 10> \
-Dlangtools.jdk.home=<path to JDK 11> \
clean build

And then to package that into the jdk_compiler.jar and java_compiler.jar artifacts:

(cd build/langtools/modules/java.compiler/; jar cf ../../../../java_compiler.jar .)
(cd build/langtools/modules/jdk.compiler/; jar cf ../../../../jdk_compiler.jar .)

(cd src/java.compiler/share/classes/; jar cf ../../../../java_compiler-src.jar .)
(cd src/jdk.compiler/share/classes/; jar cf ../../../../jdk_compiler-src.jar .)

@cushon
Copy link
Contributor

cushon commented Jan 13, 2022

Thinking about this a little more, I'm going to see whether it's possible to backport those JDK patches into JDK 11u, which might allow us to drop the --patch-module= stuff and remove the need for the custom javac.

@james-deee
Copy link
Author

That would be amazing @cushon

@comius comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Jan 14, 2022
@cushon cushon self-assigned this Jan 18, 2022
@cushon
Copy link
Contributor

cushon commented Jan 18, 2022

I backported those fixes to 11u. The next JDK 11u release is scheduled for April 19, 2022, so I set a reminder to follow up then

@james-deee
Copy link
Author

Amazing. Thanks @cushon. We'll check back then as well and try with an upgraded jdk. I assume it still means we would make our own toolchain, but just remove the patch-modules?

@cushon
Copy link
Contributor

cushon commented Jan 18, 2022

I'm going to look at removing the --patch-module= flags from the built-in toolchains as well when that happens.

But yes, if you're still on an older version of Bazel at that point, you could update to the latest JDK 11u release and then define custom toolchains that remove all of the --patch-module= flags

cushon added a commit to cushon/bazel that referenced this issue Apr 20, 2022
The javac changes that Bazel depends on are now available in the latest
JDK 11 update release.

bazelbuild#14533
@james-deee
Copy link
Author

@cushon Thanks for all of your work on this. I ran our build with a version of 11.0.15 and it worked!! We are hitting one more issue that I'm not sure if you have any insight into that could help.

Just a recap, we are still on Bazel 1.2.1. We created our own java toolchain to remove the --patch-module. We also are not specifying target/source and instead using --release 11 as a javacopts:

java_toolchain(
    name = "redfin_toolchain_java11",
    bootclasspath = ["@bazel_tools//tools/jdk:platformclasspath"],
    forcibly_disable_header_compilation = False,
    genclass = ["@bazel_tools//tools/jdk:genclass"],
    header_compiler = ["@bazel_tools//tools/jdk:turbine"],
    header_compiler_direct = ["@bazel_tools//tools/jdk:turbine_direct"],
    ijar = ["@bazel_tools//tools/jdk:ijar"],
    javabuilder = ["@bazel_tools//tools/jdk:javabuilder"],
    javac = ["@bazel_tools//tools/jdk:javac_jar"],
    javac_supports_workers = True,
    javacopts = [
        "--release 11",
    ],
    jvm_opts = [
        "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
        "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
        "--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
        "--add-opens=java.base/java.nio=ALL-UNNAMED",
        "--add-opens=java.base/java.lang=ALL-UNNAMED",
    ],
    misc = [
        "-XDskipDuplicateBridges=true",
        "-g",
        "-parameters",
    ],
    singlejar = ["@bazel_tools//tools/jdk:singlejar"],
    tools = [
        "@bazel_tools//tools/jdk:java_compiler_jar",
        "@bazel_tools//tools/jdk:jdk_compiler_jar",
    ],
)

So this all works great! But when I try to use --release 8 to build, we are getting a failure when it attempts to build com_google_protobuf.

http_archive(
    name = "com_google_protobuf",
    sha256 = "cfcba2df10feec52a84208693937c17a4b5df7775e1635c1e3baffc487b24c9b",
    strip_prefix = "protobuf-3.9.2",
    urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.9.2.zip"],
)

We get an issue with the now private Unsafe

(09:08:48) ERROR: /home/jd/.cache/bazel/_bazel_jd/3dd34ff3e780938fd1411f3fe0284fc3/external/com_google_protobuf/BUILD:618:1: Compiling Java headers external/com_google_protobuf/libprotobuf_java-hjar.jar (78 source files, 1 source jar) failed (Exit 1) java failed: error executing command external/jdk11_linux/bin/java -Xverify:none '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED' ... (remaining 112 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
external/com_google_protobuf/java/core/src/main/java/com/google/protobuf/UnsafeUtil.java:411: error: could not resolve sun.misc.Unsafe
    MemoryAccessor(sun.misc.Unsafe unsafe) {
                   ^
external/com_google_protobuf/java/core/src/main/java/com/google/protobuf/UnsafeUtil.java:409: error: could not resolve sun.misc.Unsafe
    sun.misc.Unsafe unsafe;
....

I saw another ticket over there, but for java 9: protocolbuffers/protobuf#4257

Is there something we can do to get this to compile? Something around overriding the protobuf java_toolchain? Or am I doing something wrong with the above redfin_toolchain_java11?

@cushon
Copy link
Contributor

cushon commented Apr 21, 2022

@james-deee glad to hear 11.0.15 is (mostly) working!

You can also remove the entries from tools, they were only used by the --patch-module= flags that aren't being passed anymore:

    tools = [
        "@bazel_tools//tools/jdk:java_compiler_jar",
        "@bazel_tools//tools/jdk:jdk_compiler_jar",
    ],

The error: could not resolve sun.misc.Unsafe with --release 8 is because of of https://bugs.openjdk.java.net/browse/JDK-8206937. The --release flag doesn't allow access to internal APIs like sun.misc.Unsafe. I don't have a workaround other than using -source 8 -target 8 -bootclasspath instead of --release. That bug is the main reason Bazel isn't using --release in the default toolchains.

bazel-io pushed a commit that referenced this issue Apr 21, 2022
The new binaries are:

```
e064b61d93304012351242bf0823c6a2e41d9e28add7ea7f05378b7243d34247 https://cdn.azul.com/zulu/bin/zulu11.56.19-ca-jdk11.0.15-linux_x64.tar.gz
fc7c41a0005180d4ca471c90d01e049469e0614cf774566d4cf383caa29d1a97 https://cdn.azul.com/zulu-embedded/bin/zulu11.56.19-ca-jdk11.0.15-linux_aarch64.tar.gz
9750e11721282a9afd18a07743f19c699b2b71ce20d02f3f0a906088b9ae6d9a https://github.com/adoptium/temurin11-binaries/releases/download/jdk-11.0.14.1+1/OpenJDK11U-jdk_ppc64le_linux_hotspot_11.0.14.1_1.tar.gz
79a27a4dc23dff38a5c21e5ba9b7efcf0aa5e14ace1a3b19bec53e255c487521 https://github.com/adoptium/temurin11-binaries/releases/download/jdk-11.0.14.1+1/OpenJDK11U-jdk_s390x_linux_hotspot_11.0.14.1_1.tar.gz
2614e5c5de8e989d4d81759de4c333aa5b867b17ab9ee78754309ba65c7f6f55 https://cdn.azul.com/zulu/bin/zulu11.56.19-ca-jdk11.0.15-macosx_x64.tar.gz
6bb0d2c6e8a29dcd9c577bbb2986352ba12481a9549ac2c0bcfd00ed60e538d2 https://cdn.azul.com/zulu/bin/zulu11.56.19-ca-jdk11.0.15-macosx_aarch64.tar.gz
a106c77389a63b6bd963a087d5f01171bd32aa3ee7377ecef87531390dcb9050 https://cdn.azul.com/zulu/bin/zulu11.56.19-ca-jdk11.0.15-win_x64.zip
```

This is a step towards #14533

Closes #15302.

PiperOrigin-RevId: 443381025
cushon added a commit to cushon/bazel that referenced this issue Apr 21, 2022
The javac changes that Bazel depends on are now available in the latest
JDK 11 update release.

bazelbuild#14533
cushon added a commit to cushon/bazel that referenced this issue Apr 22, 2022
The javac changes that Bazel depends on are now available in the latest
JDK 11 update release.

bazelbuild#14533
bazel-io pushed a commit that referenced this issue Apr 22, 2022
The javac changes that Bazel depends on are now available in the latest JDK 11 update release.

#14533

Closes #15303.

PiperOrigin-RevId: 443698942
cushon added a commit to cushon/bazel that referenced this issue Apr 22, 2022
@james-deee
Copy link
Author

@cushon Just wanted to follow up and thank you for all of your time and patience :)

The 11.0.15 release of Java fixes everything for us (with the removal of --patch-module). Thank you!!!!

bazel-io pushed a commit that referenced this issue Apr 26, 2022
This is the third_party part of 18c17da

#14533

Partial commit for third_party/*, see #15322.

Signed-off-by: Yun Peng <pcloudy@google.com>
cushon added a commit to cushon/bazel that referenced this issue Apr 26, 2022
This is the third_party part of
bazelbuild#15319

bazelbuild#14533
bazel-io pushed a commit that referenced this issue Apr 26, 2022
Follow-up to #15303

#14533

Closes #15319.

PiperOrigin-RevId: 444583929
cushon added a commit to cushon/bazel that referenced this issue Apr 27, 2022
This is the third_party part of
bazelbuild#15319

bazelbuild#14533
bazel-io pushed a commit that referenced this issue Apr 28, 2022
This is the third_party part of
#15319

#14533

Partial commit for third_party/*, see #15347.

Signed-off-by: Sunil Gowroji <sgowroji@google.com>
@ghost
Copy link

ghost commented May 5, 2022

Is this (removing the langtools dependency) something worth bringing back to Bazel 5.2.0 (or the next 5.x release), or would raising the minimum requirement of OpenJDK 11.15 be considered a breaking change? (This is something I'm looking into backporting to 5.1.1/5.2.0 for our internal builds.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: support / not a bug (process)
Projects
None yet
Development

No branches or pull requests

4 participants