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

Upgrade javac to JDK10 to support classfile version 54 #860

Closed
don-vip opened this issue Dec 12, 2017 · 19 comments
Closed

Upgrade javac to JDK10 to support classfile version 54 #860

don-vip opened this issue Dec 12, 2017 · 19 comments

Comments

@don-vip
Copy link
Contributor

don-vip commented Dec 12, 2017

When running error-prone with JDK 10-ea+35 we get the following compilation warnings:

[javac] warning: /modules/java.base/java/lang/Deprecated.class: major version 54 is newer than 53, the highest major version supported by this compiler.
[javac]   It is recommended that the compiler be upgraded.

for each class of the JDK. This is due to JDK-8188870: Bump classfile version number to 54.

With the new release cycle, the compiler will have to be updated every 6 months to support the new classfile version.

Maybe you could directly switch to JDK11 as classfile version 55 is coming soon, see JDK-8191913: Bump classfile version number to 55.

don-vip referenced this issue Jan 10, 2018
RELNOTES: Update to javac 9+181-r4173-1

MOE_MIGRATED_REVID=180950287
@eaftan
Copy link
Contributor

eaftan commented Feb 6, 2018

We're trying to figure out our strategy for these short-term support Java releases. The problem is that shipping a new javac entails running the compiler on a newer JDK. Do most Error Prone users want to hop on that every-6-month JDK upgrade treadmill?

Thoughts?

@don-vip
Copy link
Contributor Author

don-vip commented Feb 6, 2018

Maybe we could have the choice? I don't know how feasible it would be to have javac9 as default compiler but allowing developers to override it with a newer javac10/11/etc. Maven dependency in their pom.xml.

@msridhar
Copy link
Contributor

msridhar commented Feb 6, 2018 via email

@Stephan202
Copy link
Contributor

Our company is on JDK 8 still, but we'll move to JDK 9 (or 10) as soon as is feasible. I'd rather not get into the situation where internal projects have to either be held back or do without Error Prone altogether.

I guess it comes down to how frequently the internal APIs against which EP integrates change in practice?

@PhilippWendler
Copy link

Having to upgrade the JDK would be a major obstacle for us. We would probably stop upgrading error-prone if this becomes a requirement. The reason is that many LTS Linux distribution will probably not ship these new JDK versions anytime soon, and we do not want to tell the developers and users of our project that they have to install a JDK manually.

@jbduncan
Copy link
Contributor

I believe I'm hitting this same issue as well over at junit-team/junit5#1266.

JUnit 5 is kind of uncommon in that it's always tested against the bleeding-edge version of the Java compiler and uses the -Werror flag, so whereas it currently works without problems with JDK 9, if I try to introduce error-prone for the part of the build that runs JDK 10, it just errors out with messages like:

warning: /8769/java/lang/Void.sig: major version 54 is newer than 53, the highest major version supported by this compiler.
  It is recommended that the compiler be upgraded.

If the error-prone team would be interested in a reproducible example, one can try the following commands:

  1. git clone https://github.com/jbduncan/junit5/tree/error-prone-investigation-4
  2. git checkout 3ff428cb6d045aa81f6812b767c05d7142ad78b9
  3. ./gradlew clean build

jbduncan added a commit to jbduncan/junit5 that referenced this issue Feb 25, 2018
Currently, it doesn't run on Java 10. See
google/error-prone#860
jbduncan added a commit to jbduncan/junit5 that referenced this issue Mar 1, 2018
Currently, it doesn't run on Java 10. See
google/error-prone#860
floscher pushed a commit to floscher/josm that referenced this issue Mar 4, 2018
@tbroyer
Copy link
Contributor

tbroyer commented Mar 24, 2018

Fwiw, using Error Prone as a JavaC Plugin (-Xplugin:ErrorProne) works with JDK 10; but it then have other problems (see #974)

simon04 pushed a commit to JOSM/josm that referenced this issue Mar 30, 2018
dmitry-timofeev added a commit to exonum/exonum-java-binding that referenced this issue Jun 7, 2018
Error-prone does not support Java 10 yet. A setup to activate
it conditionally would be too verbose (a profile activation
in each sub-module, because it does not work for
<build><pluginManagement>...).

See google/error-prone#860
@shakuzen
Copy link

I apologize for not knowing more of the intimate details, but as far as I know, Error Prone has relied on the Java 9 compiler for more than 3 years at this point (since cfbbe69 released in version 2.0.2) without requiring JDK 9 be used to compile projects using Error Prone.

Do most Error Prone users want to hop on that every-6-month JDK upgrade treadmill?

Why would this be required going forward when it hasn't been required while Error Prone has internally used JDK9? Could Error Prone not do the same for Java 10 and 11ea? It seems ideal, if possible, to allow users to use anything from JDK 8-11 in their projects now.


The reason is that many LTS Linux distribution will probably not ship these new JDK versions anytime soon

This is at least not true with Ubuntu: https://wiki.ubuntu.com/BionicBeaver/ReleaseNotes#OpenJDK

As of 18.04 release, OpenJDK 10 is the default JRE/JDK. Once OpenJDK 11 reaches GA in September 2018, it will become the default in 18.04.

@tbroyer
Copy link
Contributor

tbroyer commented Jun 11, 2018

I apologize for not knowing more of the intimate details, but as far as I know, Error Prone has relied on the Java 9 compiler for more than 3 years at this point (since cfbbe69 released in version 2.0.2) without requiring JDK 9 be used to compile projects using Error Prone.

Do most Error Prone users want to hop on that every-6-month JDK upgrade treadmill?

Why would this be required going forward when it hasn't been required while Error Prone has internally used JDK9? Could Error Prone not do the same for Java 10 and 11ea? It seems ideal, if possible, to allow users to use anything from JDK 8-11 in their projects now.

The version of Javac used in Error Prone has been patched to be compilable with JDK 8, so that it can run on Java 8. I guess this is no longer possible (and IIUC, the version of Javac used in Error Prone has been "cut"/forked from the JDK before they added JPMS, and not upgraded since then –besides backports of some bug fixes– also because of that).

In the mean time, using Error Prone as a Javac plugin seems to Just Work™ (disclaimer: I haven't tried using var, or JPMS).

For example, with Maven (in a profile triggered when using JDK 9+, because it won't work with JDK 8; use the plexus-compiler-javac-errorprone with JDK 8):

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-compiler-plugin</artifactId>
  <version>3.7.0</version>
  <configuration>
    <compilerArgs>
      <!-- XXX: all Error Prone args should come in a single argument -->
      <arg>-Xplugin:ErrorProne -XepDisableWarningsInGeneratedCode</arg>
    </compilerArgs>
    <annotationProcessorPaths>
      <path>
        <groupId>com.google.errorprone</groupId>
        <artifactId>error_prone_core</artifactId>
        <version>2.3.1</version>
      </path>
    </annotationProcessorPaths>
  </configuration>
</plugin>

(note that this will also put com.google.errorprone:javac in the processor path, while not needed, but annotationProcessorPaths doesn't allow exclusions; this shouldn't be a problem in practice though)

With Gradle, you can use the net.ltgt.errorprone-javacplugin plugin.

@eaftan Should the documentation be updated to recommend -Xplugin:ErrorProne with JDK9+? Should there be new releases/artifacts without the com.google.errorprone:javac dependency? (or marked as <optional>true</optional>, which is almost equivalent)

PhilippWendler added a commit to sosy-lab/java-common-lib that referenced this issue Jun 12, 2018
This is the recommended way in the future, but not supported on Java 8,
cf. google/error-prone#860

Also move all the special error-prone options for this project
to build.xml to allow reuse of build-compile.xml across projects.
dbeyer-bot pushed a commit to sosy-lab/cpachecker that referenced this issue Jun 12, 2018
This is the recommended way in the future, but not supported on Java 8,
cf. google/error-prone#860

Also move all the special error-prone options for this project
to build.xml to allow reuse of build-compile.xml across projects.

git-svn-id: https://svn.sosy-lab.org/software/cpachecker/trunk@28304 4712c6d2-40bb-43ae-aa4b-fec3f1bdfe4c
vitvakatu pushed a commit to exonum/exonum-java-binding that referenced this issue Jun 19, 2018
* Add third party licenses for direct Rust dependencies [ECR-1502]. (#277)

* Update the project dependencies [ECR-1521, ECR-1518]:

- Add versions plugin
- Update the plugins that we use, specify the versions
  in the pluginManagement.
- Update the versions of dependencies.
- Disable Powermock tests until the library is updated.
- Verify the project builds on 10, run tests with openjdk-10 on the CI server.
- Check for updates on the CI server.

* Use the default Java compiler instead of error-prone:

Error-prone does not support Java 10 yet. A setup to activate
it conditionally would be too verbose (a profile activation
in each sub-module, because it does not work for
<build><pluginManagement>...).

See google/error-prone#860

* Do not install RocksDb — prevents SIGILL on older systems.

On some systems with older CPUs our build produces an executable
with an illegal instruction for those processors.

The problem needs further investigation, but if we do not use
the system version of RocksDb, it goes away. As I remember
(have no stack at the moment), the illegal instruction appears
somewhere in RocksDb initialization.

See: ECR-1558

* Upgrade to Exonum 0.8 [ECR-1620]

* Improve the documentation of a test. (#284)

* Change Java serialization to protobuf and use owner’s public key as wallet id [ECR-1493, ECR-1494]

  - Change Java Serialization of transactions and wallets to protobuf.
  - Wallet String name is changed to the  wallet owner’s public key.
  - Fix a bug in send money transaction when a transaction with the sender 
  equal to the receiver resulted in doubling the balance of that wallet.

PR: #272

* Messages payload length fix [ECR-1624]:

'payload length' field in the message header used to be equal to 
'body' length in bytes, while it should be equal to the size 
of the whole message (header + body + signature).

Also, improve the tests and exception messages.

PR: #285

* Fix the service API path: (#288)

The service API path must start with a slash 
to be correctly mounted.

* Fix the transaction location URL [ECR-1648] (#293).

The base path of explorer was changed in Exonum 0.7 
from ""/api/system/v1" to "/api/explorer/v1/".

* Add Javadoc plugin [ECR-1510] (#290):

Javadoc generation does not happen during normal builds 
and must be triggered manually. JARs with the Javadoc 
for all modules can be generated with 

$ mvn javadoc:jar

* Fix build on Travis CI [ECR-1658] (#292):

* Build the dependencies instead of using installed in the system
 to prevent illegal instructions appearing in our artefacts (the actual cause
 is still unknown).
* Disable caching of rust/target for it takes at least 2.5G, 
which is too much for Travis to archive and upload.

* Add an Exonum App [ECR-869, ECR-867] (#238):

Implemented JavaServiceRuntime abstraction around JVM 
to configure and run Java services. 

JavaServiceRuntime is a ServiceFactory, allowing to configure
Java services in the same way as any other Exonum service.

Also includes EJB App - a Rust application to configure and run
an Exonum node with a specific Java service.

* Add LICENSE and license headers in every source file [ECR-1499] (#295):

Also added a script generating a text file with third-party licenses 
of Rust dependencies:

./generate_licenses.sh

* Prepare the build configuration for a release [ECR-1526] (#298):

Add required plugins to prepare artefacts for a publication 
to the repository.

* Update native-proxies branch to Exonum 0.8.1 [ECR-1157] (#299).

* Fix NodeProxy so that it can create a View

* Remove 'provided' scope of the system dependencies of a service: (#301)

Until we ship an app that includes these dependencies,
they must be available on the classpath of the service (the only
configuration option that we will have at 0.1 release).

* Remove unused imports.

* Use the Java home that Maven reports in the scripts setting LD_LIBRARY_PATH

* Fix the Exception class that we use in tests:

The mocked methods can not throw checked exceptions.

* Skip all tests until the bug with loading shared libraries is fixed.

* Add a tutorial describing how to launch a node with a Java service (#300)

* Fix the application name and the script to start the crypto demo.

* Update the project documents [ECR-486, ECR-1515, ECR-1516] (#296):

Add a contribution guide, clean up the readme, add a roadmap.

* Fix the formatting.

* Fix anchors in the contribution guide.

* Change travis-ci.com to travis-ci.org (#305)
@shakuzen
Copy link

@tbroyer Thank you for the background. That clears that up. And thank you for the workaround (or possibly just The Solution).

It would be nice to have some official statement on the planned direction from the maintainers. Java 10 has been released for months and Java 11 (which likely will see more adoption more quickly due to the LTS designation) will be released in a couple months. I'm trying to figure out for projects on Java 10+ whether it is best to drop error-prone in the meantime or use it as a compiler plugin.

@cushon
Copy link
Collaborator

cushon commented Aug 8, 2018

TL;DR Error Prone now supports JDK 10 and 11 when running as a javac plugin (-Xplugin). That's the approach I recommend to use Error Prone with 10 or 11 today.

We're going to keep maintaining error-prone-javac and try to closely track the six month release cadence, but we have some work to do to catch up to the new schedule.

Responding to some of the earlier discussion:

IIUC, the version of Javac used in Error Prone has been "cut"/forked from the JDK before they added JPMS

That used to be the case, but we caught up to the JDK 9 GA in e7323f3. The main hurdle for JDK 10 is the issue @eaftan mentioned that it will likely mean dropping support for running on JDK 8 in error-prone-javac. Then anyone using JDK 8 for builds would be stuck on an old release, or we'd need to maintaing branches for JDK major versions, neither of which seem like great options.

My hope is that eventually we can stop using error-prone-javac and make-Xplugin the best and only way to run Error Prone, since it simplifies supporting the six-monthly releases.

I apologize for not knowing more of the intimate details, but as far as I know, Error Prone has relied on the Java 9 compiler for more than 3 years at this point (since cfbbe69 released in version 2.0.2) without requiring JDK 9 be used to compile projects using Error Prone.

Do most Error Prone users want to hop on that every-6-month JDK upgrade treadmill?

Why would this be required going forward when it hasn't been required while Error Prone has internally used JDK9?

It's true that we've been using the JDK 9 javac for a while, but effectively what we've been doing is trying to use a javac close to head. We still want to do that. It was simpler when the release cadences were longer, since there were about three years where "javac 9" and "javac at head" were more or less the same thing.

Version N of javac is a Java N program that expects to run on JDK N. There's some support for running in a bootstrap configuration on JDK N-1, but that's not really a supported production configuration. (e.g. modular compilations aren't well supported with javac 9 running on JDK 8.) So when we move to javac 10 we can probably still support JDK 9, but not JDK 8, which is problematic.

@don-vip
Copy link
Contributor Author

don-vip commented Aug 8, 2018

@cushon That's great news! Current documentation states we need to add a lot of -J--add-exports options with Java 9 and error_prone_ant 2.3.1. Is it still needed with Java 11 and current head?

@cushon
Copy link
Collaborator

cushon commented Aug 8, 2018

Those flags aren't needed. (I think I added them testing against a pre-release JDK 9, some of those errors were relaxed or downgraded to warnings.)

@cushon
Copy link
Collaborator

cushon commented Aug 8, 2018

I updated the docs: 3005cc9

@don-vip
Copy link
Contributor Author

don-vip commented Aug 8, 2018

As far as I am concerned this issue is solved: with the same error_prone binary we're able to build JOSM on Java 8 to 11 👍
The Ant chapter must be updated in the documentation to explain the correct syntax (it is not easy to guess if @tbroyer does not help you 🥇).

@tbroyer
Copy link
Contributor

tbroyer commented Aug 8, 2018

BTW, Gradle and Maven sections would need to be updated too (Maven using annotationProcessorPaths, Gradle using the net.ltgt.errorprone-javacplugin).

@cushon
Copy link
Collaborator

cushon commented Aug 8, 2018

Great, thanks for verifying!

The work to update error-prone-javac will still happen, but I can track that somewhere else.

I'll leave this open until the docs are fixed. PRs for ant and gradle especially would be very welcome :)

floscher pushed a commit to floscher/josm that referenced this issue Aug 9, 2018
@cushon cushon mentioned this issue Aug 10, 2018
codefromthecrypt pushed a commit to openzipkin/brave that referenced this issue Aug 20, 2018
This uses the commandline instructions here:
  http://errorprone.info/docs/installation

However, there's a glitch as I'm not sure why
`-XepDisableWarningsInGeneratedCode` doesn't work.

See google/error-prone#860
@codefromthecrypt
Copy link

I haven't found anyone using this approach with maven before. I gave a stab and it looks to work except I can't pass arguments like I could before. If someone doesn't mind having a look, would be appreciated openzipkin/brave#761

@cushon
Copy link
Collaborator

cushon commented Aug 21, 2018

I updated the ant and maven examples in 3f2ca43 and 82b319c, and also pushed an updated to the installation docs: http://errorprone.info/docs/installation

@cushon cushon closed this as completed Aug 21, 2018
tbroyer added a commit to tbroyer/gradle-errorprone-plugin that referenced this issue Aug 24, 2018
Travis doesn't have oraclejdk9 on language:android, and
the openjdk9 installed by install-jdk.sh doesn't have the
necessary certificate roots to download dependencies, so
given that JDK9 (and soon JDK10!) is EOL, let's no longer
run tests on it ¯\_(ツ)_/¯

As a side note, we'll have to wait for the next version of
Error Prone before we can test on JDK 11 (JDK 10 isn't even
"officially" supported by Error Prone 2.3.1):
google/error-prone#860 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants