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

Complete the upgrade to servlet6 and grizzly4 #5006

Closed
dmatej opened this issue Mar 17, 2022 · 37 comments · Fixed by #5009
Closed

Complete the upgrade to servlet6 and grizzly4 #5006

dmatej opened this issue Mar 17, 2022 · 37 comments · Fixed by #5009

Comments

@dmatej
Copy link
Contributor

dmatej commented Mar 17, 2022

@dmatej
Copy link
Contributor Author

dmatej commented Mar 17, 2022

Working on that ... it will need 4.0.0-M1 of grizzly.

@arjantijms
Copy link
Contributor

Good!

dmatej added a commit to dmatej/jersey that referenced this issue Mar 28, 2022
dmatej added a commit to dmatej/jersey that referenced this issue Mar 28, 2022
senivam pushed a commit that referenced this issue Mar 28, 2022
@arjantijms
Copy link
Contributor

Are we there yet to release an -M4 of Jersey? @jansupol ?

@jansupol
Copy link
Contributor

@arjantijms Including CDI 4 support or without?

@arjantijms
Copy link
Contributor

A milestone of the current 3.1 branch (https://github.com/eclipse-ee4j/jersey/tree/3.1) would be great. Any further additions could then go to a M5.

In what way is CDI support lacking currently in the state of the 3.1 branch? You mean the dependencies are still set to CDI 3?

@jansupol
Copy link
Contributor

jansupol commented Mar 30, 2022

Yes, the deps are set to CDI 3, right now. We have the OSGi headers set to accept both CDI 3 and CDI 4. That is ok. I have an open PR to update CDI.

The issue is with bean validation, For the CDI 4, hibernate validator 8 (and EL 5) is required (by OSGi). On the other hand, it only affects jersey-bean-validation.jar which might not be used by GF at all.

@jansupol
Copy link
Contributor

Also, I can see this SNAPSHOT dependency - Please know that Arguillian cannot be used for the deployment with GF 7 M2 & M3 because of this.

@jansupol
Copy link
Contributor

There would be an issue with CDI 4, too, with current 3.1 branch - A deprecated method is still used, see #5016

@arjantijms
Copy link
Contributor

Also, I can see this SNAPSHOT dependency - Please know that Arguillian cannot be used for the deployment with GF 7 M2 & M3 because of this.

I hear you, it's super annoying to have these. In general there's a lot of chicken and egg going on, where various projects depend on each other and we can't make progress otherwise, or would have to wait for months when each individual project makes a move.

I made a special note about these dependencies here: https://github.com/eclipse-ee4j/glassfish/releases/tag/7.0.0-M3

You can use GF with Arquillian, but obviously have to build that snapshot dependency locally first. It's absolutely not great, I know, hence why we are trying to resolve them as quick as possible (which still take weeks often).

@senivam
Copy link
Contributor

senivam commented Mar 30, 2022

Hi Arjan, the 3.1.0-M4 version of Jersey is in staging. (@arjantijms)

@arjantijms
Copy link
Contributor

@senivam thanks for the update! I'll integrate it in GF 7 asap. Thx!

@arjantijms
Copy link
Contributor

@senivam still one small issue with M4:

Unable to resolve
    org.glassfish.main.admin.rest-service [277]
    missing requirement
        &(package = org.glassfish.jersey.inject.hk2) (version >= 3.1.0) (!(version >= 4.0.0))
        caused by:
            Unable to resolve
                org.glassfish.jersey.inject.jersey-hk2 [83]
                missing requirement
                    &(package = org.glassfish.jersey.inject.hk2) (version >= 3.1.0) (!(version >= 4.0.0)))]

@arjantijms
Copy link
Contributor

I guess "org.glassfish.jersey.inject.jersey-hk2" should link to the latest HK2 release instead of an older version?

@jansupol
Copy link
Contributor

There is no new HK2 (major), still version 3.
This sounds like org.glassfish.main.admin.rest-service depends on Jersey 3.1, but it is not on classpath. Maybe it was not loaded by the OSGi Framework, because of some reason...

@jansupol
Copy link
Contributor

Could Import-Package: jakarta.inject;version="[2.0,3)" be the issue?

@jansupol
Copy link
Contributor

I have downloaded the GF (from eclipse-ee4j/glassfish#23876):

$asadmin start-domain

Cannot find a default implementation of the HK2 ServiceLocatorGenerator
Exception in thread "main" java.lang.IllegalStateException: No generator was provided and there is no default generator registered
        at org.glassfish.hk2.internal.ServiceLocatorFactoryImpl.internalCreate(ServiceLocatorFactoryImpl.java:285)
        at org.glassfish.hk2.internal.ServiceLocatorFactoryImpl.create(ServiceLocatorFactoryImpl.java:245)
        at org.glassfish.hk2.internal.ServiceLocatorFactoryImpl.create(ServiceLocatorFactoryImpl.java:202)
        at com.sun.enterprise.module.common_impl.AbstractModulesRegistryImpl.newServiceLocator(AbstractModulesRegistryImpl.java:118)
        at com.sun.enterprise.module.common_impl.AbstractModulesRegistryImpl.createServiceLocator(AbstractModulesRegistryImpl.java:197)
        at com.sun.enterprise.module.common_impl.AbstractModulesRegistryImpl.createServiceLocator(AbstractModulesRegistryImpl.java:203)
        at com.sun.enterprise.module.single.StaticModulesRegistry.createServiceLocator(StaticModulesRegistry.java:65)
        at com.sun.enterprise.admin.cli.CLIContainer.getServiceLocator(CLIContainer.java:194)
        at com.sun.enterprise.admin.cli.CLIContainer.getLocalCommand(CLIContainer.java:232)
        at com.sun.enterprise.admin.cli.CLICommand.getCommand(CLICommand.java:183)
        at com.sun.enterprise.admin.cli.AdminMain.executeCommand(AdminMain.java:330)
        at com.sun.enterprise.admin.cli.AdminMain.doMain(AdminMain.java:271)
        at org.glassfish.admin.cli.AsadminMain.main(AsadminMain.java:33)

Looks like an HK2 issue more then Jersey

@dmatej
Copy link
Contributor Author

dmatej commented Mar 30, 2022

Nope ... I rebuilt jersey at bf4d9f4 and then glassfish with -Djersey.version=3.1.0-SNAPSHOT and it doesn't start from another reason:

[2022-03-30T20:12:27.887+0200] [glassfish 7.0] [SEVERE] [] [] [tid: _ThreadID=34 _ThreadName=Thread-8] [timeMillis: 1648663947887] [levelValue: 1000] [[
org.osgi.framework.BundleException: Unable to resolve org.apache.felix.scr [304](R 304.0): missing requirement [org.apache.felix.scr [304](R 304.0)] osgi.wiring.package; (&(osgi.wiring.package=org.osgi.framework)(version>=1.10.0)(!(version>=2.0.0))) Unresolved requirements: [[org.apache.felix.scr [304](R304.0)] osgi.wiring.package; (&(osgi.wiring.package=org.osgi.framework)(version>=1.10.0)(!(version>=2.0.0)))]
at org.apache.felix.framework.Felix.resolveBundleRevision(Felix.java:4398)
at org.apache.felix.framework.Felix.startBundle(Felix.java:2308)
at org.apache.felix.framework.Felix.setActiveStartLevel(Felix.java:1566)
at org.apache.felix.framework.FrameworkStartLevelImpl.run(FrameworkStartLevelImpl.java:308)
at java.base/java.lang.Thread.run(Thread.java:833)

I can try yet previous commit in several minutes ... seems like some transitive jersey dependency issue.

@jansupol
Copy link
Contributor

Sorry, what makes you think missing org.apache.felix.scr is Jersey related?

@dmatej
Copy link
Contributor Author

dmatej commented Mar 30, 2022

Also 3.1.0-M4 has hibernate-validator 8 in dependencies, gf uses 7.0.0-SNAPSHOT.

Sorry, what makes you think missing org.apache.felix.scr is Jersey related?

Because jersey version is the only change since the last successful build where gf maybe 100 times started :-)
But I still did not find the exact cause. I guess some transitive dependency (as glassfish breaks many standard maven rules and heavily relies on transitive dependencies - like other JEE projects)

@jansupol
Copy link
Contributor

Because jersey version is the only change since the last successful build where gf maybe 100 times started :-)

That's dodgy!

What we can do then, is to grab the latest working GF and try to deploy the new Jersey jars there...

@dmatej
Copy link
Contributor Author

dmatej commented Mar 30, 2022

Haha, I am wrong, I forgot on three other snapshots ... the jersey commit before your changes now failed from the same reason as the last build. Ok, so what is next ... probably we can try to do also other upgrades ...

@arjantijms
Copy link
Contributor

arjantijms commented Mar 30, 2022

@jansupol

It looks like the following is the concrete issue.

The MANIFEST.MF of jersey-hk2.jar contains the following line:

"Require-Capability: osgi.ee;filter:="(osgi.ee=UNKNOWN)"

GlassFish/Felix looks for a capability matching that, but the only ones we have are:

  • "OSGi/Minimum"
  • "JavaSE"

In full, these are:

  • [org.apache.felix.framework [0](R 0)] osgi.ee; {osgi.ee=OSGi/Minimum, version=[1.0.0, 1.1.0, 1.2.0]}
  • [org.apache.felix.framework [0](R 0)] osgi.ee; {osgi.ee=JavaSE, version=[1.0.0, 1.1.0, 1.2.0, 1.3.0, 1.4.0, 1.5.0, 1.6.0, 1.7.0, 1.8.0, 9.0.0, 10.0.0, 11.0.0, 12.0.0, 13.0.0, 14.0.0, 15.0.0, 16.0.0, 17.0.0]}

So we don't have a capability matching "UNKNOWN". Because of that resolving fails.

The question is, is "UNKNOWN" a mistake in that jar, or do we actually need a capability called "UNKNOWN"?

@dmatej
Copy link
Contributor Author

dmatej commented Mar 30, 2022

Shocking surprise - I tried to sync also with hibernate-validator 8.0.0.Alpha2, built too on jenkins, and it has the same issue:
Require-Capability: osgi.ee;filter:="(osgi.ee=UNKNOWN)"

@arjantijms
Copy link
Contributor

Oh dear...

@dmatej
Copy link
Contributor Author

dmatej commented Mar 30, 2022

And another surprise: hibernate-validator 8 build does it even on my laptop, at least when I used JDK17 for the build. So at least it is reproducible, tomorrow I will find out why :-)

