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

JDK 13/14 support #1324

Closed
tomparle opened this issue Jan 3, 2020 · 19 comments · Fixed by #1332
Closed

JDK 13/14 support #1324

tomparle opened this issue Jan 3, 2020 · 19 comments · Fixed by #1332
Labels
triage
Milestone
1.6.0

Comments

@tomparle
Copy link
Contributor

tomparle commented Jan 3, 2020 •
edited
Loading

Hi everybody,

with the upcoming release of JDK 14 in March, I think we should try to be up-to-date with the latest JDK. I have spent several hours to start the work to support JDK 13, and here are the state so far :

  • Groovy upgraded to 2.5.8 (for jdk13 compatibility)
  • Jaxen upgraded to 1.2.0 (to avoid module conflict with org.w3c.dom)
  • one test had to be modified to prevent the use of reflection which has been forbidden by JDK-8210522 CSR.

It seems to work for simple projects, but I have two issues :

  • the JDT Compiler gives "Type already existing errors" when hot-recompiling classes in dev mode. I could force a policy so that all compile errors are ignored to go further, but that's far from ideal
  • modules do not work : the classes are not properly recompiled and lead to errors (like "FasterGT" : there are some conflicts between package names and compiled template names)

If you want to get a look or contribute, you can see the work in progress here : https://github.com/tomparle/play1/tree/upgrade-jdk-13

I'll try to make some more progress later but module errors seem hard to solve.
All comments appreciated !
@tazmaniax
Copy link
Collaborator

tazmaniax commented Jan 3, 2020 •
edited
Loading

@tomparle I’ve had JDK13 running in production for a couple of months now and been meaning to raise a pull request. Will have a look

@tomparle
Copy link
Contributor Author

tomparle commented Jan 5, 2020

Hi @tazmaniax ,
thank you for your reply !
Do you mean that you have been using Play with JDK 13 ? I would be surprised because it does not seem to run out of the box.
With pleasure to work together on this one ! (see details in my initial post)

@tazmaniax
Copy link
Collaborator

tazmaniax commented Jan 5, 2020 •
edited
Loading

@tomparle I had to upgrade a number of libraries but otherwise it was good to go. Here is a summary of the dependency updates...
- org.ow2.asm -> asm 7.2
- org.ow2.asm -> asm-commons 7.2
- org.ow2.asm -> asm-util 7.2
- org.ow2.asm -> asm-tree 7.2
- org.ow2.asm -> asm-analysis 7.2
- org.codehaus.groovy -> groovy 2.5.8
- org.codehaus.groovy -> groovy-xml 2.5.8
- org.eclipse.jdt -> org.eclipse.jdt.core 3.18.0
- org.javassist -> javassist 3.26.0-GA

I think I had a Groovy template (FasterGT) issue with org.eclipse.jdt.core 3.19.0 so kept it at 3.18.0

Apart from that all good running on Heroku with OpenJDK 13

@tazmaniax
Copy link
Collaborator

Oh you found the org.w3c.dom module conflict, cool! I spent ages trying to find that but it didn't block deployment for me so left it

@tomparle tomparle mentioned this issue Jan 6, 2020
Merged
@tomparle
Copy link
Contributor Author

tomparle commented Jan 6, 2020

Thank you @tazmaniax for sharing this !
Indeed, there is a compilation regression starting from org.eclipse.jdt.core 3.19.0. Reverting to 3.18.0 fixed the module issues ! You helped us a lot here.

Unfortunately that means that we are currently limited to compiling sources up to JDK 12 (java.source parameter in application.conf), but that is not so much an issue for now. Still, it'd be great to be able to make future versions of jdt core work when we have some time to look at it.

I updated my branch and I think it's ready for a pull request, so I submitted it for review : #1325

@tomparle
Copy link
Contributor Author

Hi,

some update on this : I have also made the needed evolutions for compatibility with JDK 14, see https://github.com/tomparle/play1/tree/upgrade-jdk-14
There were only some libs to upgrade (Groovy to 3.x, asm, javassist and jdt compiler) and the GroovyTemplateCompiler class to fix.

Unfortunately, the Groovy migration causes the module faster-gt (Faster Groovy Template) to fail. The same GroovyTemplateCompiler in the gt-engine should be applied the same fix, but I do not have access to the gt-engine repository.

@asolntsev it seems that you are also maintaing the Replay fork of the Play! framework which seems more up to date, should we all join the efforts to this new project ? It is, unfortunately, becoming harder and harder to maintain.

@asolntsev
Copy link
Contributor

asolntsev commented May 29, 2020 •
edited
Loading

Hi @tomparle.
Yes, we do use RePlay in production, and we continuously upgrade it to all newest Java versions.

But it's backward-incompatible with Play1. Very-very incompatible. :) We removed quite a lot of Play1 features, all the classloading magic, auto-generation of getters/setters, JPA enhancements etc. to make code faster, smaller and magic-less.

You can use RePlay, but you need to modify your code quite a lot to do it.

@tomparle
Copy link
Contributor Author

tomparle commented May 30, 2020 via email

@jvosloo
Copy link

jvosloo commented May 30, 2020

Hi @tomparle, @tazmaniax and co... we're still big Play1 users and sincerely appreciate the effort you guys are putting in into keeping up with the latest Java versions etc.

Big up to you guys! 💪 👏

@blinder
Copy link

blinder commented May 30, 2020

yes, just to second @jvosloo really appreciative of the work and effort here! it has not gone unnoticed!

@tazmaniax
Copy link
Collaborator

@tomparle I had a look at the faster-gt issue a couple of weeks ago and even had a brief chat with the Eclipse Compiler folk. The chat was not overly productive but the long and short is it looks like there was change in the compiler to be a bit more strict around file naming that doesn't work well with the faster-gt approach of including the package name in the file name. If we can get faster-gt to use more conventional final names then we might be able to resolve this and move forward

@tomparle
Copy link
Contributor Author

tomparle commented Jun 2, 2020

Wow, thank you for your support messages @jvosloo and @blinder, it was not expected and it's really motivating !
I'll try to dig a bit further into faster-gt, especially the gt-engine, whose RePlay version seems to be up to date, to deliver a functional JDK 14-compatible PR !
@tazmaniax thanks for your feedback. I think I remember these errors which occurs when we specify a java version > 11 in the application.conf file. I'll see what can be done, but I think I will first try to make Play run with the new JDK, then in a second step make it compiles with the source > 11.

@tomparle
Copy link
Contributor Author

tomparle commented Jun 2, 2020 •
edited
Loading

I could make Play work with the JDK 14, but unfortunately the Eclipse JDT compiler lib upgrade makes the faster-gt plugin conflicts with generated compiled ressource names, as expected, which is unfortunate for those who uses it (like me).
@tazmaniax , would you have an idea of where to look in faster-gt code to change the naming policy ?

For people wanting to use the JDK 14 compatible version : https://github.com/tomparle/play1/tree/upgrade-jdk-14
But I'd prefer not to push it before we can deal with the faster-gt issue.

@asolntsev
Copy link
Contributor

asolntsev commented Jun 3, 2020 via email

@tomparle tomparle mentioned this issue Jun 3, 2020
Merged
5 tasks
@tomparle
Copy link
Contributor Author

tomparle commented Jun 3, 2020

Thanks @asolntsev for the tip, indeed it's much easier to do this.

Here we go !

In brief, faster-gt users will have to use the new faster-gt once the PR is reviewed and accepted.

Feedback welcome !

@tazmaniax
Copy link
Collaborator

@tomparle was the gt-engine upgrade to Groovy 3 necessary? Of course that needs to be done at some point but that wasn't a blocker for the file naming issue, was it? Just curious.

@tomparle
Copy link
Contributor Author

tomparle commented Jun 3, 2020 via email

@tazmaniax
Copy link
Collaborator

Yes I have jdk14 running without the Groovy 3 upgrade. I wonder what I did differently

@tomparle
Copy link
Contributor Author

tomparle commented Jun 4, 2020

That, I do not understand !
When I tried the first time with the original faster-gt module, the gt-engine jar threw an exception in the GTGroovyCompileToClass class, due to the same cast issue I had to fix in Play.
It seems that @asolntsev had to make the same fix in their version of gt-engine, so it seems mandatory.

@tomparle tomparle changed the title JDK 13 support JDK 13/14 support Jun 8, 2020
This was referenced Oct 25, 2020
Closed
Merged
@xael-fry xael-fry added this to the 1.6.0 milestone Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
triage
Projects
None yet
Milestone
1.6.0
Development

Successfully merging a pull request may close this issue.

Compatibility with JDK 13 and 14 tomparle/play1
6 participants
@blinder @jvosloo @asolntsev @tazmaniax @xael-fry @tomparle