Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 313502 - Bundles org.slf4j.api and org.slf4j.jcl breaks commons.logging
Summary: Bundles org.slf4j.api and org.slf4j.jcl breaks commons.logging
Status: RESOLVED FIXED
Alias: None
Product: Orbit
Classification: Tools
Component: bundles (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 blocker (vote)
Target Milestone: ---   Edit
Assignee: Orbit Bundles CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 313387 313732 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-05-19 06:40 EDT by Michael Pellaton CLA
Modified: 2010-10-06 03:54 EDT (History)
10 users (show)

See Also:


Attachments
Stack trace (8.73 KB, text/plain)
2010-05-19 06:42 EDT, Michael Pellaton CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Pellaton CLA 2010-05-19 06:40:53 EDT
Build Identifier: Eclipse 3.6.RC1 + Helios M7

The orbit SLF4J packages seem broken.

The SLF4J needs access to org.slf4j.impl.StaticLoggerBinder. However, this class is not present nor exported in either SLF4J bundles (it should be in the org.slf4j.jcl bundle under /org/slf4j/impl and the package needs to be exported).

The bundle org.slf4j.api needs to import this exported package.

The contents of the bundle org.slf4j.jcl don't seem to match with the original jar file from http://repo2.maven.org/maven2/org/slf4j/slf4j-jcl/1.5.11/


Bundle details (from headers in OSGi console):

Bundle-Name = SLF4J API
Bundle-SymbolicName = org.slf4j.api
Bundle-Version = 1.5.11.v20100419-1106
Export-Package = org.slf4j;version="1.5.11",org.slf4j.helpers;version="1.5.11",org.slf4j.spi;version="1.5.11"

Bundle-Name = SLF4J JCL 1.1.1 implementation over SLF4J
Bundle-SymbolicName = org.slf4j.jcl
Bundle-Version = 1.5.11.v20100419-1106
Export-Package = org.apache.commons.logging;version=1.1.1,   org.apache.commons.logging.impl;version=1.1.1
Import-Package = org.slf4j;version=1.5.11, org.slf4j.spi;version=1.5.11




Reproducible: Always

Steps to Reproduce:
1. unzip eclipse 3.6.rc1
2. install Mylyn from the helios miletsones repository
3. manually install the 2 slf4j bundles above from the helios miletsones repository (using p2 director)
4. start eclipse, open the task list view
5. -> attached stack trace :-(
6. uninstall the slf4j bundles
7. restart eclipse -> everything is fine :-)

The issue seems to become visible through an optional dependency of Mylyn or one of it's dependencies. Otherwise it would not work without the two bundles.
Comment 1 Michael Pellaton CLA 2010-05-19 06:42:03 EDT
Created attachment 169086 [details]
Stack trace
Comment 2 Gunnar Wagenknecht CLA 2010-05-19 06:54:08 EDT
(In reply to comment #0)
> Bundle-Name = SLF4J JCL 1.1.1 implementation over SLF4J
> Bundle-SymbolicName = org.slf4j.jcl

We discussed the BSN on orbit-dev [1] and decided on this naming scheme. I agree that this is confusing. My hope was that the bundle name should make it clear that this is not slf4j-jcl.jar but jcl-over-slf4j.jar.

The actual logger implementation must be provided to org.slf4j.api via a fragment. We have a set of logback bundles in Orbit for this task. 

[1] http://dev.eclipse.org/mhonarc/lists/orbit-dev/msg01605.html
Comment 3 Philippe Marschall CLA 2010-05-19 07:46:34 EDT
No, sorry. You break _everyone_ who does a

Import-Package = org.apache.commons.logging

because org.slf4j.jcl exports org.apache.commons.logging but is itself not runnable without fragments which must be installed manually with p2 director.
Comment 4 Gunnar Wagenknecht CLA 2010-05-19 09:44:46 EDT
(In reply to comment #3)
> because org.slf4j.jcl exports org.apache.commons.logging but is itself not
> runnable without fragments which must be installed manually with p2 director.

I asked in bug 177851 to check if there is a possibility to express such a requirement for p2.
Comment 5 Gunnar Wagenknecht CLA 2010-05-19 10:18:29 EDT
(In reply to comment #3)
> No, sorry. You break _everyone_ who does a
> 
> Import-Package = org.apache.commons.logging

No we don't. Works fine if you install a SLF4J logger implementation fragment. That's how SLF4J works. If SLF4J doesn't find a StaticLoggerBinder on its classpath it won't work. Note, that this behavior was recently modified in SLF4J 1.6. ([1])

Meanwhile, I updated the manifest headers for the SLF4J API bundle to define a requirement on a SLF4J logger implementation. I also modified the Logback bundle to provide such a capability.

Documented as well:
http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.orbit/org.slf4j.api/readme-bundles.txt?hideattic=0&revision=1.1.2.1.2.1.2.1&root=Tools_Project&view=markup

This will prevent the org.slf4j.api bundle from resolving at runtime. I'm still waiting on confirmation if this syntax is understood by p2. However, I consider this this just a workaround for the 1.5.x version of SLF4J. Once we provide 1.6.0 SLF4J will not yell at you when no implementation is available but default to a no-op logger. This is well documented in [1] and a user is expected to explicitly install a binding.

[1] http://slf4j.org/manual.html#libraries
Comment 6 Philippe Marschall CLA 2010-05-19 12:50:37 EDT
(In reply to comment #5)
> (In reply to comment #3)
> > No, sorry. You break _everyone_ who does a
> > 
> > Import-Package = org.apache.commons.logging
> 
> No we don't.

Here's what happened for us:
- Install stuff from Helios with update manager (or p2 director)
- one of the bundles has Import-Package = org.apache.commons.logging
- there are several bundles that provide this, for example org.apache.commons.logging or org.slf4j.jcl
- p2 picks org.slf4j.jcl
- at runtime the bundle will fail to load because org.slf4j.jcl can't load because org.slf4j.api can't load because a fragment is missing
- this could even happen if org.apache.commons.logging was installed and Equinox chose org.slf4j.jcl over org.apache.commons.logging

Do I understand you correctly that this is no problem because I can google, find this bug, invoke p2 director from the command line and install the missing fragment?
Comment 7 David Williams CLA 2010-05-19 13:01:08 EDT
This sounds complicated. And not that I understand it any better than either of you ... but, are you saying you'd be happy no matter where 
org.apache.commons.logging
comes from? 

Is that just an interface? 

Definitely sounds like org.slf4j.jcl needs to "work" out of the box, if its exporting such a common service.
Comment 8 David Williams CLA 2010-05-19 13:02:34 EDT
Gunnar, if you are making changes for this ... should it be marked as invalid? Or was that setting a mistake? (Hope I didn't do it accidentally :/
Comment 9 Michael Pellaton CLA 2010-05-19 13:35:28 EDT
Well, from a user - not a programmer point of view:

If I install something from the Helios site (let's say Mylyn and XText) and p2 is happy about the dependencies and installs everything, I absolutely expect the stuff to work.

That was THE big promise of p2. Fragments kind of invert dependencies and p2 has no chance to know that a fragment is needed and therefore resolves everything and then it does not work (as here).

p2 reads from the consuming bundle that it needs org.apache.commons.logging
and as slf4j exports this package, p2 resolves this to slf4j instead of a "real" bundle providing the "real" org.apache.commons.logging implementation.


In the p2 world everything is fine. However, nothing is fine at all and only at runtime can you find out that things don't work.

If dependencies are done using require-bundle, that's no problem. But if import package is used, every provider of a package must provide the contents expected by consumers, especially for such common stuff as org.apache.commons.logging
Comment 10 Daniel Le Berre CLA 2010-05-19 15:16:00 EDT
Hi guys,

The problem is worst than you think: you can break ECF (thus the update manager) with that bug (see bug 313387).

The problem is not specific to Mylyn, in my case it was Xtext.

SLF4J bundles metadata should be fixed ASAP, IMHO.
Comment 11 Daniel Le Berre CLA 2010-05-19 15:17:04 EDT
*** Bug 313387 has been marked as a duplicate of this bug. ***
Comment 12 Gunnar Wagenknecht CLA 2010-05-19 15:17:33 EDT
According to the steps provided SLF4J was installed manually. Therefore, care has to be taken to also install a binding. Frankly, that's how SLF4J works. A user has to select a binding manually. Even worse, SLF4J < 1.6 fails explicitly if no binding is available. Starting with 1.6 they fixed SLF4J so that it will continue to work.

It's indeed unfortunate that p2 selects the org.slf4j.jcl bundle. This is quite a heavy runtime change late in the cycle. Obviously, this is a combination that was not tested at all at runtimes. Bundle org.slf4j.jcl was introduced as an alternative to those that want to capture all ACL log messages via SLF4J into their prefered SLF4J logger. 

Thanks to hints from the p2 team I just committed and release some p2 instructions which should serve as a workaround to (hopefully) prevent p2 from installing a not working runtime combination. However, it still does not resolve the issue in general. Even worse, p2 might now download and install a full logging framework (eg. logback) as yet another component into an Eclipse IDE which is yet another untested runtime combination. 

I wonder if there is an option to tell p2 that this choice is more expensive and should be avoided? I'll open a separate bug to discuss that.
Comment 13 David Williams CLA 2010-05-19 15:25:47 EDT
What were the p2 instructions? In the bundle? 

Does anyone in Helios _require_ org.slf4j.jcl? 
If it it problematic enough, we could leave it out of orbit builds and repo, for now?
Comment 14 Gunnar Wagenknecht CLA 2010-05-19 15:32:25 EDT
(In reply to comment #13)
> What were the p2 instructions? In the bundle? 

Yes, META-INF/p2.inf in the bundle.

> Does anyone in Helios _require_ org.slf4j.jcl? 

Not sure. I think the Jetty team offers a "logging" feature.

> If it it problematic enough, we could leave it out of orbit builds and repo,
> for now?

Just a rebuild and we should be fine....
Comment 15 Pascal Rapicault CLA 2010-05-19 15:32:52 EDT
> It's indeed unfortunate that p2 selects the org.slf4j.jcl bundle. This is quite
> a heavy runtime change late in the cycle. Obviously, this is a combination that
> was not tested at all at runtimes. Bundle org.slf4j.jcl was introduced as an
> alternative to those that want to capture all ACL log messages via SLF4J into
> their prefered SLF4J logger. 
   To me the real issue is that the metadata for the bundle is incomplete or at least the bundle lies about what it provides. From there p2 does not have a way to distinguish between the cases.
  If an actual bundle is needed for the proper behavior of another then an explicit dependency needs to be specified.
Comment 16 Gunnar Wagenknecht CLA 2010-05-19 15:49:50 EDT
(In reply to comment #15)
>    To me the real issue is that the metadata for the bundle is incomplete or at
> least the bundle lies about what it provides. From there p2 does not have a way
> to distinguish between the cases.

That's an issue. In the case of SLF4J manual intervention is intended.

As state on [1]: "This [error] happens when no appropriate SLF4J binding could be found on the class path. Placing one (and only one) of slf4j-nop.jar, slf4j-simple.jar, slf4j-log4j12.jar, slf4j-jdk14.jar or logback-classic.jar on the class path should solve the problem."

>   If an actual bundle is needed for the proper behavior of another then an
> explicit dependency needs to be specified.

Done that. However, what happens then if two capabilities are available instead of just one? As noted above, there are plenty available. At some point we'd like to offer them all in the same repository. Which one will be picked by p2? Let's discuss this on bug 313614.
Comment 17 Gunnar Wagenknecht CLA 2010-05-19 15:52:58 EDT
(In reply to comment #16)
> As state on [1]: "This [error] happens when no appropriate SLF4J binding could
> be found on the class path. Placing one (and only one) of slf4j-nop.jar,
> slf4j-simple.jar, slf4j-log4j12.jar, slf4j-jdk14.jar or logback-classic.jar on
> the class path should solve the problem."

[1] http://www.slf4j.org/codes.html#StaticLoggerBinder
Comment 18 David Williams CLA 2010-05-19 16:01:12 EDT
(In reply to comment #15)

>    To me the real issue is that the metadata for the bundle is incomplete or at
> least the bundle lies about what it provides. From there p2 does not have a way
> to distinguish between the cases.
>   If an actual bundle is needed for the proper behavior of another then an
> explicit dependency needs to be specified.

I agree. As a possible alternative to explicit dependency, perhaps some higher level unit (bundle?) needs to be provided that provides the complete solution. 

Pascal, I assume it was you that reviewed/suggested the p2.inf solution? And you agree its correct and adequate for this case? 

I think we need to take very seriously the maxim to "first, do no harm" and if this bundle does harm (or has a possibility of doing so), then it should be pulled. And, I don't feel qualified to review its correctness.
Comment 19 Andrew Niefer CLA 2010-05-19 16:45:04 EDT
(In reply to comment #18)
> Pascal, I assume it was you that reviewed/suggested the p2.inf solution? And
> you agree its correct and adequate for this case? 

I suggested this over in bug 177851 comment #9, I haven't review the actual details of the change beyond what was posted in IRC which looked ok to me.

<kreismeister>	now I'm about to add p2.inf manually to express the dependency
<kreismeister>	requires.0.namespace = org.slf4j.api
<kreismeister>	requires.0.name = org.slf4j.impl.StaticLoggerBinder
<kreismeister>	requires.0.range = [1.5.5,1.5.11]
<kreismeister>	and in the fragment it will be:
<kreismeister>	provides.0.namespace = org.slf4j.api
<kreismeister>	provides.0.name = org.slf4j.impl.StaticLoggerBinder
<kreismeister>	provides.0.version = 1.5.11
Comment 20 Michael Pellaton CLA 2010-05-19 17:43:55 EDT
In reply to comment #12:
slf4j was only manually installed as part of the attempt to track down the problem which started when we installed Mylyn and xtext from the Helios milestone repo at the same time into an eclipse sdk 3.6.rc1.

By playing with Mylyn and un/installing the two slf4j bundles with p2 we could see that they are the culprits causing the problem.

When installing xtext, I get the following dependency-chain from xtext to slf4j:
org.eclipse.xtext --> org.eclipse.emf.mwe.core --> org.slf4j.jcl


It just happened that the problem appeared with mylyn first, but I am certain that some functionality of xtext would not work neither as it imports org.apache.commons.logging too and I assume it expects something else than the orbit slf4j bundle provides.



In reply to comment #13:
Yes, at least jetty seems to use it optionally:
org.mortbay.jetty.util --> org.slf4j.api
Comment 21 David Williams CLA 2010-05-19 21:45:55 EDT
Thanks for reviewing Andrew. 

I've changed the title to (try) and be more descriptive. 

While this won't be in /helios/releases until next week, I hope some can test it early. Perhaps by adding the "committers" build repo to list of sites? 

http://download.eclipse.org/tools/orbit/committers/drops/S20100519200754/repository

Thanks all.
Comment 22 Gunnar Wagenknecht CLA 2010-05-20 02:21:17 EDT
(In reply to comment #21)
> While this won't be in /helios/releases until next week, I hope some can test
> it early. Perhaps by adding the "committers" build repo to list of sites? 

Yes. It would be very helpful to get this verified before the next RC.

(In reply to comment #20)
> When installing xtext, I get the following dependency-chain from xtext to
> slf4j:
> org.eclipse.xtext --> org.eclipse.emf.mwe.core --> org.slf4j.jcl

Did you have org.slf4j.api already installed? According to comments in bug 313614 this particular choice should only happen *if* org.slf4j.jcl is the only bundle necessary to satisfy the dependency "org.apache.commons.logging". However, I can only imagine this being the case if org.slf4j.api was already installed previously. Please report any details on bug 313614.

(In reply to comment #18)
> I agree. As a possible alternative to explicit dependency, perhaps some higher
> level unit (bundle?) needs to be provided that provides the complete solution. 

Not sure. I still think this is dangerous because we enforce people to use (and stick with) a particular combination. However, on the other hand we pray that OSGi is all about modularity and choice and that people don't need to take a particular implementation but can pick their own. That's what "import package" is all about. Now if we were to provide a higher unit, we'd loose the choice and flexibility.

> I think we need to take very seriously the maxim to "first, do no harm" and if
> this bundle does harm (or has a possibility of doing so), then it should be
> pulled. And, I don't feel qualified to review its correctness.

Frankly, "org.slf4j.jcl" was never intended to be picked up by p2 automatically to be installed within the IDE. It's intended as a target platform component for producers of products/applications that rely on capturing all log events through SLF4J (usually server applications). If possible I'm ok with pulling it from the Helios repo (together with the other SLF4J log-capturing bundles). But we can't pull it from Orbit.
Comment 23 Michael Pellaton CLA 2010-05-20 03:04:47 EDT
(In reply to comment #21)
> While this won't be in /helios/releases until next week, I hope some can test
> it early. Perhaps by adding the "committers" build repo to list of sites? 
> http://download.eclipse.org/tools/orbit/committers/drops/S20100519200754/repository

I installed Mylyn in Eclipse (worked) and then added slf4j with p2:

/home/mpellato/java/eclipse/eclipse-3.6RC1/eclipse 
  -application org.eclipse.equinox.p2.director 
  -repository http://download.eclipse.org/tools/orbit/committers/drops/S20100519200754/repository  
  -installIU org.slf4j.api,org.slf4j.jcl 
  -destination /home/mpellato/Downloads/eclipse
  -profile SDKProfile

What I got was:

$ ls -1 plugins/*slf*
plugins/ch.qos.logback.slf4j_0.9.19.v20100519-1910.jar
plugins/org.slf4j.api_1.5.11.v20100519-1910.jar
plugins/org.slf4j.jcl_1.5.11.v20100419-1106.jar

With this configuration, Mylyn now worked without any trouble -> test succes!

Thanks for rapidly fixing the issue!
Comment 24 Pascal Rapicault CLA 2010-05-21 10:17:04 EDT
*** Bug 313732 has been marked as a duplicate of this bug. ***
Comment 25 Patrick Flanagan CLA 2010-06-27 13:44:08 EDT
I see that this bug has been marked as fixed, but I'm getting it with the version of Helios that I downloaded this morning. I can't update or install plugins. Am I wrong to assume the fix would have been incorporated into recent builds? Thanks. JPF


eclipse.buildId=I20100608-0911
java.version=1.6.0_20
java.vendor=Apple Inc.
BootLoader constants: OS=macosx, ARCH=x86, WS=cocoa, NL=en_US
Framework arguments:  -keyring /Users/JPF/.eclipse_keyring -showlocation
Command-line arguments:  -os macosx -ws cocoa -arch x86 -keyring /Users/JPF/.eclipse_keyring -showlocation


Error
Sun Jun 27 12:22:27 PDT 2010
An internal error occurred during: "Contacting Software Sites".

java.lang.NullPointerException
at org.eclipse.equinox.internal.p2.repository.RepositoryTransport.download(RepositoryTransport.java:75)
at org.eclipse.equinox.internal.p2.repository.RepositoryTransport.download(RepositoryTransport.java:127)
at org.eclipse.equinox.internal.p2.repository.helpers.AbstractRepositoryManager.loadIndexFile(AbstractRepositoryManager.java:719)
at org.eclipse.equinox.internal.p2.repository.helpers.AbstractRepositoryManager.loadRepository(AbstractRepositoryManager.java:640)
at org.eclipse.equinox.internal.p2.metadata.repository.MetadataRepositoryManager.loadRepository(MetadataRepositoryManager.java:96)
at org.eclipse.equinox.internal.p2.metadata.repository.MetadataRepositoryManager.loadRepository(MetadataRepositoryManager.java:92)
at org.eclipse.equinox.p2.ui.LoadMetadataRepositoryJob.doLoad(LoadMetadataRepositoryJob.java:115)
at org.eclipse.equinox.p2.ui.LoadMetadataRepositoryJob.runModal(LoadMetadataRepositoryJob.java:100)
at org.eclipse.equinox.internal.p2.ui.sdk.PreloadingRepositoryHandler$2.runModal(PreloadingRepositoryHandler.java:82)
at org.eclipse.equinox.p2.operations.ProvisioningJob.run(ProvisioningJob.java:177)
at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
Comment 26 David Williams CLA 2010-06-27 17:49:48 EDT
(In reply to comment #25)
>  Am I wrong to assume the fix would have been incorporated into recent
> builds? 

Should have been. Though there are some errors known (e.g. bug 317392) so you might document exactly what you installed. 

But I am not sure how this bug about these loggers is related to RepositoryTransport NPE ... could be, may I just don't see it in my casual reading of previous comments ... but, thought I'd ask explicitly ... in case you commented in bug?
Comment 27 Patrick Flanagan CLA 2010-06-28 12:50:30 EDT
Thanks for the hint, Dave. I was able to fix the bug by moving log4j-1.2.15 (part of Axis2-1.5) off of my classpath. Cheers.
Comment 28 Bill Northcott CLA 2010-10-06 03:47:05 EDT
(In reply to comment #27)
> Thanks for the hint, Dave. I was able to fix the bug by moving log4j-1.2.15
> (part of Axis2-1.5) off of my classpath. Cheers.

I have the same problem as Patrick.

Fresh install of MacOS 10.6.4 no previous Eclipse installation, no plugins or additions.

Eclipse 3.6.1 20100909-0800

both 32 and 64 bit versions fail updating as described.

Went back to 64 bit version of 3.5.2 that works without a problem.
Comment 29 David Williams CLA 2010-10-06 03:54:55 EDT
(In reply to comment #28)
> (In reply to comment #27)
> > Thanks for the hint, Dave. I was able to fix the bug by moving log4j-1.2.15
> > (part of Axis2-1.5) off of my classpath. Cheers.
> 
> I have the same problem as Patrick.
> 
> Fresh install of MacOS 10.6.4 no previous Eclipse installation, no plugins or
> additions.
> 
> Eclipse 3.6.1 20100909-0800
> 
> both 32 and 64 bit versions fail updating as described.
> 
> Went back to 64 bit version of 3.5.2 that works without a problem.


From that I can tell, "Patrick's problem" is unrelated to this bugzilla entry. 

It is something else. More directly related to p2. I suggest opening (and searching) for a bug there. (Its in 'Runtime (RT), Equinox, p2). 
It sounds serious, so I'm sure they'd appreciate hearing about it. 
Let me know if I'm missing something about how it is related to this bugzilla entry.