eclipse-pde / eclipse.pde

Eclipse Public License 2.0
25 stars 64 forks source link

Consider EE requirements in EE-Section of ManifestEditor #1318

Open HannesWell opened 3 months ago

HannesWell commented 3 months ago

The Execution Environments section in the Manifest Editor is currently empty if a bundle only specifies an EE requirement instead of a dedicated Bundle-RequiredExecutionEnvironment header. For example for third-party OSGi bundles published to Maven-Central this is quite common.

With this change EE requirements are now considered as well: grafik

The main building block for that is the addition of new method ManifestUtils.getRequiredExecutionEnvironments() in the first commit, that derives the required EEs of an OSGi resource based on its EE requirements. For that it only uses the OSGi resources API and is thus one small step to migrate PDE off the Equinox resolver API.

github-actions[bot] commented 3 months ago

Test Results

   291 files  ±0     291 suites  ±0   1h 1m 16s :stopwatch: + 6m 38s  3 580 tests ±0   3 504 :white_check_mark: ±0   76 :zzz: ±0  0 :x: ±0  11 037 runs  ±0  10 806 :white_check_mark: ±0  231 :zzz: ±0  0 :x: ±0 

Results for commit 71517165. ± Comparison against base commit 029c7d79.

:recycle: This comment has been updated with latest results.

HannesWell commented 3 months ago

Previously the ExecutionEnvironmentSection just operated on the text in the Manifest. With the current state it requires a resolved target-platform that contains a BundleDescription/osgi.Resource for the Manifest to open. While the latter is present anyways in probably most of the cases I'm not confident it is always the case. Therefore I think we should continue to operate just on the textual model of the Manifest opened in the editor. A lot of the new logic introduced in ManifestUtils can still be used for that and the changes in PluginInputContextManager would be avoided.

laeubi commented 2 weeks ago

Previously the ExecutionEnvironmentSection just operated on the text in the Manifest. With the current state it requires a resolved target-platform that contains a BundleDescription/osgi.Resource for the Manifest to open

I just wanted to note one do not need a resolved state for this.

Assume you have the String and parse it into a Manifest you can then use (full namespaces to prevent confusion with possible existing classes):

java.util.jar.Manifest manifest = ...
aQute.bnd.osgi.ResourceBuilder resourceBuilder = new aQute.bnd.osgi.ResourceBuilder();
resourceBuilder.addManifest(manifest);
org.osgi.resource.Resource resource = resourceBuilder.build();

to get an OSGi Resource from it, something similar can be archived with P2 in BundlesAction.createBundleDescription(Dictionary<String, String>, File) but requires the legacy fragment.

One can possibly hide this of course in a Util method as well, e.g. ManifestUtils#asResource(String)