-
Notifications
You must be signed in to change notification settings - Fork 95
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
TAP5-2741: switch to Jakarta EE #43
Conversation
Hello, @derkoe ! Thank you very much for this contribution! Actually merging it will take some coordination and decisions regarding Tapestry versions. Just one question: was the change in source and target Java versions from 8 to 11 needed for the Jakarta EE upgrade? Cheers! |
@thiagohp actually it works also with Java 8. I'm not sure if this makes sense since most platforms switch at least to Java 11 with Jakarta EE (Tomcat 10.1, Jetty 11). Only the Spring module needs to be built with Java 17 because of their new requirements (https://spring.io/blog/2022/11/16/spring-framework-6-0-goes-ga) We have this branch already running with some of our Tapestry apps - so far everything looks good. |
Amazing PR! Great job @derkoe, I think this is the way (jakarta and Spring 6 / Spring boot 3) and I will force to java 17+. As Tapestry developers, we need that the framework will run as java ecosystem. |
I'm very grateful for your work provided here to ensure compatibility with future Spring versions @derkoe 👍 |
Hi @HansMontana, merging the changes themselves isn't the big problem, it's remaining backward-compatible without doubling the required work on our end. We're still working on a solution to support both javax and jakarta as we don't want to leave users behind who can't upgrade and cut them off from future Tapestry features/changes. Our resources are quite limited, and we don't want to stretch them even thinner by maintaining two branches by hand, so we are still looking for a technical solution that can do at least the grunt work. There are some promising leads, like https://github.com/eclipse/transformer which was initially created for the javax/jakarta issue and is used by Hibernate via https://github.com/hibernate/jakarta-transformer-plugin Right now, we can't promise any timeline. If anyone can weigh in on a solution for the javax/jakarta issue or contribute in any way, we'd love to hear about it! |
Hey @benweidig - I think your concerns around backward-compatibility are valid but I think it is much easier to maintain two branches than a hacky solution using the transformer. I think it might be even impossible to have one codebase for both versions - the changes in fileupload, Jetty and Tomcat APIs are difficult to handle without duplication. The changes of ASM in tapestry-plastic are needed for Java 17/21 in any way. Should I separate them into a separate PR? |
Hello, @derkoe ! Yes, please separate them into a separate PR. What are the needed changes? I had already done a pass on getting Tapestry-IoC and its dependencies (Plastic included) to get everything working with Java 17. I haven't checked on later Java versions, though. |
I missed this talk about ASM and we just hit the issue at work, so I whipped up a quick fix in #46, maybe there is something useful in there (I removed the copy of the ASM sources , which should make it easier to upgrade in the future). We have not yet tested Java 21 extensively, but things seem to be working on Java 21 with my fix |
@thiagohp I guess the solution proposed by @chrispoulsen is better. |
Things changed: - Switch all Java EE to Jakarta EE APIs - Update references in JavaDoc for Jakarta EE - Switch to Spring 6 (Jakarta EE, Java 17) - Switch to Jetty 11 and Tomcat 10.1 (for support of Jakarta EE)
fcf0cf7
to
492577b
Compare
Removed the ASM change from this PR. Now it's only Jakarta EE (incl. Tomcat and Jetty) + Spring 6 |
@derk, any reason to set the Servlet API to version 5.0 though Tomcat 10.1 uses Servlet API 6.0? |
@coderkun I have used the minimum version that uses Jakarta EE - also Jetty 11 is based on Servlet 5. |
Any update on this? I think many Tapestry users have the need to switch to Jakarta EE. We already run these changes successfully and would be happy to move off our fork. |
Hi @derkoe, we talked a little bit about the general progress/timeline of the Jakarta release on the mailing list https://lists.apache.org/thread/6ydk6ors5r2qp73ks3bwvk3op3k9hkz7 Summary:
|
Things changed: - Switch all Java EE to Jakarta EE APIs - Update references in JavaDoc for Jakarta EE - Switch to Spring 6 (Jakarta EE, Java 17) - Switch to Jetty 11 and Tomcat 10.1 (for support of Jakarta EE)
Things changed: - Switch all Java EE to Jakarta EE APIs - Update references in JavaDoc for Jakarta EE - Switch to Spring 6 (Jakarta EE, Java 17) - Switch to Jetty 11 and Tomcat 10.1 (for support of Jakarta EE)
Things changed:
Set Java source/target version to 11Get the current version of ASM from https://gitlab.ow2.org/asm/asmAll tests pass for this version: https://github.com/derkoe/tapestry-5/actions/runs/6184852595