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

org.slf4j dependency imports osgi packages at 2.0 #6354

Closed
tjwatson opened this issue Jun 3, 2021 · 26 comments · Fixed by #6356, #6381 or #6395
Closed

org.slf4j dependency imports osgi packages at 2.0 #6354

tjwatson opened this issue Jun 3, 2021 · 26 comments · Fixed by #6356, #6381 or #6395
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@tjwatson
Copy link
Contributor

tjwatson commented Jun 3, 2021

Jetty version
10.0.x

Java version/vendor (use: java -version)
All

OS type/version
All

Description
The jetty JARs are packaged as OSGi bundles. The ones that require slf4j have an Import-Package header in their META-INF/MANIFEST.MF that have the following specified:

Import-Package: ... org.slf4j;version="[2.0.0,3)"

When using such JARs as bundles in an OSGi framework this forces the installation of a the slf4j 2.0 (which currently is only available as alpha) in order to resolve the jetty bundles. I've been told jetty should work with the current stable released versions of slf4j (version 1.x). If this is true then the imports should be updated to include the minimum version jetty must have.

Also, if slf4j is entirely optional then the imports should be declared as optional (resolution:=optional)

@joakime
Copy link
Contributor

joakime commented Jun 3, 2021

Also, if slf4j is entirely optional then the imports should be declared as optional (resolution:=optional)

slf4j-api is not optional, but it the version range should be fixed to be org.slf4j;version="[1.7.0,3)"

@jmcc0nn3ll
Copy link
Contributor

Are those versions still computed by the bundle plugin at build time? We still need to release distro with 2.0.0 for the jpms start option works, correct?

tjwatson added a commit to tjwatson/jetty.project that referenced this issue Jun 3, 2021
This updates the parent pom to set the range for all
imports of slf4j by the jetty bundles

Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
@joakime joakime linked a pull request Jun 3, 2021 that will close this issue
joakime added a commit that referenced this issue Jun 3, 2021
Fixes #6354 - allow jetty bundles to use 1.7 slf4j
@tjwatson
Copy link
Contributor Author

tjwatson commented Jun 4, 2021

Is there any chance a release of Jetty 10.0.x can be done to include this fix? I'm currently trying to evaluate the feasibility of us including this fix for the Eclipse 2021-06 release of the Eclipse release train.

@sbordet
Copy link
Contributor

sbordet commented Jun 4, 2021

@tjwatson we're in the process of staging new versions.
After testing, we may release middle/end next week.

@joakime
Copy link
Contributor

joakime commented Jun 4, 2021

@tjwatson there is a SNAPSHOT repository that currently has this fix.
Can you use that to test the behavior?

Snapshot Repository - https://oss.sonatype.org/content/repositories/jetty-snapshots/
Use 10.0.4-SNAPSHOT

I've verified that the latest maven internal SNAPSHOT version has the desired fix.

[joakim@hyperion snapshots]$ curl -O https://oss.sonatype.org/content/repositories/jetty-snapshots/org/eclipse/jetty/jetty-home/10.0.4-SNAPSHOT/jetty-home-10.0.4-20210604.022623-9.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 9387k  100 9387k    0     0  11.9M      0 --:--:-- --:--:-- --:--:-- 11.9M
[joakim@hyperion snapshots]$ tar -zxf jetty-home-10.0.4-20210604.022623-9.tar.gz 
[joakim@hyperion snapshots]$ cd jetty-home-10.0.4-SNAPSHOT/
[joakim@hyperion jetty-home-10.0.4-SNAPSHOT]$ java -jar start.jar --list-config | grep build
 jetty.build = fbbe584a303bc47e1c542f82011df2bebbe3579f

That commit: fbbe584 happens to be your exact change.

@tjwatson
Copy link
Contributor Author

tjwatson commented Jun 4, 2021

Thanks, I confirmed that using the jetty bundles from this snapshot build allows me to remove the slf4j 2.0.0 library and only have slf4j 1.7.x. The Eclipse help system is functional using these jetty bundles also.

@tjwatson
Copy link
Contributor Author

tjwatson commented Jun 7, 2021

Thanks for the support on this one. Can we get a more firm timeline on when 10.0.4 will be released? From our side we want to be ready to consume/test with it as soon as we can so we can get 2021-06 release out the door. Thanks.

@joakime
Copy link
Contributor

joakime commented Jun 7, 2021

The 10.0.4 release is underway, it's currently staged on oss.sonatype.org and being tested.
We'll either promote it, or reroll it with fixes, this week.

@joakime
Copy link
Contributor

joakime commented Jun 9, 2021

@tjwatson the 10.0.4 release is now available on maven central.

@nitind
Copy link

nitind commented Jun 9, 2021

It looks like the downloadable jetty-jndi-10.0.4.jar and jetty-plus-10.0.4.jar still refer to 2.0 in their lower bounds, causing at least the generated p2 sites to still contain slf4j.api_2.0.0.alpha1.jar.

@joakime joakime reopened this Jun 9, 2021
@joakime
Copy link
Contributor

joakime commented Jun 9, 2021

I've reopened this ...

A quick analysis of 10.0.x HEAD shows the following are not correctly set ...

rootPath is /home/joakim/code/jetty/jetty.project-10.0.x
[jetty-alpn/jetty-alpn-client/target/jetty-alpn-client-10.0.5-SNAPSHOT.jar]  org.slf4j;version="[2.0.0,3)"
[jetty-alpn/jetty-alpn-conscrypt-client/target/jetty-alpn-conscrypt-client-10.0.5-SNAPSHOT.jar]  org.slf4j;version="[2.0.0,3)"
[jetty-alpn/jetty-alpn-conscrypt-server/target/jetty-alpn-conscrypt-server-10.0.5-SNAPSHOT.jar]  org.slf4j;version="[2.0.0,3)"
[jetty-annotations/target/jetty-annotations-10.0.5-SNAPSHOT.jar]  org.slf4j;version="[2.0.0,3)"
[jetty-jndi/target/jetty-jndi-10.0.5-SNAPSHOT.jar]  org.slf4j;version="[2.0.0,3)"
[jetty-osgi/jetty-osgi-boot/target/jetty-osgi-boot-10.0.5-SNAPSHOT.jar]  org.slf4j;resolution:=optional;version="[2.0.0,3)"
[jetty-osgi/test-jetty-osgi-context/target/test-jetty-osgi-context-10.0.5-SNAPSHOT.jar]  org.slf4j;resolution:=optional;version="[2.0.0,3)"
[jetty-osgi/test-jetty-osgi-server/target/test-jetty-osgi-server-10.0.5-SNAPSHOT.jar]  org.slf4j;resolution:=optional;version="[2.0.0,3)"
[jetty-plus/target/jetty-plus-10.0.5-SNAPSHOT.jar]  org.slf4j;version="[2.0.0,3)"

All of those modules have specific Import-Package declarations which seems to have gotten in the way.

Some examples:

https://github.com/eclipse/jetty.project/blob/f171a5ce61ab8fb0bae3510f5828f4f53af2997f/jetty-jndi/pom.xml#L34

https://github.com/eclipse/jetty.project/blob/f171a5ce61ab8fb0bae3510f5828f4f53af2997f/jetty-plus/pom.xml#L26

@joakime
Copy link
Contributor

joakime commented Jun 9, 2021

Does the Eclipse IDE need jetty-jndi and jetty-plus? I cannot imagine why.

@joakime joakime changed the title org.slfj dependency imports packages at 2.0 org.slfj4h dependency imports packages at 2.0 Jun 9, 2021
@joakime joakime changed the title org.slfj4h dependency imports packages at 2.0 org.slfj4 dependency imports packages at 2.0 Jun 9, 2021
@joakime joakime changed the title org.slfj4 dependency imports packages at 2.0 org.slf4j dependency imports packages at 2.0 Jun 9, 2021
@joakime
Copy link
Contributor

joakime commented Jun 9, 2021

@tjwatson shouldn't the definition in the manifest be all of the packages exposed by slf4j-api-<ver>.jar?

org.slf4j;version="[1.7,3.0)", org.slf4j.event;version="[1.7,3.0)", org.slf4j.helpers;version="[1.7,3.0)", org.slf4j.spi;version="[1.7,3.0)"

@nitind
Copy link

nitind commented Jun 9, 2021

@joakime Speaking for Eclipse Web Tools and not the Eclipse Platform, we use jetty-plus to fulfill jetty-webapp's Require-Capability: osgi.extender;filter:="(osgi.extender=osgi.servicelo ader.processor)",osgi.serviceloader;cardinality:=multiple;filter:="(osg i.serviceloader=org.eclipse.jetty.webapp.Configuration)" manifest entry. It's jetty-webapp that we require.

