| Summary: | BundleRequirement.getAttributes for osgi.wiring.* namespaces must be empty | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Thomas Watson <tjwatson> | ||||||
| Component: | Framework | Assignee: | Thomas Watson <tjwatson> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | hargrave, jwross | ||||||
| Version: | 3.7 | Flags: | hargrave:
review+
jwross: review+ dj.houghton: review+ |
||||||
| Target Milestone: | 3.7 RC3 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Thomas Watson
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. +1 for RC3. We need to make sure people do not become dependent on information that should not be there. 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.
BJ, please review. John, please review. Links to the relevant CPEG defects: https://www.osgi.org/members/bugzilla/show_bug.cgi?id=1922 https://www.osgi.org/members/bugzilla/show_bug.cgi?id=1870 Looks okay to me. 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.
+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. 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. |