[LOGGING-189] Fix Import-Package entry for org.slf4j.

[wilx]

No description provided.

[wilx]

Cross reference: https://issues.apache.org/jira/projects/LOGGING/issues/LOGGING-189

[sebbASF]

I wonder if the upper version should be 3.0.0?

[wilx]

Well, I guess the author wanted all version between 1.7 and less than 3, i.e., all 2.x versions. Seems reasonable.

[sebbASF]

All the other entries use the same number of parts for the lower and upper limits.

[wilx]

All the other entries use the same number of parts for the lower and upper limits.

I don't think that matter to OSGi a tall.

[garydgregory]

Hello @wilx
Thank you for the PR.
How was this tested and how can we avoid breakage in the future?
How about adding a test like https://github.com/apache/commons-compress/blob/master/src/test/java/org/apache/commons/compress/OsgiITest.java here?

[wilx]

You can use a BND library macro like I did there. It results in "[1.7.0,3)" which is fine with OSGi.

[wilx]

While looking at the imports for the above, I noticed more possible problems. Like, the javax.servlet;version="[2.1.0, 3.0.0)";resolution:=optional, import and its constraint. That should be "[2.1.0,5)" because the 4.0 servlet is compatible with the 2.1.

Is there a log4j2 version 3.x already? Because the constraint here org.apache.logging.log4j;version="[2.0, 4.0)";resolution:=optional, suggests that. It allows >=2 and <4, implying version 3.

[garydgregory]

@wilx
Thank you for your updates. The issue is that there is no way to validate the changes in your PR without a matching test, see #188 (comment)
This means that a future innocent-looking change could break this and cause a regression.
The test should fail without your changes to the POM.

[garydgregory]

Ping @wilx 😀

[HannesWell]

I just noticed that also the import of the org.slf4j.spi package is missing, but it is referenced in

commons-logging/src/main/java/org/apache/commons/logging/impl/Slf4jLogFactory.java

Line 36 in bded390

import org.slf4j.spi.LocationAwareLogger;

This leads to classloading errors when it is attempted to load the Slf4jLogFactory in an OSGi runtime (see e.g. eclipse-mylyn/org.eclipse.mylyn#373).
Would you mind to add that package as well with this PR?

[HannesWell]

Suggested change

	org.slf4j;version="[1.7.0, $(versionmask;+;${slf4j.version}))";resolution:=optional
	org.slf4j;version="[1.7.0, $(versionmask;+;${slf4j.version}))";resolution:=optional,
	org.slf4j.spi;version="[1.7.0, $(versionmask;+;${slf4j.version}))";resolution:=optional

[HannesWell]

Are there any updates on this change?
As it is, common-logging is hardly usable in OSGi runtimes like Eclipse if the new SLF4JLogFactory is used.

If desired I can create an alternative PR to fix this.

[ppkarwasz]

Is there a log4j2 version 3.x already? Because the constraint here org.apache.logging.log4j;version="[2.0, 4.0)";resolution:=optional, suggests that. It allows >=2 and <4, implying version 3.

There are already beta version of 3.x that have no breaking changes in the API. However we decided recently that log4j-api will stay at version 2.x, so the range should be corrected to [2.0, 3).

[HannesWell]

Thanks @garydgregory for pushing this forward.

I just noticed that also the import of the org.slf4j.spi package is missing, but it is referenced in

commons-logging/src/main/java/org/apache/commons/logging/impl/Slf4jLogFactory.java

Line 36 in bded390

import org.slf4j.spi.LocationAwareLogger;

This leads to classloading errors when it is attempted to load the Slf4jLogFactory in an OSGi runtime (see e.g. eclipse-mylyn/org.eclipse.mylyn#373). Would you mind to add that package as well with this PR?

Has this already been addressed anywhere else?

[garydgregory]

Hello @HannesWell CC @ppkarwasz

I do not know. Please test and/or provide a PR. I am about to create a release candidate for 1.3.1. I can always do a 1.3.2 but the turnaround on a release is at least 4 days.