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

Jdk11 compatibility #1203

Closed
wants to merge 2 commits into from
Closed

Jdk11 compatibility #1203

wants to merge 2 commits into from

Conversation

davido
Copy link

@davido davido commented Aug 26, 2018

Switch to experimental asm API that supports JDK11.

This release fixed numer of smalle issues: [1].

[1] https://asm.ow2.io/versions.html
Latest asm release added experimental support for JDK11 features:
* nest mates
* ConstantDynamic

Consume this new features through ASM7_EXPERIMENTAL API.
@cushon
Copy link
Contributor

cushon commented Aug 26, 2018

What are the compatibility implications of this? The docs [1] for ASM7_EXPERIMENTAL say:

Experimental, use at your own risk. This field will be renamed when it becomes stable, this will break existing code using it.

So there's going to be a source compatibility issue, but I'm not sure if it's also a binary compatibility issue: ASM7_EXPERIMENTAL is a static final field that will be inlined, but will future ASM releases continue to accept that option, or will the eventual non-experimental ASM7 have a different value?

[1] https://gitlab.ow2.org/asm/asm/blob/3f7d6ba206ec6280a584bc135cd86c2b2cf5c279/asm/src/main/java/org/objectweb/asm/Opcodes.java#L50

Copy link

@jjrebel89 jjrebel89 left a comment

Choose a reason for hiding this comment

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

Got ya

@mkurz
Copy link
Contributor

mkurz commented Sep 1, 2018

@davido I think just updating the pom.xml isn't enough: You also have to upgrade the jar file, see how that was done earlier: #1197 (and also #1169)

@davido
Copy link
Author

davido commented Sep 2, 2018

@mkurz, Ah thanks for pointing this out. Apparently Guice is supporting both build tools Apache Ant and Apache Maven? I tested this PR with Maven and it worked for Gerrit Code Review purpose (Build and run with JDK11).

@davido davido mentioned this pull request Sep 26, 2018
@frankgrimes97
Copy link

OpenJDK 11 has now been released... any ETA on when we might expect this to be addressed?

@mkurz
Copy link
Contributor

mkurz commented Sep 27, 2018

@frankgrimes97 See these comments here: #1198 (comment) - Seems like we get an updated Guice very soon.

@sameb sameb closed this Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants