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

Add support to jakarta servlet #647

Closed
wants to merge 3 commits into from
Closed

Conversation

mmadoo
Copy link

@mmadoo mmadoo commented Mar 30, 2021

The module simpleclient_servlet_jakarta is a copy of simpleclient_servlet where javax.servlet is replaced by jakarta.servlet (real changes can be seen in the second commit)

Signed-off-by: Nicolas Trangosi <nicolas.trangosi@dcbrain.com>
Signed-off-by: Nicolas Trangosi <nicolas.trangosi@dcbrain.com>
@fstab
Copy link
Member

fstab commented Mar 30, 2021

Thanks a lot. Do you see any way to reuse the existing code? For example, if we merge #639 the jakarta servlet will not have that change.

@mmadoo
Copy link
Author

mmadoo commented Mar 30, 2021

I currently see no easy way to reuse the existing code.
Jetty have the same issue (the main branch is 10.0.x (using javax.servlet) and the branch 11.0.x (using jakarta.servlet) is updated to take changes from 10.0.x), but I do not know how they deal with it.

Changes from src dir are easy to report as the only changes are on import but for test directory it is not the case.

… profile updateSource

Signed-off-by: Nicolas Trangosi <nicolas.trangosi@dcbrain.com>
@mmadoo
Copy link
Author

mmadoo commented Apr 2, 2021

I have found a solution:
I add the profile updateSource which copy the src dir simpleclient_servlet to simpleclient_servlet_jakarta and do required replace in order to allow compilation of simpleclient_servlet_jakarta.
You could test it with: mvn compile -P updateSource.

This solution is not perfect as it may requires some adpations on test evolution and requires to modify first simpleclient_servlet, but it should do the job in a first time.
If this approach fit to you, I could update the doc (just provide me the best file to update)

@mmadoo mmadoo mentioned this pull request Apr 2, 2021
@travisspencer
Copy link

Are there plans to merge this PR? Without this kinda change, we cannot upgrade to Jakarta and drop the old javax servlet API.

Copy link
Member

@fstab fstab left a comment

Choose a reason for hiding this comment

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

@mmadoo sorry that it took so long to get back to this. Could you rebase it to the current master branch and squash the commits into one?

fstab added a commit that referenced this pull request Aug 2, 2021
@fstab
Copy link
Member

fstab commented Aug 2, 2021

Hey, I just pushed a commit that replaces this one, so I'm closing it. The next release will have Jakarta support. The reason why I pushed my own commit is that I played with re-using code across both implementations (javax and jakarta), and I ended up having a quite complete solution. Thanks a lot for bringing this up, jakarta support is now on its way.

@fstab fstab closed this Aug 2, 2021
@travisspencer
Copy link

Did this get released yet, @fstab ?

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.

None yet

3 participants