Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 346979

Summary: BundleRequirement.getAttributes for osgi.wiring.* namespaces must be empty
Product: [Eclipse Project] Equinox Reporter: Thomas Watson <tjwatson>
Component: FrameworkAssignee: Thomas Watson <tjwatson>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: hargrave, jwross
Version: 3.7Flags: hargrave: review+
jwross: review+
dj.houghton: review+
Target Milestone: 3.7 RC3   
Hardware: PC   
OS: Mac OS X - Carbon (unsup.)   
Whiteboard:
Attachments:
Description Flags
patch
none
updated patch none

Description Thomas Watson CLA 2011-05-24 09:06:00 EDT
Some late clarifications to the R4.3 specification will require changes to the Equinox Framework implementation for R4.3.

It has been clarified that requirement attributes must not contain entries that have semantic meaning at runtime.  For the osgi.wiring.* namespaces, the requirements specified in the bundle manifest header (e.g. for Import-Package, Require-Bundle, Fragment-Host) must have empty attribute Maps.  Instead the matching attributes specified on the bundle manifest headers must be encoded into a matching filter directive for the BundleRequirement.

This requires two changes to the Equinox framework

1) Create a valid filter string from the matching attributes specified on the bundle manifest headers for Import-Package, Require-Bundle and Fragment-Host

2) Return empty Maps from the method BundleRequirement.getAttributes().
Comment 1 Thomas Watson CLA 2011-05-24 09:16:37 EDT
Going to look into this for RC3.  I think it would be good to make sure the attributes map for osgi.wiring.* name spaces is an empty map so that clients do not become dependent on the content.

I believe the fix should be relatively simple for this one.  The most complicated part is going to be make sure we create a valid filter string from the manifest header attributes for the osgi.wiring.* name spaces.
Comment 2 BJ Hargrave CLA 2011-05-24 10:22:43 EDT
+1 for RC3. We need to make sure people do not become dependent on information that should not be there.
Comment 3 Thomas Watson CLA 2011-05-24 11:42:20 EDT
Created attachment 196453 [details]
patch

Here is the fix.  I modified the osgi CT to test.  Could use some additional testing in Equinox tests, but for the time being we need to focus on updating the OSGi CT.
Comment 4 Thomas Watson CLA 2011-05-24 11:42:41 EDT
BJ, please review.
Comment 5 Thomas Watson CLA 2011-05-24 11:43:06 EDT
John, please review.
Comment 7 John Ross CLA 2011-05-24 14:44:08 EDT
Looks okay to me.
Comment 8 Thomas Watson CLA 2011-05-24 16:34:52 EDT
Created attachment 196486 [details]
updated patch

After a review with BJ this patch contains the following fixes

1) add @Override to a few methods where appropriate
2) when creating a filter for VersionRange leave the check for maximum off if the range is open (i.e. Import-Package: foo; version=2.0).

There was one additional comment by BJ which I did not address.  The generation of the filter directive can end up with a filter string with a single operand for &.  For example:  "Import-Package: foo" ends up with the filter (&(osgi.wiring.package=foo)).  The extra (&) is not necessary and could be removed.  To fix this reasonably we would need to introduce a FilterBuilder of some sorts I think.  Otherwise it would involve a bunch of hacks to keep track of the number of filter components we added to the StringBuffer.
Comment 9 BJ Hargrave CLA 2011-05-24 17:21:36 EDT
+1 on the change. Tom will add a reminder to go back after the release and replace the max version logic with something in VersionRange to avoid the impl knowledge of VersionRange in this code. It is too late in this release to avoid.
Comment 10 Thomas Watson CLA 2011-05-24 17:26:27 EDT
Thanks all for the review.  Patch has been released with an added TODO.  I also changed the open range check to be a bit more readable.