@arjantijms
Copy link
Contributor

When I patch jersey-hk2.jar to contain

Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=11))"

The build and tests do pass: eclipse-ee4j/glassfish#23876

@senivam
Copy link
Contributor

senivam commented Mar 31, 2022

regarding the fix for the missing OSGi version I have a fix (which is maven-bundle-plugin version update from 3.5.0 to 5.1.4) in the another PR - #5001, but it's not merged yet.

@arjantijms
Copy link
Contributor

Shall I merge that PR, or do we need to wait for the CQ?

@senivam
Copy link
Contributor

senivam commented Mar 31, 2022

Hi Arjan, I would appreciate if @jansupol take a look to the PR prior to its merging. And, for sure, CQ also has to be resolved. I think, your manual patch for now is sufficient to keep GF going with the actual M4 version of Jersey, so we have a bit of time to prepare M5 with some more changes. Thank you for the patience.

@arjantijms
Copy link
Contributor

Thanks for your reply @senivam, looking forward to M5. Maybe I can help a little with it, but I also have many other things todo for GF and EE 10 still. Let me know if there's anything I can directly help with.

@jansupol
Copy link
Contributor

@arjantijms Is there any rush for Jersey M5? From my perspective, GF patched our issue - thanks - so the M5 is not required at the moment? Or does GF want to have M5 so that the patch is not required?

@arjantijms
Copy link
Contributor

@jansupol not a super big rush of course, but indeed, it's the latter reason.

Patches are always nasty to have in a project and generally some user will be confused by it in some way. So just trying to keep patches to a minimal and therefor hope to be able to use a Jersey version without needing to patch it.

@dmatej
Copy link
Contributor Author

dmatej commented Mar 31, 2022

@arjantijms Is there any rush for Jersey M5? From my perspective, GF patched our issue - thanks - so the M5 is not required at the moment? Or does GF want to have M5 so that the patch is not required?

I would not hurry so much now - I'm trying to push GF to another stable milestone, it would be nice to have M5 in dependencies rather than any custom hack or snapshot build, but it must be compatible with other parts. So let's say that tomorrow morning it would be perfect to have milestone releases for jersey and also hibernate which has the same problem (and also milestone for Metro-WSIT). I will push PR for both soon.

If not, we can still build a snapshot, but to avoid another issues with builds I will use my own repo which will be periodically updated after successful build with glassfish (or I will report/fix the issue before updating the branch, so any development in eclipse's repo will not break glassfish's build). There is always some way :-)

@jansupol
Copy link
Contributor

@dmatej, @arjantijms We will create M5 without any changes but the fixed OSGi headers so that GF release does not have any additional issues.
Regarding hibernate-validator, Jersey used 8.0.0.Alpha1, which has the OSGi header correct.

@senivam
Copy link
Contributor

senivam commented Mar 31, 2022

@dmatej , @arjantijms , @jansupol So, basically we have it / I mean Jersey 3.1.0-M5. I've just checked the OSGi versions in the MANIFEST.MF files and it looks like fixed. So, I presume, it's possible to integrate the M5 version into the GF.

@senivam
Copy link
Contributor

senivam commented Apr 1, 2022

just to keep related PRs tracked - PRs #5019 and #5021 were aimed to fix the issue with OSGi version, after the fix was applied the 3.1.0-M5 version of Jersey was released (to staging).

@jansupol
Copy link
Contributor

Link

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 a pull request may close this issue.

4 participants