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.

Sign up for GitHub

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

Jump to bottom

Overriding javac is broken #11262

Closed
Jonpez2 opened this issue Apr 30, 2020 · 14 comments
Closed

Overriding javac is broken #11262

Jonpez2 opened this issue Apr 30, 2020 · 14 comments
Labels
area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules type: bug

Comments

@Jonpez2
Copy link
Contributor

Jonpez2 commented Apr 30, 2020

Description of the problem / feature request:

Attempting to override the javac version to something newer via the advertised mechanisms does not work.

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

I am trying to avoid this bug in javac - https://bugs.openjdk.java.net/browse/JDK-8210483

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

This repo provides a simple repro of the two different ways we have attempted to solve this, and the problems we ran into with each route. Massive hat tip to https://github.com/AngusDavis, who has been super helpful and wrote the basic repro.

https://github.com/Jonpez2/bazel-custom-java

If you look at the .bazelrc in that repo it has simple instructions on how to repro the problems.

What operating system are you running Bazel on?

uname -a
Linux 4.9.0-12-amd64 #1 SMP Debian 4.9.210-1 (2020-01-20) x86_64 GNU/Linux

What's the output of bazel info release?

release 3.1.0

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

https://github.com/Jonpez2/bazel-custom-java
e0d0b70396c921cdeabf7c54921f347951cbc2bd
e0d0b70396c921cdeabf7c54921f347951cbc2bd

Have you found anything relevant by searching the web?

This google-groups thread summarizes our work so far
https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!topic/bazel-discuss/6wKwd3Xdhk0
@jin jin added team-Rules-Java Issues for Java rules untriaged labels May 3, 2020
@ulfjack
Copy link
Contributor

ulfjack commented May 8, 2020

I can reproduce the issue. I say 'the issue' even though it looks more like multiple separate issues. There are three different possible results based on different flag combinations:

  1. JavaBuildTest fails to compile with errors involving var, which presumably is a Java 10 feature
  2. JavaBuildTest fails to compile with an error involving OkHttpClient, saying that it comes from an indirect dependency (even though it clearly comes from a direct dependency)
  3. JdkBug fails to compile with a compiler assertion error due to the aforementioned Javac bug

1 is not a bug, but simply the combination of a pre-10 compiler (or compiler setting) with code that requires Java 10.

I think 2 is a bug in the Java tools, and it seems to be primarily triggered by the custom OpenJDK 11. 3 is clearly an upstream bug, and the use of the custom OpenJDK 11 in combination with pulling in the latest Java tools release seems to avoid hitting it. Unfortunately, that's exactly the combination that runs into issue 2.

@cushon, any ideas?

@Jonpez2
Copy link
Contributor Author

Jonpez2 commented May 12, 2020

Thanks @ulfjack. Is there anything else I can supply here?

@cushon
Copy link
Contributor

cushon commented May 17, 2020

JavaBuildTest fails to compile with an error involving OkHttpClient, saying that it comes from an indirect dependency (even though it clearly comes from a direct dependency)

That error is happening because 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:

http_archive(
    name = "openjdk13_linux_archive",
    build_file_content = "java_runtime(name = 'runtime', srcs =  glob(['**']), visibility = ['//visibility:public'])",
    sha256 = "3e5dc530c9e9466fc76b860d924876ea93bbdae80a12d47649e11dda3dbd9bac",
    strip_prefix = "zulu13.31.11-ca-jdk13.0.3-linux_x64",
    urls = [
        "https://cdn.azul.com/zulu/bin/zulu13.31.11-ca-jdk13.0.3-linux_x64.tar.gz",
    ],
)
$ bazel build --host_javabase=@openjdk11_linux_archive//:runtime \
  --javabase=@openjdk11_linux_archive//:runtime \
  --java_toolchain=//tools/toolchains:jdk11_toolchain \
  --host_java_toolchain=//tools/toolchains:jdk11_toolchain \
  //java:all

...
JavaBuildTest.java:14: error: [strict] Using type okhttp3.OkHttpClient from an indirect dependency (TOOL_INFO: "~.cache/bazel/_bazel_cushon/29b5745a2b592ce57ecc0334c3961675/execroot/__main__/bazel-out/k8-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/okhttp3/okhttp/3.14.3/okhttp-3.14.3.jar").
    OkHttpClient ok = new OkHttpClient.Builder()
    ^
$ bazel build --host_javabase=@openjdk13_linux_archive//:runtime \
  --javabase=@openjdk13_linux_archive//:runtime \
  --java_toolchain=//tools/toolchains:jdk11_toolchain \
  --host_java_toolchain=//tools/toolchains:jdk11_toolchain \
  //java:all
...
INFO: Build completed successfully

I hope this is helpful in building an understanding of what's going on, but I'm not sure that using JDK 13 is going to be a practical solution in the short term. It works for those examples, but you'll probably run into other incompatibilities trying to use it for real (e.g. the version of Error Prone that's shipping in Bazel doesn't support JDK 13).

@Jonpez2
Copy link
Contributor Author

Jonpez2 commented May 17, 2020 via email

@ulfjack
Copy link
Contributor

ulfjack commented May 18, 2020

The Java toolchain used in Bazel is a wrapper around the underlying Javac in the JDK. Note that the wrapper already lives in a separate repository with periodic releases (at least, that's the intent), it just uses the Javac in the host JDK. The underlying problem here is that the wrapper uses internal APIs that are not guaranteed to be stable, and so the wrapper has to be updated for every major JDK release.

In this specific case, a change in JDK 11 caused the wrapper to misbehave - @cushon fixed that problem in the upstream JDK, but the fix hasn't made it into a minor release of JDK 11 (or maybe it has by now? - I'm not sure how the JDK releases work). It looks like that fix did make it into the Zulu JDK that Bazel is pulling (but that doesn't have the Javac fix? Confusing!).

I don't think this is related to Bazel being implemented in Java.

One option is to use the vanilla Java builder, which has fewer features, but should be compatible with future JDK releases.

I'm not sure if there is any long-term plan to address this inherent discrepancy. I don't suppose we can ask Oracle to maintain Javabuilder for us?

@Jonpez2
Copy link
Contributor Author

Jonpez2 commented May 18, 2020 via email

@cushon
Copy link
Contributor

cushon commented May 18, 2020

cc @lberki re: long term plans and timelines for java_tools releases

it just uses the Javac in the host JDK

It uses the vendored javac version I mentioned in #11262 (comment), see e.g. 10a4261.

The repro is using a stock javac by disabling the configuration that normally causes Bazel to use the vendored JDK: https://github.com/Jonpez2/bazel-custom-java/blob/2808c253508d9420a83cb72ec4ff422d29f69ff9/tools/toolchains/BUILD#L22-L24

@davido davido mentioned this issue May 23, 2020
Closed
@davido
Copy link
Contributor

davido commented May 28, 2020 •
edited
Loading

Thanks @cushon, Error Prone@Head supports JDK 14 now.

I uploaded this PR to bump EP release in Bazel (from unknown version 2.3.2-SNAPSHOT) to EP@Head.

Based on the above EP upgrade to EP@HEAD with JDK 14 support, I uploaded a PR for Java toolchain 14 support here.

I have built and conducted java_tools release for JDK 14 toolchain here (currently only Linux version).

Example project that demonstrates that Java 14 syntax class can be built out of the box is here:

  $ bazel version
Starting local Bazel server and connecting to it...
Build label: 3.2.0

  $ bazel run java:Javac14Example 
INFO: Analyzed target //java:Javac14Example (18 packages loaded, 707 targets configured).
INFO: Found 1 target...
INFO: Deleting stale sandbox base /home/davido/.cache/bazel/_bazel_davido/1790aabdce25864b2f68511bd57514be/sandbox
Target //java:Javac14Example up-to-date:
  bazel-bin/java/Javac14Example.jar
  bazel-bin/java/Javac14Example
INFO: Elapsed time: 9.035s, Critical Path: 0.73s
INFO: 0 processes.
INFO: Build completed successfully, 2 total actions
INFO: Build completed successfully, 2 total actions
0

The real world example building with JDK 14 toolchain, Gerrit Code Review built on real CI (with installed vanilla Bazel 3.1.0 only) is here.

@Jonpez2
Copy link
Contributor Author

Jonpez2 commented May 29, 2020 via email

@davido
Copy link
Contributor

davido commented Jun 6, 2020 •
edited
Loading

Now, that Bazel supports Java 14 toolchain with my PR and custom java_tools release, it turns out that some tests related to regex processing for medium size project (Gerrit) are failing: [1].

It seems, that Pattern.compile() behavior has changed between JDK 8/11 and JDK 14/15:

Example.java

class Example {
  public static void main(String[] args) {
    java.util.regex.Pattern.compile("(bugs{+#?)(d+)");
  }
}

Java 8/11 produces different exception on wrong pattern compared to JDK 14/15:

  $ java -fullversion
  openjdk full version "11.0.7+10-suse-1.1-x8664"
  $ java -cp . Example

Exception in thread "main" java.util.regex.PatternSyntaxException: Illegal repetition near index 4
(bugs{+#?)(d+)
    ^
	at java.base/java.util.regex.Pattern.error(Pattern.java:2027)

While JDK 14/15 produce

  $ java -fullversion
  openjdk full version "15-ea+26-1287"

  $ java -cp . Example
  Exception in thread "main" java.util.regex.PatternSyntaxException: Illegal repetition near index 6
(bugs{+#?)(d+)
      ^
	at java.base/java.util.regex.Pattern.error(Pattern.java:2028)

Reproducer in gerrit, with: [1] applied:

  $ bazel test \
  --test_filter=com.google.gerrit.server.project.ProjectConfigTest.readCommentLinkInvalidPattern \
  //javatests/com/google/gerrit/server:server_tests
  [...]
1) readCommentLinkInvalidPattern(com.google.gerrit.server.project.ProjectConfigTest)
value of:
    getValidationErrors().onlyElement()
expected:
    …petition near index 4
    (bugs{+#?)(d+)
        ^]
but was:
    …petition near index 6
    (bugs{+#?)(d+)
          ^]
	at com.google.gerrit.server.project.ProjectConfigTest.readCommentLinkInvalidPattern(ProjectConfigTest.java:573)

FAILURES!!!
Tests run: 1,  Failures: 1

//CC @msohn.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/269382

@Jonpez2
Copy link
Contributor Author

Jonpez2 commented Jun 22, 2020 via email

@Jonpez2
Copy link
Contributor Author

Jonpez2 commented Jul 24, 2020 via email

@lberki lberki added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Oct 5, 2020
@lberki
Copy link
Contributor

lberki commented Oct 5, 2020

@comius , another use case for your platformization work?

@comius comius added the area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository label Nov 21, 2020
@comius
Copy link
Contributor

comius commented Jan 27, 2021

I still have my meta-question about how this will work in the long term.
Who should I ask that of, please? Sorry to be so tediously insistent after
you've done so much.

With Java Platformization work https://docs.google.com/document/d/1MVbBxbKVKRJJY7DnkptHpvz7ROhyAYy4a-TZ-n7Q0r4/edit a lot of issues like this should be resolved.

java_tools has been refactored, so that it doesn't include toolchain definitions. It only contains the tools that are needed by the toolchain. This makes it more usable in different Bazel versions, starting after 4.0.0 release. Also a pure Java part and source part of java_tools has been split into a separate repo. Precompiled parts with ijar and singlejar are now really small, and not needed if one has a C compiler.

Toolchain definitions are currently part of Bazel, @bazel_tools repository. This should make it way easier to import a new version JDK (e.g 16), because there's no need to fix java_tools. In fact it should be possible to import new JDK by adding couple of rules in the user's repository.

In the future when Java rules are rewritten in Starlark, toolchain definitions will be moved into rules_java repository.

About javac overrides. Java_toolchain rule has been extended with java_runtime parameter. This way a fixed/pinned JDK is used for compilation. Changing it requires toolchain definition, where I'd say it's user's responsibility to either use vanilla configuration or have a compatible combination of javac and JDK. Or even fix Bazel's javac patches.

Additional information is on https://docs.bazel.build/versions/master/bazel-and-java.html

@comius comius closed this as completed Jan 27, 2021
@cushon cushon mentioned this issue Dec 25, 2021
Closed
@cushon cushon mentioned this issue Jan 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@davido @jin @cushon @comius @lberki @Jonpez2 @ulfjack