joakime added a commit that referenced this issue Jun 9, 2021
… be 1.7

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime added the Bug For general bugs on Jetty side label Jun 9, 2021
@joakime joakime added this to To do in Jetty 10.0.5/11.0.5 FROZEN via automation Jun 9, 2021
@joakime
Copy link
Contributor

joakime commented Jun 9, 2021

PR #6381 opened to attempt to address this.

joakime added a commit that referenced this issue Jun 9, 2021
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
janbartel added a commit that referenced this issue Jun 10, 2021
Signed-off-by: Jan Bartel <janb@webtide.com>
@gregw gregw moved this from To do to In progress in Jetty 10.0.5/11.0.5 FROZEN Jun 10, 2021
joakime added a commit that referenced this issue Jun 10, 2021
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Jetty 10.0.5/11.0.5 FROZEN automation moved this from In progress to Done Jun 10, 2021
sbordet pushed a commit that referenced this issue Jun 10, 2021
… be 1.7 (#6381)

Issue #6354 - OSGI manifest for slf4j-api packages lower limit should be 1.7

* Fixed OSGi manifest in all jars.
* Make osgi tests work with slf4j < 2.0.0. 
This required to remove the dependency on SLF4J from the demos.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Co-authored-by: Jan Bartel <janb@webtide.com>
@sbordet
Copy link
Contributor

sbordet commented Jun 10, 2021

@tjwatson @nitind @jonahgraham we have deployed Jetty 10.0.5-SNAPSHOT.

Can you please try it out and report back if it works?
https://oss.sonatype.org/content/repositories/jetty-snapshots/

joakime added a commit that referenced this issue Jun 10, 2021
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
sbordet pushed a commit that referenced this issue Jun 10, 2021
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@sravanlakkimsetti
Copy link

Source bundles of the following contains .java.orig files. I think these were temporary files
jetty-http
jetty-io
jetty-server and
jetty-util

@sbordet sbordet linked a pull request Jun 10, 2021 that will close this issue
@sbordet
Copy link
Contributor

sbordet commented Jun 10, 2021

@sravanlakkimsetti we had a temporary glitch on our CI network, but we wanted you guys to be able to test, so I did a deploy from my laptop, and I guess those files were wrongly included (they are the side effect of git merges).

@sravanlakkimsetti
Copy link

Thank you for the information. I already started Platform build it should be ready in 3 hours time. We should be able to test these bundles shortly.

@sbordet
Copy link
Contributor

sbordet commented Jun 10, 2021

FYI I did another deploy as we discovered another smaller issue in the OSGi manifests, #6395.

The latest snapshot is has this timestamp:
jetty-<module>-10.0.5-20210610.16mmss-N.jar

The previous snapshot had:
jetty-<module>-10.0.5-20210610.15mmss-N.jar

Note 15 vs 16 as the hour in the file name.

@sbordet
Copy link
Contributor

sbordet commented Jun 11, 2021

@tjwatson @nitind @jonahgraham @sravanlakkimsetti how did the testing go?

Note that we can stage the release into a staging, non-snapshot, repository if that makes things simpler for you.
We are waiting on your feedback to stage and complete the release process.

@jonahgraham
Copy link

@sbordet - things are looking pretty good within the IDE. Overnight (for me) last night we got some positive reports based on the respun builds using 10.0.5 snapshot.

IDE builds now are free of slf4j and the Help system works. Here are the report links:

Thank you again for the fast turnaround times on this. The Eclipse Project and SimRel participants will collectively respin for release once we get the final 10.0.5 from Jetty.

Thanks!

@sbordet
Copy link
Contributor

sbordet commented Jun 11, 2021

@jonahgraham thanks! We are resuming the Jetty release process, will let you know here when it's completed.

@joakime joakime changed the title org.slf4j dependency imports packages at 2.0 org.slf4j dependency imports osgi packages at 2.0 Jun 11, 2021
@joakime
Copy link
Contributor

joakime commented Jun 11, 2021

@jonahgraham Jetty 10.0.5 release is now in maven central.

@jonahgraham
Copy link

Thank you! Let the Eclipse IDE respins begin :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment