-
Notifications
You must be signed in to change notification settings - Fork 41
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
VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet #15
Conversation
…tead of javax.servlet
This can't be serious |
I think it's totally serious. IMO, VelocityTools needs a branch (or main) to migrate to Jakarta EE. Not everybody is going to let their web applications languish forever. At $work we are planning our migrations and for the time being we are looking at relying on Tomcat's Migration Tool for projects like Velocity-Tools that don't appear to have any plans to produce compatible versions. That's not a good long-term solution. |
@michael-o Can you explain why modernising Velocity to use Jakarta is a joke for you? It's not right to expect all downstream projects to use the Tomcat migration tool when this issue can be easily fixed centrally in Velocity. If Velocity is never updated, should the community consider that it's part of the Apache attic? I'm more than happy to switch to Thymeleaf if that's the attitude we get when trying to contribute to make a project more usable. Besides, it would be good to also keep the issue tracker up-to-date with explanations there too, that's the basics. |
I can easily explain why I have immediate closed this out: Before opening a PR for such a high-impact change I would expect a discussion in the JIRA issue about a possible solution for everyone. You just dropping off this PR which solves your problem by changing the code in main leaving everyone else behind which do rely on Make a reasonable proposal and I will have a look at this. |
I can't believe that this has been turned down. this is not the problem of one committer but of everyone who wants to keep his software up-to-date. Currently, velocity-tools is preventing this (and as this commit shows, intentionally). |
I hope you have read my explanation... |
I wholeheartedly support the sentiment of @ghueller, @ChristopherSchultz, and @ppodgorsek , and I also understand the perspective of @michael-o in the desire to not break the current API. I have proposed a "yes, and" solution in my related comment on VELTOOLS-202 that provides a clean path for supporting Servlet API 3.0 through 6.0 (inclusively) in the same build. The Apache FreeMarker team recently took this approach in their update from 2.3.32 to 2.3.33, which does not break any backwards compatibility and opens the path for developers who need to use a modern Servlet API in their projects. As demonstrated in their minor version update, they did not consider this to be a major change, as it did not affect any dependent projects. They did, however, consider this to be a Major Priority. See FREEMARKER-218. I have successfully migrated Apache Tiles 3 to Servlet API 6.0 (which relies on velocity-tools-view as well as FreeMarker) and have found that it works perfectly in Tomcat 10.1. I feel that this is a very reasonable approach that cuts through all arguments against providing support for Servlet API 6.0 in velocity-tools-view. In light of this proven solution, I respectfully request that you reopen this issue. I would be happy to work with @michael-o on a PR that works toward this solution. |
@ct-parker Thanks for the offer. I am waiting for a Core release first. |
@michael-o I greatly appreciate your reconsideration of this issue. I am willing to continue the discussion as needed here or in the JIRA issue, so please just let me know of any support you need on this, and I will jump in. I am eager to resolve this. |
Please point me to the exact commit you want me to look at and propose as well. I want to check the approach taken. |
It was apache/freemarker#104, which had a single commit: apache/freemarker@c88ed78 At some time in the past, the FreeMarker project switched their build process from Ant to Gradle, which gave them the option of using a code generator for solving this issue. Essentially, the code generator copies the source code for the package/classes of interest into a package named "freemarker.ext.jakarta" and does a search/replace on javax.servlet to jakarta.servlet prior to compiling. See this part in particular. I'm not suggesting using a code generator, but rather simply duplicating the packages into a jakarta package and search/replace for the "javax." imports. The build itself apparently brings both the Servlet 3.1 and 6.0 APIs in as compile dependencies, but after compiling either version may be used in downstream projects. The FreeMarker project commentary is here:
I'm not familiar with the velocity-tools-view build process, but this is an approach that could easily be replicated with Ant, eliminating the need to continuously copy/paste source code changes into both packages. |
Do you consider it similar to mine: https://github.com/michael-o/activedirectory-dns-locator/blob/9dff8dfb3fce8890a8166be6de89b9c05ef77108/pom.xml#L92 |
Yes, that should work! It looks like the same approach. @michael-o Would you like me to put a PR together for this, or would you prefer to do it? |
I would appreciate one. Still occupied with some Maven releases... |
Fantastic! I'll post it in a few days (I don't use Maven for builds, so I need to get a special build/test environment together for this). |
Here are a few takeaways for you why my approach with a single module will not work for Velocity Tools:
There are more arguments I cannot remember ATM, but we have here only one choice: Yet another module in the reactor which just reuses the sources somehow, rewrites them and builds appropriately. What I wouldn't also do is to duplicate the sample/showcase modules at all. Why? If a developer cannot make the mental stretch to replace some namespace then he shouldn't deal with it at all. I'd solely focus on Servlet/JSP related code: |
I will support any solution that results in a way to use
I would be totally fine with that. This sounds like more than a simple pull request. You probably don't want me trying to change the module names and creating new ones. I'd be willing to try, but I'll stand by for your recommendation.
While I agree that it would be unnecessary to duplicate the sample/showcase modules, we were all beginner developers at some point. As a courtesy, perhaps you could simply add a comment above the relevant imports to suggest that the examples will also work with the jakarta version of the modules if "javax." is replaced with "jakarta.". At least that would show that it is intentional and prevent any unnecessary questions. IIRC, Freemarker also included some comments like that somewhere. |
A new module seems inevitable, a copy and replace of the sources seems appropriate. But I'd rather have the main artifacts pointing to jakarta and the new ones named -javax, as at some point we'll drop the later. |
I agree with Claude. We should ask people moving to new versions with old javax to update their module name. The jakarta versions should be the new default going forward. |
First is to create the new jakarta modules. I wouldn't even copy the source code, I would use the
I'd leave this afterwards, but I would do is that I would skip the deployment of all showcases and examples being for them being in Maven Central has zero value. |
@michael-o I just wanted to ping this issue again. If I create a PR for this, should I go with the -javax strategy that @arkanovicz proposed, or stick with the -jakarta label? |
I'll get back to you tomorrow with an idea for an MVP to get forward. |
So let me summarize how I understand to get the MVP:
|
The javax packages are not maintained anymore, are they? We typically want to deprecate them, and encourage users to migrate. It's largely enough to provide support for javax packages for users who just can't migrate for now, why put the hassle of changing package names for people who have migrated along with the flow? And how are you going to deprecate standard packages use? |
There is another course of action, now that I think of it: maintain the tools 3.1.x on javax and switch on jakarta for 3.2+. |
I am against that. Cutting off people for the years to come for a long waited update is not right. Both Jetty and Tomcat will support javax for quite some time. |
The API is stable, implementations are refined. I see nothing wrong with that. |
I'm not speaking of cutting them off. I'm speaking of keeping javax- for 3.x releases and switch to jakarta- for 4.x releases. It's twice as many releases but they would be done in the same momentum. The bonus is that there is no extra module. |
There is a lot more than the servlet API to migrate in a real J2E webapp (activation, mail, beans, etc.). And the dependencies are intertwined, you cannot keep the servlets API on javax- and use the remaining on jakarta-. So yes, jakarta EE will become the de facto standard and we should align with this fact today. If we stay on the course of two modules rather than the 3.x/4.x proposal aforementioned, it means that the javax should be the one with copied sources and a dedicated suffix, not the reverse. |
I see, this contradicts your message here: #15 (comment) |
The reverse copy means a breaking change which needs to be communicated, but I still consider this wrong for a new minor version. Rather announce that in 3.0 jakarta will be the default module and |
So, 4.0 you mean. If it seems preferable to you, rather than 3.x with javax and 4.x with jakarta (yes I agree on major/minor of course), then let's do that. |
No description provided.