Extend StateBuilder to handle EE from Require-Capability header #645
Conversation
| return ee + '-' + version; | ||
| } | ||
| } catch (InvalidSyntaxException e) { | ||
| // TODO Special handling needed? |
There was a problem hiding this comment.
The filter here is checked in org.eclipse.osgi.internal.resolver.StateBuilder.createOSGiRequires(ManifestElement[], List<GenericSpecification>) when we create the GenericSpecification objects. So I think we can ignore the syntax error here.
| @@ -128,7 +131,7 @@ static BundleDescription createBundleDescription(StateImpl state, Dictionary<Str | |||
| } | |||
| result.setLocation(location); | |||
| result.setPlatformFilter(manifest.get(StateImpl.ECLIPSE_PLATFORMFILTER)); | |||
| String[] brees = ManifestElement.getArrayFromList(manifest.get(Constants.BUNDLE_REQUIREDEXECUTIONENVIRONMENT)); | |||
| String[] brees = getDeclaredRequiredEE(manifest); | |||
There was a problem hiding this comment.
Adding to the local brees object here will end up creating duplicate requires below when the brees is passed to createGenericRequires. Somehow we need to split this up such that the originally specified brees used for the call to createGenericRequires.
There was a problem hiding this comment.
If the BREEs are converted to generic capabilities, then it probably makes more sense go in the other direction:
To extract the EE once the generic capabilities have been created. Then we get the elements from both the Bundle-RequiredExecutionEnvironment and Require-Capability header and don't risk duplicate entries.
|
It would be good if the code to extract the BREEs would be made available from a place independent from |
PDE could do the conversion to BREE and add the header (if missing) to the Map used to create the |
So how should we proceed with this item? Fixing this in either Equinox or PDE should take roughly the same amount of effort. However, I believe that this should be decided before investing more time into this issue. I don't have a deep understanding of either projects, so I would leave it up to you (or any other committer). |
Seems more simple to fix in Equinox (and correct?). But Hannes has a better understanding of PDE usage of the resolver. I'm fine either way. |
ptziegler
force-pushed
the
issue643
branch
2 times, most recently
from
dad90af
to
8b8c2eb
Compare
With this change, the BundleDescription instance that is returned by the StateBuilder now also contains the execution environments that are required by the Require-Capability header, rather than only the Bundle-RequireExecutionEnvironment header. Previously, a call to getExecutionEnvironments() would always return an empty array, if a bundle lacks the Bundle-RequireExecutionEnvironment header, even though a minimum version is required as a capability. Resolves eclipse-equinox#643
For now I think it is better added to PDE since it allows us to reuse it in multiple places in PDE while keeping it internal and consequently kind of incubate, fix, etc. it there. If more users show up we can still move this at a more general place. Sorry for the delayed reply but this is a busy week. I intend to look into your issue in more detail to give a more detailed advice at the weekend. |
|
Since eclipse-pde/eclipse.pde#1302 is submitted I think this is obsolete. |
With this change, the BundleDescription instance that is returned by the StateBuilder now also contains the execution environments that are required by the Require-Capability header, rather than only the Bundle-RequireExecutionEnvironment header.
Previously, a call to getExecutionEnvironments() would always return an empty array, if a bundle lacks the Bundle-RequireExecutionEnvironment header, even though a minimum version is required as a capability.
Resolves #643