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

[joakime]

The existing OSGi manifests for org.slf4j in Jetty 10.0.4 are broken.

They list only 1 out of the 4 packages that need import, and a few or our modules did not pull in the updated version range properly.

This PR fixes the top level configuration to include all 4 packages, and uses it in modules that declare their own Import-Package specifics.

Signed-off-by: Joakim Erdfelt [email protected]

[joakime]

The Eclipse IDE side issue that needs the OSGI manifest fix from this PR is ...

https://bugs.eclipse.org/bugs/show_bug.cgi?id=573454

[jonahgraham]

@nitind this appears to resolve the WTP problem that "jetty-jndi and jetty-plus did not have their SLF4J ranges lowered under 2.0". Are you able to test to see if the 10.0.5 snapshot that @joakime put out resolves that and means that no slf4j 2 ends up in https://download.eclipse.org/webtools/jetty/10.0.4/repository/plugins/ equivalent?

[joakime]

I haven't pushed the SNAPSHOT with the PR changes yet.

[nitind]

@jonahgraham I should be able to once the snapshots are up, but the process we use for our build doesn't modify the manifests aside from embedding SHA-256-Digests within them, so what Jetty creates is basically what we use.

[joakime]

I had to push a small fix in commit 8193af6 - i'm going to wait for CI to give a green light before i build the snapshot for you.

[olamy]

@nitind @jonahgraham snapshots deployed to https://oss.sonatype.org/content/repositories/jetty-snapshots

[nitind]

@nitind @jonahgraham snapshots deployed to https://oss.sonatype.org/content/repositories/jetty-snapshots

All of the manifests look good to me. Output from my local build still includes a slf4j.api_2.0.0.alpha1.jar, but that must be because it's technically newer and not a snapshot.

[olamy]

@nitind feel free to approve the PR ;)

[jonahgraham]

@nitind @jonahgraham snapshots deployed to https://oss.sonatype.org/content/repositories/jetty-snapshots

All of the manifests look good to me. Output from my local build still includes a slf4j.api_2.0.0.alpha1.jar, but that must be because it's technically newer and not a snapshot.

I suspect there are two reasons it is included:

1. If you based your build off of what Eclipse Platform did, then the category.xml explicitly lists org.slf4j
2. The version it pulls in is presumably the version that the Jetty's pom.xml's dependencyManagement - see: https://github.com/eclipse/jetty.project/blob/8193af67ef800fe4263274ab5dc96b73b442c329/pom.xml#L22 and https://github.com/eclipse/jetty.project/blob/8193af67ef800fe4263274ab5dc96b73b442c329/pom.xml#L1166-L1170

If that sounds correct, then perhaps this change to Platform's build can be applied to WTPs too? https://git.eclipse.org/r/c/platform/eclipse.platform.releng.buildtools/+/181742

[janbartel]

We need to make sure that the osgi tests run against slf4j 1.7.30 to have some kind of assurance that indeed jetty will deploy into osgi against an slf4j version lower than 2.0.0. I've done the changes to do that and committed to this PR: it wasn't pretty. Note that the demo-jetty-webapp was relying on slf4j 2.0.0, along with the jetty-slf4j-logging impl: I've had to remove that reliance because whilst it's perfectly possible to deploy both slf4j-api 2.0.0 and slf4j-api-1.7.30 at the same time, it isn't possible to ensure that all our jars only resolve against the 1.7.30 (ie what we're trying to test!). So I've changed the demo-jetty-webapp to only log to the servlet context logger. I'm not even sure that any logging is necessary at all - we should review that.

I've made the test-jetty-osgi module use slf4j 1.7.30, and now all of the tests run correctly, so we have some level of assurance that this fix works. Unfortunately, changing the logging level in the simplelogger.properties file I've provided isn't changing the jetty logging, so it's now harder to debug osgi tests. I think there's some conflict with pax-exam slf4j logging, but that needs further investigation.

So, AFAICT the fix in this PR works, however, I think it is an extremely fragile fix: the next person who creates a jetty module that specifies a custom Import statement in the pom.xml probably won't know that they need to use this special ${osgi.common.import.packages} value to make logging work.

A far better solution would be to make all of jetty rely on the lowest level of slf4j at build time (ie 1.7.30), and then make sure that with jetty.home we deploy against the higher version of slf4j (2.0.0). Note that we would need to change the jetty-slf4j-logging implementation to work against 1.7.30 - currently it doesn't because the way 1.7.30 finds its impl is different to 2.0.0 - I've raised issue #6385.

So now that I've committed code to this branch, I think we need to ask somebody else to review this.

[sbordet]

This property should be renamed to slf4j.import.packages.

[joakime]

I considered that, as that is how it is currently used.
But using the more generic osgi.common.* prefix means we can add to this list for other reasons without having to touch all of the locations that use this new property again.

[joakime]

How about osgi.slf4j.import.packages?
I don't what to drop the osgi portion.

[joakime]

Changed to osgi.slf4j.import.packages instead in commit 4720f07

[jonahgraham]

Thank you Jetty team for all your work supporting the 2021-06 Eclipse SimRel release! Can you give us an ETA / time-range for a possible 10.0.5 release? The 2021-06 ship date is Wed 16 and the planning council needs some guidance so that we can start planning for a slip on the release day if needed. AFAICT the release engineering to respin is happening in (at least) 4 very disparate different timezones (Australia, India, Germany, Canada). Thanks again!

[sbordet]

@jonahgraham we are processing the release in #6390.

I'm merging the latest PR and then I will deploy a 10.0.5-SNAPSHOT; I will notify here when the snapshot is deployed.

Will be great if you can thoroughly test that snapshot and confirm that it works.