[WICKET-6999] Update bundle plugin to support slf4j [1.7,3) #540
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattrpav
force-pushed
the
WICKET-6999
branch
from
cb1e94b
to
10fa846
Compare
|
At issue is the bundle plugin runs different at various maven phases. When combined with the enforcer plugin to restrict config to an execution, the bundle plugin definition will get chatty and redundant. Thoughts? |
mattrpav
force-pushed
the
WICKET-6999
branch
from
10fa846
to
f2edd61
Compare
|
Commit f2edd61 fixes bundle plugin execution phase+goal to generate correct MANIFEST and allow for enforcer plugin to be maintained. |
mattrpav
force-pushed
the
WICKET-6999
branch
from
f2edd61
to
b6ab0d5
Compare
|
Much better with per-module properties per 174065b! wicket-util: wicket-request: wicket-core: wicket-auth-roles wicket-jmx wicket-devutils wicket-extensions; |
|
NOTE: Generate export packages list for wicket-core Generate import package list |
mattrpav
force-pushed
the
WICKET-6999
branch
2 times, most recently
from
125eb6c
to
73bbe37
Compare
|
I've removed the DynamicImport-Package: * rule. I think that should only be needed by the application wars to support modular pages and allow Wicket to instantiate those classes from the war's classpath. |
mattrpav
force-pushed
the
WICKET-6999
branch
from
73bbe37
to
174065b
Compare
I think it would be better not to remove DynamicImport-Package: *, because of deserialization problems in OSGi env (Wicket need to know all classes serialized inside components from other bundles). It can be achieve in different way, but I think it requires more changes in the framework. |
@dstoch the actual application bundle would need to do that as they require. Further backing evidence is the Wicket 8.x bundles do not have 'DynamicImport-Package: *' specified. |
MANIFESTs are broken since Wicket 8.x, so it is not a good evidence ;). |
What is broken? We've been running unmodified bundles on Wicket 8.x for years. |
|
I described a problem in this issue description in JIRA. Eg. without exporting org.apache.wicket.markup.html.internal package is not possible to use Wicket in OSGi. Do you really use it in OSGi without problems? I wonder how is this possible, maybe you don't use the code which requires abstractions from this packages? |
Correct. We do not extend anything from that package. If you see in the above, the new OSGi wiring includes exporting that package from wicket-core. |
Keep in mind-- if you need to extend packages that have not been exported you can create a fragment bundle to attach your extension classes. Generally, things like 'internal' are not exported since they are not part of a proper API or SPI. |
In OSGi world it is indeed. But as I wrote in description in Wicket they are part of API (maybe these packages should not have "internal" word in name). |
Agreed, Wicket's API+SPI definition and encapsulation could be better. The API+SPI should be separated into its own jar and only include interfaces and POJO classes. I created https://issues.apache.org/jira/browse/WICKET-6855 to open discussion on this, but I didn't articulate the use case in a meaningful way. |
bitstorm
approved these changes
Oct 1, 2022
|
Let me get a couple more done. For sure we should have the web socket bundles in good shape |
martin-g
reviewed
Oct 3, 2022
pom.xml
Outdated
| <Import-Package>org.apache.wicket*, | ||
| org.junit.jupiter*</Import-Package> | ||
| <DynamicImport-Package>*</DynamicImport-Package> | ||
| <Require-Capability>osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=11.0))"</Require-Capability> |
There was a problem hiding this comment.
This probably should be version 17.0 for master and 11 for wicket-9.x
pom.xml
Outdated
| <_nouses>true</_nouses> | ||
| <!-- _noee>true</_noee --> |
There was a problem hiding this comment.
OsgiClashingPackagesTest.collectProjectPackages() is failing.
I'll look into that.
osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=11.0))"
This probably should be version 17.0 for master and 11 for wicket-9.x
Does Wicket master have runtime require on JDK 17?
This could be removed, right ?
This flag would prevent the Require-Capability header from being generated.
There was a problem hiding this comment.
For Wicket, I think it is safe to remove the Require-Capability header altogether. (ie.. use the _noee = true flag.
Web app users are going to need to know full stack to align JDK + Karaf + Pax Web + Wicket at all times. This header only provides a pre-wiring check to fast fail.
There was a problem hiding this comment.
Does Wicket master have runtime require on JDK 17?
Yes!
|
New feature to ease installs on Apache Karaf |
|
Ready for final review and/or merge I updated the rest of the modules-- I don't use guice, spring, velocity etc so don't have a test for them. The bundles look like they generated correctly. I did test installing websocket bundles and they wire up correctly. Osgi test passed as soon as the rest of the bundles were updated I added a karaf feature file to ease installation |
martin-g
approved these changes
Oct 5, 2022
|
Updated user guide w/ Apache Karaf feature info
|
martin-g
approved these changes
Oct 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finalize Require-Capability header yes|no, version various on branches?