-
Notifications
You must be signed in to change notification settings - Fork 555
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
Migrate to jakarta namespace #238
Conversation
We are currently migrating an application using this proxy servlet to JakartaEE and are looking for a solution as well. |
We are also waiting for this change so that we can move to Tomcat 10. |
@reda-alaoui regarding I found an updated version at hazendaz/httpunit which works with JakartaEE. So instead of
use this:
|
I tested this locally and for me this worked. And the tests also run successfully with the updated httpunit variant mentioned in the previous comment. |
@reda-alaoui may I suggest to update http-unit to the aforementioned version and publish this draft PR? |
Is the intent for this to be in a new release branch or fork of the code? Just aware that this will be a breaking change for anyone who can't update their Java or servlet engine?
Thanks
Phil
Sent from Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Wolfgang Schell ***@***.***>
Sent: Sunday, February 26, 2023 11:31:56 AM
To: mitre/HTTP-Proxy-Servlet ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [mitre/HTTP-Proxy-Servlet] Migrate to jakarta namespace (PR #238)
@jetztgradnet commented on this pull request.
________________________________
In pom.xml<#238 (comment)>:
<dependency>
- <groupId>javax.servlet</groupId>
- <artifactId>javax.servlet-api</artifactId>
- <version>3.1.0</version>
+ <groupId>jakarta.servlet</groupId>
we also need to update/change httpunit to a variant supporting Jakarta EE:
<dependency>
<groupId>com.github.hazendaz.httpunit</groupId>
<artifactId>httpunit</artifactId>
<version>2.0.0</version>
<scope>test</scope>
</dependency>
—
Reply to this email directly, view it on GitHub<#238 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIPKURC73QEDHRX4RVREM2LWZM5KZANCNFSM6AAAAAAUDIKTDE>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
Unless otherwise stated, this email has been sent from Fujitsu Services Limited (registered in England No 96056); Fujitsu EMEA PLC (registered in England No 2216100) both with registered offices at: Lovelace Road, Bracknell, Berkshire RG12 8SN; PFU (EMEA) Limited, (registered in England No 1578652) registered offices at: Belmont, Belmont Road, Uxbridge, England, UB8 1HE and Fujitsu Research of Europe Ltd (registered in England No. 4153469) 4th Floor, Building 3, Hyde Park Hayes, 11 Millington Road, Hayes, UB3 4AZ.
This email is only for the use of its intended recipient. Its contents are subject to a duty of confidence and may be privileged. Fujitsu does not guarantee that this email has not been intercepted and amended or that it is virus-free.
|
I'll branch the current master branch to maybe "branch_1x" leaving the current master branch for the new 2x. |
Great. Many thanks David
Sent from Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: David Smiley ***@***.***>
Sent: Sunday, February 26, 2023 6:22:53 PM
To: mitre/HTTP-Proxy-Servlet ***@***.***>
Cc: Pinel, Phil ***@***.***>; Comment ***@***.***>
Subject: Re: [mitre/HTTP-Proxy-Servlet] Migrate to jakarta namespace (PR #238)
I'll branch the current master branch to maybe "branch_1x" leaving the current master branch for the new 2x.
—
Reply to this email directly, view it on GitHub<#238 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIPKURD2SGL4CHX5QSUN5QDWZONP3ANCNFSM6AAAAAAUDIKTDE>.
You are receiving this because you commented.Message ID: ***@***.***>
Unless otherwise stated, this email has been sent from Fujitsu Services Limited (registered in England No 96056); Fujitsu EMEA PLC (registered in England No 2216100) both with registered offices at: Lovelace Road, Bracknell, Berkshire RG12 8SN; PFU (EMEA) Limited, (registered in England No 1578652) registered offices at: Belmont, Belmont Road, Uxbridge, England, UB8 1HE and Fujitsu Research of Europe Ltd (registered in England No. 4153469) 4th Floor, Building 3, Hyde Park Hayes, 11 Millington Road, Hayes, UB3 4AZ.
This email is only for the use of its intended recipient. Its contents are subject to a duty of confidence and may be privileged. Fujitsu does not guarantee that this email has not been intercepted and amended or that it is virus-free.
|
There are different ways to go about this. In paultuckey/urlrewritefilter#242 instead of migrating everything they use a Maven plugin to create a different But IMHO in the long run, having a 2.0 release for the Jakarta-based servlet containers should be the right path. |
That's a fantastic idea @jetztgradnet ! Basically use shading and a Maven classifier to ship it. I didn't know it could be so easy. This allows me to have it both ways -- support I'd like to go in that direction. LMK if there are concerns. |
@dsmiley as you can see at https://github.com/mitre/HTTP-Proxy-Servlet/pull/238/files#r1101714206 , this is not a simple package replacement. |
@reda-alaoui right, but adding the shading plus changing this line is still possible, right? the |
@dsmiley whether with a new major version for jakrata-only or a dual-variant approach, it would be great to get a release version soon :-) |
I agree for this particular case. But more cases like that may appear in the near future. More generally, I think allowing people to stay for years on |
So we now have two possible approaches:
@dsmiley so which way do you want to go? I am following @reda-alaoui argument, but at the same time it is my main short-term interest in getting a release supporting Jakarta EE with any of the two approaches: If you prefer the first approach, I would be willing to provide a PR with that one-line change required to work for Servlets 6.x plus the shading (assuming we can simply transfer the approach from paultuckey/urlrewritefilter#242). |
Then we do a bit of both. Embrace the "shade" mechanism to allow one maintained codeline and release but also do everything this PR is doing (new major version; use Jakarta in the maintained code). The "shade" configuration would be to produce a "javax-servlet" classifier (or something like that; dunno if there's a standard). |
That sounds like a great compromise. In the simplest case we can take the example from the urlrewritefilter and reverse 'javax.servlet' and 'jakarta.servlet'. @reda-alaoui would you also do that or would you want me to provide a snippet for that? @dsmiley any preference for a classifier name? 'javax' or 'javax.servlet' |
I guess |
So I didn't want to re-create @reda-alaoui's PR or provide a different PR include that commit, but adding this snippet to
Verifying generated jars:
So @reda-alaoui or @dsmiley, whoever is first, would either of you add this commit to this PR or afterwards after merging this PR? |
sure thing. I'll do a release this weekend |
eb99a83
to
c7de0d7
Compare
@jetztgradnet @dsmiley I added the shading plugin |
Password stored in database / EncryptedStringType not supported by jasypt jasypt/jasypt#147 Cache / Ehcache 2 to 3 migration https://www.ehcache.org/documentation/3.10/migration-guide.html and https://docs.jboss.org/hibernate/orm/6.2/userguide/html_single/Hibernate_User_Guide.html#caching-provider-jcache-cache-manager Java / ReadOnlyMvcInterceptor / Something to remove? Java / ServiceManager / EofException Java / MailUtil / use jakarta.mail.*? Spring security / Check openid (getClientRegistration), keycloak (todo), cas (change to apero) Wro4J / No support for Spring6 wro4j/wro4j#1135 HTTP Proxy / Jakarta migration in progress mitre/HTTP-Proxy-Servlet#238 (comment) Camel / https://camel.apache.org/blog/2022/05/camel317-whatsnew/.
@plan3d are you going to remove those old httpclient version or would you prefer if somebody else jumps in? I didn't want to just push another commit to this PR without checking with you first... |
I assume removig support/testing for old httpclient versions would mean to simply remove those lines from |
Or maybe this would rather be for @reda-alaoui to answer: would you simply remove the tests for older httpclient versions? Or is it ok if somebody else adds another commit to your PR? |
@reda-alaoui any comment from your side on removing the old httpclient tests? |
On it. |
Done. Looks like someone has to trigger the build. |
@reda-alaoui thanks for providing this update! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks everyone!
The CHANGES.md needs to be updated as does README.md. Can that be done please? README references older HttpClient support which is now false. Needn't be thorough but don't want wrong information. README should reference the dual classifier thing.
@dsmiley CHANGES.md and README.md updated. |
Please don't force-push changes to PRs. It means reviewers (like me) can't see what changes you did between now and before. |
Ok noted. Some repositories want 1 commit per PR. Others don't. That's why I prefer Gerrit over Github. |
Co-authored-by: David Smiley <[email protected]>
Interesting... never seen such a 1-commit per PR policy. In GitHub, it's trivial to view all changes collapsed -- it's the default view if you click the "Files Changed" view. So I don't get the point. |
Co-authored-by: David Smiley <[email protected]>
I noticed Jakarta Servlet spec 5.0 was introduced here but that there are newer specs like 6 (judging from a question in #242 ) that was raised today. Is there an opinion here for/against raising the spec level, and to what? |
@dsmiley , I think my comment still stands. |
Fixes #237
I don't know what to do with
httpunit
. Any advice? :)