-
Notifications
You must be signed in to change notification settings - Fork 599
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
Create and update several Jakarta EE 10 API bundles #21535
Create and update several Jakarta EE 10 API bundles #21535
Conversation
jhanders34
commented
Jun 23, 2022
•
edited
Loading
edited
- Add Jakarta EE10 tests for other compatible features
- Add most of the missing EE10 component API bundles
- Add missing api bundles except for a few which don't use the ones from jakarta group.
- Update the version for the EE9 API bundles so it doesn't pull in the EE10 API artifacts
- Update the jakarta version properties for the transformer to be right for EE9, EE10 and the EE9/EE10 file.
- Update servlet and json b to their final API version
- Change to export servlet APIs as 6.0 instead of 5.0.
- Update buildpath and import statements to work with the new API versions.
- Move CDI, lang-model and annotation API bundles to their x.y.1 version.
- Add jsonp-2.1 feature to the appSecurity-5.0 feature.
#build #spawn.fullfat.buckets=io.openliberty.jakartaee10.internal_fat |
Joe's DHE check: ✔️ I found your oss_ibm.maven GAV artifacts on DHE. Thank you! ✔️ |
Please code review feature-related files, @OpenLiberty/delivery-approvers |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_dMOy4PNIEeyYXLZn0n3zUg Target locations of links might be accessible only to IBM employees. |
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.
Reviewed jakarta.servlet-api official jar is updated in dev/io.openliberty.jakarta.servlet.6.0/bnd.bnd and exports as 6.0; also removed from dev/cnf/oss_ibm.maven
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.
made a few minor suggestions since Libby is flagging the nlsprops for review- but I imagine these aren't new messages and might not be able to update them in the scope of this PR so I'll approve so as not to block it unnecessarily.
...ources/io/openliberty/jbatch/jakarta/batch/runtime/internal/resources/BatchMessages.nlsprops
Outdated
Show resolved
Hide resolved
...ources/io/openliberty/jbatch/jakarta/batch/runtime/internal/resources/BatchMessages.nlsprops
Outdated
Show resolved
Hide resolved
...ources/io/openliberty/jbatch/jakarta/batch/runtime/internal/resources/BatchMessages.nlsprops
Outdated
Show resolved
Hide resolved
91b5bf5
to
bad8963
Compare
@@ -0,0 +1,113 @@ | |||
/* |
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.
Is this a temporary overlay? Where can the original be found? What is the delta that we are adding? Is there an issue open to remove the overlay again in the future?
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.
It was an overlay in 2.0 as well. That is how it works with batch. @scottkurz can explain about it. I took the source from the maven repo sources.zip and used the 2.0 version and put the changes that were done in the 2.0 version into the 2.1 version.
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.
To answer the rest of the question, most of the class is modified.
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.
There may be a way to improve this. May need to talk with @scottkurz and/or @cgianfra about it.
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.
It's been an "overlay" since batch-1.0 back in 2015 or so (EE 7).
The API JAR from Maven Central uses ServiceLoader to find the JobOperator . In batch-1.0 we decided to use this approach to get a hold of our DS instance rather than I guess exposing it to the ServiceLoader. I'm sure whoever we asked at the time suggested this approach.
I'm curious what other APIs/features in Liberty do in similar situations. We did say clearly though in the Batch 2.1 UFO that it worked this way and that an application could NOT load a different impl using META-INF/services and ServiceLoader.
It's not something that we expect will change except possibly on a release boundary.
@cgianfra maybe you should review the 2.0 vs. 2.1 batch-api Maven JARs to see if there are any changes we should pull in.
Also the "batch.container.unavailable" message doesn't seem right to me. What does this have to do with <batchPersistence />
not being configured? Maybe this message has been dragged along since an early prototype?
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.
While the copyright holder (IBM) is unlikely to sue in this case ;-), more generally the Apache v2 license states You must cause any modified files to carry prominent notices stating that You changed the files
.
6062a5d
to
7a5b8ab
Compare
The build jhanders34-21535-20220624-1649 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_rCtlAPQNEeyYXLZn0n3zUg |
f5fd36c
to
028ed73
Compare
The build jhanders34-21535-20220626-0728 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_280h4PVSEey3m9ea3wJj4A |
....appserver.features/visibility/public/appSecurity-5.0/io.openliberty.appSecurity-5.0.feature
Show resolved
Hide resolved
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.
JSONB changes look good
26ee719
to
55a60da
Compare
@@ -13,7 +13,7 @@ bVersion=1.0 | |||
-nouses=true | |||
|
|||
Bundle-SymbolicName: io.openliberty.jakarta.pages.tld.3.0 | |||
Bundle-Description: Jakarta Standard Tag Library, version 2.0 | |||
Bundle-Description: Jakarta Standard Tag Library, version 3.0 |
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.
This is actually version 2.0, but the bundle name actually refers to 3.0 version in pages 3.0, not the underlying Tags technology. I don't know why we did this. I think we should rename to bundle now that we have Jakarta Tags 3.0 out.
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.
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.
So I should leave it be 2.0?
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.
I put it back to 2.0
55a60da
to
ea102a6
Compare
The build jhanders34-21535-20220627-1251 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_Holo0PZJEey3m9ea3wJj4A |
The build jhanders34-21535-20220627-1353 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_ieO78PZREey3m9ea3wJj4A |
#libby |
ea102a6
to
3fc2fb9
Compare
- Add missing api bundles except for a few which don't use the ones from jakarta group. - Update the version for the EE9 API bundles so it doesn't pull in the EE10 API artifacts - Update the jakarta version properties for the transformer to be right for EE9, EE10 and the EE9/EE10 file. - Update servlet and json b to their final API version - Change to export servlet APIs as 6.0 instead of 5.0. - Update buildpath and import statements to work with the new API versions.
3fc2fb9
to
1be2022
Compare
Code analysis and actionsDO NOT DELETE THIS COMMENT.
|
Joe's DHE check: ✔️ I found your oss_ibm.maven GAV artifacts on DHE. Thank you! ✔️ |
The build jhanders34-21535-20220628-0332 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_FFgLIPbEEey3m9ea3wJj4A |