Bugzilla – Full Text Bug Listing
| Summary: | Support for arbitrary matching attributes on Require-Bundle/Fragment-Host causes bundle to not resolve | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Olivier Thomann <Olivier_Thomann> | ||||
| Component: | Framework | Assignee: | equinox.framework-inbox <equinox.framework-inbox> | ||||
| Status: | RESOLVED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | aniefer, david_williams, hargrave, jeffmcaffer, kim.moir, markus.tiede, pwebster, remy.suen, tjwatson | ||||
| Version: | 3.7 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Olivier Thomann
It also looks like some orbit bundles don't have a consistent range that let them resolve. org.apache.batik.css (1.6.0.v200912221622) refers to the version range: Require-Bundle: org.apache.batik.util;version="[1.6.0,1.7.0)",org.w3c. css.sac;version="[1.3.0,1.4.0)",org.w3c.dom.svg;version="[1.1.0,1.3.0 )" But the util bundle comes with version: org.apache.batik.util (1.7.0.v200903091627) So it should not resolve. I guess it used to because version was ignored. I think we just need to fix the broken manifests. It is clear the intention of the manifest is to match versions on the required bundles. However, the wrong attribute name was specified, so it was ignored. The change/clarification to the OSGi spec and thus Equinox will now root out these incorrect manifests which can then be fixed. I am not sure what change to the framework this bug is requesting? The changes that need to be made are to the bundles with incorrect manifests. I think it would be good to have a temporary mode that can log which bundles are using the wrong attributes names and check the corresponding version range to see if they match. Like that it would be easier to fix all orbit bundles that have wrong manifest attribute names and wrong version ranges. (In reply to comment #3) > I think it would be good to have a temporary mode that can log which bundles > are using the wrong attributes names and check the corresponding version range > to see if they match. > Like that it would be easier to fix all orbit bundles that have wrong manifest > attribute names and wrong version ranges. So some temporary code which would be remove before we finalize 3.7? Seems unusual. Perhaps a better solution is to use a tool (e.g. bnd) to analyze the manifests of the orbit bundles for problems rather than stumbling over them one at a time in the runtime. DJ opened bug329376 to fix up the orbit bundles. One possibility is to have the framework alias the version attribute to bundle-version for Require-Bundle and Fragment-Host. The misuse of the "version" attribute is going to be the most common mistake so we could try to be nice and alias that to bundle-version. But even that is problematic because it sounds like some bad bundles don't even specify the correct ranges in their incorrectly specified "version" attributes. So if we aliased them to bundle-version they would still not be able to resolve because their range is bad anyway. Are we any better off? The only option I see is to have a flag that disables arbitrary matching attributes for Require-Bundle and Fragment-Host. It would be enabled by default. PDE would still complain that the bundles cannot resolve in your workspace, but at least you can configure the runtime to allow the bad bundles to resolve. Keep in mind that the bad bundles will be allowed to resolve to bundle-versions that the developer did not intend. So either way we must fix all bad bundles in the end. Created attachment 182320 [details]
new option to disable bundle attribute matching
Adds a new resolver option that can be used to disable bundle attribute matching. Olivier can you try this patch out?
In your launch config set the following system property:
-Dosgi.resolve.bundle.attributes=false
The default is true so you must specify "false" any other value is treated as "true".
How does this bug block bug329382? bug329382 needs to be fixed regardless of what happens to this bug. (In reply to comment #6) > Created an attachment (id=182320) [details] > new option to disable bundle attribute matching > > Adds a new resolver option that can be used to disable bundle attribute > matching. Olivier can you try this patch out? > > In your launch config set the following system property: > > -Dosgi.resolve.bundle.attributes=false > > The default is true so you must specify "false" any other value is treated as > "true". These sort of options are a bad idea. We don't even have a handle on the scope of the situation and we are proposing options that will violate the spec! It would seem that if there is such an option as this that the value of the option should be the BSNs of the bundles which are broken and should have their attributes ignored. This option, as currently proposed, will break correct bundles which depend upon matching attributes working! (In reply to comment #8) > This option, as currently proposed, will break correct bundles which depend > upon matching attributes working! Only if you set the option to false! The default behavior is to correctly pay attention to declared matching attributes. (In reply to comment #9) > (In reply to comment #8) > > This option, as currently proposed, will break correct bundles which depend > > upon matching attributes working! > > Only if you set the option to false! The default behavior is to correctly pay > attention to declared matching attributes. Well of course. But as soon as you set it to false, any bundle which relies upon specified behavior will be broken. Seems a bad idea to have an option that helps broken bundles by breaking correct bundles. Your option will need to handle a mixed environment of broken bundles and bundles using the matching attributes. (In reply to comment #6) > Created an attachment (id=182320) [details] > new option to disable bundle attribute matching > > Adds a new resolver option that can be used to disable bundle attribute > matching. Olivier can you try this patch out? I can confirm that I can now run tests again with org.eclipse.osgi from HEAD after applying said patch and appending said argument to my VM arguments in my launch configuration. This does not block bug 329382. The manifests need to be fixed. (In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > This option, as currently proposed, will break correct bundles which depend > > > upon matching attributes working! > > > > Only if you set the option to false! The default behavior is to correctly pay > > attention to declared matching attributes. > > Well of course. But as soon as you set it to false, any bundle which relies > upon specified behavior will be broken. Seems a bad idea to have an option that > helps broken bundles by breaking correct bundles. Your option will need to > handle a mixed environment of broken bundles and bundles using the matching > attributes. Sigh, if that is required I would rather do nothing. The problem I have with complex configuration options is that we now need to manage the property each time a bad bundle is installed. It is not that I disagree with your assessment. It is not pragmatically anything that I think our adopters will ever be able to manage within their products. If it is not easily managed and there is no easy way out then adoption of 3.7 could be lessened. The number of bugs we have opened just for eclipse community bundles has me worried that products built on top of eclipse will be effected. see: bug329376 bug329361 bug329392 bug329382 My 2c. We should NOT make any special allowances for malformed headers. My main assumption is that most people are using the tooling and the tooling has been doing the right thing. Those who have not will need to do more work. Doing special stuff in the framework just adds to bloat, increases the testing load and leads to confusion in what is right. While I agree that some folks moving to 3.7 will be affected, I'm not that concerned about adoption per se. The fix is very easy for people and if we are really slick we could see about PDE providing quick fixes and the like in PDE for common cases (the things we would have aliased). At the very least we should defer any work here until it is seen to be absolutely necessary (e.g., hundreds of important existing bundles failing to run on 3.7). If you want to maintain strict checking (I'm all for adhering to the spec) can the error message be improved? bug 329376 comment #6 Right now when you get into this state, a bundle that loads fine in 3.5 and 3.6 will simply not work in 3.7, and possibly not even generate an error log. PW I think the only thing we need to do is to find all bundles with malformed headers and fix them. We should not accept malformed headers, but we need to make sure we have time to fix the malformed headers in bundles. (In reply to comment #14) > If you want to maintain strict checking (I'm all for adhering to the spec) can > the error message be improved? bug 329376 comment #6 > > Right now when you get into this state, a bundle that loads fine in 3.5 and 3.6 > will simply not work in 3.7, and possibly not even generate an error log. > > PW We could decide to log every time we find unresolved bundles. This will cause log entries even in cases that are valid. For example, there are a few bundles in the SDK that require Java SE 6. When on Java SE 5 you will not get unresolved errors in your log. This has caused issues in the past. I quickly checked the 4.1 bundles with a small tool I wrote. I found that: org.apache.batik.css has wrong attribute names for required bundles: org.apache.batik.util, org.w3c.css.sac and org.w3c.dom.svg. org.apache.batik.util has wrong attribute names for required bundle: org.apache.batik.util.gui and org.w3c.dom.svg has wrong attribute names for required bundle: org.w3c.dom.smil and only the range for required bundle org.apache.batik.util in org.apache.batik.css is wrong according to the provided bundles. So we don't need to get a lot of orbit bundles fixed to be able to run 4.1 builds. Hopefully I didn't miss any, but I cannot be 100% sure. For what its worth, a "quick test" check was made on all bundles in Orbit and Indigo M2 repo, and the broken bundles fixed (most related to the "batik family". See bug 329376. But still, I as worried that there was no "escape" flag to restore old, broken behavior, so there would not be true "binary compatibility" if some "old product" tried to move onto new platform. So, I checked both the Helios repo's and several versions of some large several-thousand-bundle adopter products and there were no new findings, with one exception. Back in the Galileo stream (I think) these two jars also had invalid manifest: org.eclipse.ocl_1.2.0.v200806091438.jar org.eclipse.ocl_1.2.3.v200901071407.jar but, seems long ago fixed in helios stream (if not also Ganymede? didn't check there) org.eclipse.ocl_3.0.1.R30x_v201008251030.jar So ... I'm less worried. I do not think it is a wide spread problem. My little investigation is no guarantee, of course, but I do not think "we" (i.e. you) need to do anything preemptive to allow old broken behavior to be restored, and can safely wait to see if anyone runs into problems. Seems unlikely. Great. Thanks David for doing that investigation. Thanks David. Renaming the bug to reflect the issue. The patch attached may be used to disable this new feature. I agree we should avoid adding such an option until it is proven to be an real issue. My main concern is any products that pulled in the batik bundles from Helios will be broken if they upgrade the core framework to Indigo without picking up the fixed batik bundles from Indigo. Is that not realistic? Would they always get the fixed batik bundles from Indigo? We will not add this option. If this proves to be an issue in practice then we can reopen the bug to reconsider adding the option. *** Bug 343843 has been marked as a duplicate of this bug. *** |