Skip to content

Compilation on jdk11 for release 8#312

Closed
pacomendes wants to merge 1 commit intoapache:masterfrom
pacomendes:master
Closed

Compilation on jdk11 for release 8#312
pacomendes wants to merge 1 commit intoapache:masterfrom
pacomendes:master

Conversation

@pacomendes
Copy link
Copy Markdown

Hi, we're migrating to JDK11 and came across this potential issue.

I see from the manifest that 4.2.0 is built on jdk11:

Build-Jdk: 11.0.2
Built-By: jordanzimmerman

Changes to covariant return types for ByteBuffer in JDK11 may cause a java.lang.NoSuchMethodError when running on JDK 8. Which may impact these calls:

https://github.com/apache/curator/blob/master/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java#L307-L319

This can be mitigated by setting the --release to 8. Although I see that .travis.yml is still using oraclejdk8

Comment thread pom.xml
<configuration>
<release>${jdk-release-version}</release>
<source>${jdk-version}</source>
<target>${jdk-version}</target>
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source and target can be removed when switching to jdk11. Currently this would cause compilation to fail on jdk8

@Randgalt
Copy link
Copy Markdown
Member

Please create an issue in our Jira. All PRs must be for a Jira issue. See here: https://cwiki.apache.org/confluence/display/CURATOR/Submitting+Pull+Requests

@pacomendes
Copy link
Copy Markdown
Author

Thanks @Randgalt Closing this PR, will create an issue.

@pacomendes pacomendes closed this May 22, 2019
@Randgalt
Copy link
Copy Markdown
Member

The manifest for the JAR at Maven central does show that. I'm not sure why. The classfile version is still 52 (Java 8) I just checked with javap

Classfile jar:file:xxxxxxx/curator-framework-4.2.0.zip!/org/apache/curator/framework/CuratorFramework.class
  Last modified Mar 3, 2019; size 4640 bytes
  MD5 checksum 0b16620a7a82f836290510521dc5ee9a
  Compiled from "CuratorFramework.java"
public interface org.apache.curator.framework.CuratorFramework extends java.io.Closeable
  minor version: 0
  major version: 52

@Randgalt Randgalt reopened this May 22, 2019
@pacomendes
Copy link
Copy Markdown
Author

I created https://issues.apache.org/jira/browse/CURATOR-523

If you build on JDK 11.0.2 using --source 1.8 and --target 1.8 options, the classfile version will be 52, but due to changes in covariant return types in the JDK, it fails at runtime. This is a very odd edge case we picked up in production...

If you compile the following code on JDK9+ using --source 1.8 --target 1.8 and fun on JDK8 it will fail at runtime with a java.lang.NoSuchMethodError.

If you compile with JDK9+ using --release 8, it will cast and resolve the correct method correctly (http://openjdk.java.net/jeps/247)

    public Buffer doSomething() {
        ByteBuffer byteBuffer = ByteBuffer.allocate(0x800);
        return byteBuffer.flip();
    }

Maven compiler plugin provides a --release option if you're building on JDK11, if you're also compiling and testing on JDK8, we would need to switch this using a maven profile.

@shayshim
Copy link
Copy Markdown
Contributor

@Randgalt Do we currently have any reason to build using JDK 11 (or any greater than 8)?
If not, and we rebuild 4.2.0 with latest JDK 8 - I think we don't need to do any change.

@pacomendes
Copy link
Copy Markdown
Author

pacomendes commented May 27, 2019

@shayshim @Randgalt If you've already been building and published release artifacts using JDK 11, I think it's sensible to just include the --release flag for future releases and upgrade your ci builds to use openjdk11.

With changes in Oracle's licensing, the increased release cadence of JDKs, breaking changes introduced since 11 (removal of a number of JEE classes) and support for java platform module system, it's a very good time to upgrade your builds to ensure you don't accidentally reference removed types. JDK11 is an LTS release.

@pacomendes
Copy link
Copy Markdown
Author

Created PR #313

@martin-g
Copy link
Copy Markdown
Member

This PR is obsolete and can be closed!

@tisonkun
Copy link
Copy Markdown
Member

We already achieved it.

@tisonkun tisonkun closed this Apr 30, 2022
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

Successfully merging this pull request may close these issues.

5 participants