Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce OSGi capability to wire o.e.swt to its native fragments #490

Open
HannesWell opened this issue Dec 7, 2022 · 6 comments
Open

Comments

@HannesWell
Copy link
Member

HannesWell commented Dec 7, 2022

Is your feature request related to a problem? Please describe.
At the moment the need of the org.eclipse.swt Plugin to have a platform-specific native fragment is only encoded on P2 level via p2.inf instructions or by corresponding features that contain o.e.swt and all native fragments with a corresponding platform filter.

The p2-instructions ensure that when building for example a Plug-in based product that only includes o.e.swt the native-swt fragment for the platform the product is build for is also included.
This approach has the disadvantage if one has a Eclipse/OSGi-application launch configuration based on plug-ins, where the automatic inclusion of dependencies is enabled and where only the o.e.swt Plugin is either specified or is part of the depedency closure, that then the launch fails quickly because the native fragment is not included.
The reason is that PDE computes the dependencies to include, based on the required OSGi wires of the Plug-ins/Bundles. And at the moment there is no wire from the o.e.swt bundle requiring a native fragment. Only the other way round.
To make the launch succeed one has to add the fragment for the current platform to the launch config manually, which is cumbersome.

Describe the solution you'd like

Therefore I want to suggest to introduce a OSGi capability for example named org.eclipse.swt.native.
The org.eclipse.swt Plugin then has to require that capability:

Require-Capability: org.eclipse.swt.native; filter:="(version=3.122.100)"

And each native swt fragment should provide that capbaility:

Provide-Capability: org.eclipse.swt.native; version:Version=3.122.100

Because the swt-native fragment has platform filters only one fragment resolves on a certain platform and therefore there is only one provider of that capability, which is then required by o.e.swt.

This works well in the IDE, because PDE does only consider Require-Bundle and Import-Package header for the graph from which the build order is computed and therefore there are no cycles.

So far so good, the only problem is that Tycho considers generic OSGi capabilities and therefore detects cycles in the project dependencies and consequently fails the build.
In fact this the presented idea is not new and exists for decades (see for example Bug 177851 or Bug 361901 ).
And the failure of Tycho is, at least in the second bug, the reason why this approach was discarded.

But nowadays we have more possibilities. This build-cycle problem could be solved if the native-fragment projects would be converted to Maven projects that have their Manifest generated (for example by the bnd-maven-plugin). The o.e.swt Plugin would then add the Maven artifacts as Maven dependencies to its pom.xml. All projects could still be build together in a mixed reactor (although at least in the verification builds for eclipse.platform.swt and eclipse.platform.swt.binaries are build in separated runs, but I assume that for the I-builds they are build together?). In the maven reactor the o.e.swt project would just depend on all its native fragments and because the then jar-typed natives projects are not 'OSGi-resolved' the back-link is not established in the Maven build graph.
With the latest M2E version those Maven projects can even participate in the regular PDE workspace build just like ordinary Eclipse Plugin projects, thanks to the new connectors for the bnd-maven-plugin/maven-bundle-plugin.

We already use that technique in M2E to build the maven-runtime bundles in one pass in a mixed reactor and all the mentioned things work well:
https://github.com/eclipse-m2e/m2e-core/blob/master/m2e-maven-runtime/pom.xml

This approach would allow to add the suggested required/provided capabilities and would consequently solve the launch problem in PDE. It would probably also allow to remove the p2-instructions. And it would also make it trivial to keep the Manifest-files in sync because e.g. the version of the host plugin can be set once in a variable and would then be reused and thus would ease the maintenance of the swt-native projects. IIRC there are some more SWT workarounds in the Eclipse-Platform they maybe can be removed as well.

The main blocker I see at the moment is that the Eclipse-Platform build is currently running with Tycho's classic resolver which does not support mixed reactors in Tycho 4.0 (but I hope @laeubi will make the new resolver work soon :) ). But there might be others (hopefully smaller ones) on the road.

Describe alternatives you've considered

Alternatively PDE could add support to read p2.inf instructions when computing dependencies during the launch of a Eclipse/OSGi application, but I think that would be more complicated.
PDE could also add a special SWT-case where it adds the native fragment for the current platform if o.e.swt is included, but that would just be one more workaround.
SWT could also be reworked to only have one 'multi-platform' jar (was already suggested in other places), but this would likely be a much greater effort.

@akurtakov, @mickaelistria, @laeubi what do you think? Do you see other blockers?

@akurtakov
Copy link
Member

@mickaelistria
Copy link
Contributor

I think this is a great idea and worth being implemented.
As for p2, since it has support for generic capabilities IIRC (or am I wrong?), it would indeed allow to get rid of p2.inf which would be a great win by the way.

@laeubi
Copy link
Contributor

laeubi commented Dec 7, 2022

One problem is that currently the p2 requirement is disabled for the swt build as one otherwise has a cyclic dependency (fragment requires host and host requires fragment), I think such capabilities can only work if:

  1. There is an extender pattern that is the provider to not requires the extender
  2. The build do not cares (e.g. parts is built as plain jar packaging)

About PDE: Pde does not required this but already has had "Extensible-API" header for this but for the whole purpose because platform misused fragments as "test-fragments", in OSGi a fragment do never need any capability as it is automatically attached. This is for the sole of P2 to workaround the hack with test-fragments so actually p2.inf is (for me) the correct solution and the rest is more a work around for other workarounds...

@merks
Copy link
Contributor

merks commented Dec 7, 2022

In the p2.inf the circularity is prevented by the org.eclipse.swt.buildtime filter:

requires.2.filter = (&(osgi.os=win32)(osgi.ws=win32)(osgi.arch=x86_64)(!(org.eclipse.swt.buildtime=true)))

Might the same mechanism to prevent circularity work for this new proposed approach?

@HannesWell
Copy link
Member Author

HannesWell commented Dec 7, 2022

Great, I'm glad you like the idea.

One problem is that currently the p2 requirement is disabled for the swt build as one otherwise has a cyclic dependency (fragment requires host and host requires fragment), I think such capabilities can only work if:

1. There is an extender pattern that is the provider to not requires the extender

2. The build do not cares (e.g. parts is built as plain jar packaging)

The second approach is what I'm currently working on, making the fragments 'plain-maven' jar-typed projects that have their MANIFEST.MF generated by BND and that are not 'OSGi-resolved' by Tycho. This should break the cycle because, when the fragments are build they are just plain jar-projects whose Manifest.MF is just a resource in the jar. Only the o.e.swt Plugin is build as Eclipse-Plugin typed projects afterwards and is 'OSGi-resolved' by Tycho. Because it will have a Maven dependency to all of its fragments its resolution should succeed since Tycho supports mixed reactors/resolution since 2.7.X (IIRC).

The org.eclipse.swt.buildtime should then be obsolete.

About PDE: Pde does not required this but already has had "Extensible-API" header for this but for the whole purpose because platform misused fragments as "test-fragments", in OSGi a fragment do never need any capability as it is automatically attached. This is for the sole of P2 to workaround the hack with test-fragments so actually p2.inf is (for me) the correct solution and the rest is more a work around for other workarounds...

The Eclipse-ExtensibleAPI header is not for test-fragments. It is for fragments that contribute new classes to their host that and which are referenced by other Bundles that only reference/require the host. That header tells PDE to also put all fragments of a host on the classpath of a Plugin that Required (not sure about Imports) the host.

The main problem I have with p2-instructions is that it is only poorly supported in terms of tooling and the documentation is also not very verbose. And IIRC there is some not in the doc that it is all provisional.

I started to work on this and it looks promising. It has also the advantage that many similar files can be removed. I will hopefully be able to created the first draft PRs in the next days.

@merks
Copy link
Contributor

merks commented Dec 28, 2022

Note that SWT currently is published like this:

https://repo1.maven.org/maven2/org/eclipse/platform/org.eclipse.swt/3.122.0/org.eclipse.swt-3.122.0.pom

I.e., with this dependency:

<dependencies>
  <dependency>
    <groupId>org.eclipse.platform</groupId>
    <artifactId>org.eclipse.swt.${osgi.platform}</artifactId>
    <version>3.122.0</version>
    <optional>false</optional>
  </dependency>
</dependencies>

So downstream consumers can generally depend directly on org.eclipse.swt.

The /publish-to-maven-central/SDK4Mvn.aggran provides a preview of how the latest 4.27 I-build would be published to Maven:

image

This is dependency is derived from the requirements represented in p2:

image

I suspect that if this is changed as suggested here, that maybe special handling will be needed to ensure that it maps sensibly; but then again, maybe it will just work. In any case, it's something on which we should key an eye...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants