Community
Participate
Working Groups
We should define osgi capabilities for the IEventAdmin service and declare a dependency to it in our platform code. Also I think EventAdmin in Equinox should declare capabilities and the IEventAdmin implementation should require it. This way PDE could find such missing dependencies, once Bug 528394 is fixed.
Thomas (Watson), are you aware of a naming convension for declaring capabilities? Suggestions are welcome.
The Equinox EventAdmin bundle provides the capability since Oxygen. And we added the necessary require capability also in Oxygen. So what do you actually request? I am not aware of any missing pieces for the EventAdmin. Regarding the naming convention. It depends on the capability you want to provide. For a service typically the name of the service interface. For default osgi services this is specified in the OSGi specification.
Maybe we looked at old data for EventAdmin, if I look now I also see the Capability. But org.eclipse.e4.ui.services should also also set for a Capability manifst entry for IEventBroker so that the platform which requires IEventBroker can define a dependency. Currently we have no way to express that we need an instance of IEventBroker to start the platform.
Does anyone has objections for adding capabilities to the implementation plug-in of IEventBroker?
Provide or require? Provide is not critical and sure a good thing. Require Fotos EventAdmin should be there already. And also makes sense as without it the EventBroker does not work.
(In reply to Dirk Fauth from comment #5) > Provide or require? Provide is not critical and sure a good thing. Require > Fotos EventAdmin should be there already. And also makes sense as without it > the EventBroker does not work. The discussion is about IEventBroker not EventAdmin. IMHO we should add provide and require for IEventBroker.
In Comment #4 you asked about adding capabilities to the implementation plug-in of IEventBroker. And there you can not add the Require-Capability for itself. ;) I just checked the implementation and noticed that the EventBroker is actually not an OSGi service. So it would be wrong to provide a capability for "osgi.service". But that does not mean we could not add it. But let's check your requirement first. If I understand your requirement correctly, you want to be able to define a dependency inside some platform plug-ins to the org.eclipse.e4.ui.services bundle. And you say that it is currently not possible to specify a dependency to org.eclipse.e4.ui.services because the platform plugins do not import packages from that plugin. But I can see that org.eclipse.e4.ui.workbench does require org.eclipse.e4.ui.services. Although IMHO the Require-Bundle should be changed to an Import-Package, the dependency to the e4.ui.services plugin is there. So PDE should already find it and add that bundle to the required bundles on resolve. If that is your requirement, I would not see a reason for adding the capability stuff here. In Comment #3 you say that there is no way to specify that an instance of IEventBroker is available. That is something else than specifying the bundle dependency. Note that simply said capabilities are just another way to specify a bundle dependency. It does actually not start the bundle. So if that is your requirement that an IEventBroker instance is created, we need some other solution. Actually the instance is created by the EventBrokerFactory which is a ContextFunction. Here we could have an issue in the startup order I guess. If a platform bundle does not specify a requirement on the e4.ui.services bundle before but requests an IEventBroker, the factory could probably not yet be loaded and therefore no IEventBroker is available. But to be honest, I am not sure if that issue could be solved by introducing a capability. You could try this by introducing the capability to the EventBrokerFactory as osgi.service. But from my understanding it would only help to resolve the required dependency, not to start in the required order. I haven't checked the ContextFunction stuff in detail yet, as I am not sure what issue you are trying to solve. But if it is the second issue I described, maybe we need to check how ContextFunctions are registered and consumed then.
The requirement is actually already before the runtime part. It is about detection of the bundles that need to be part of a launch config in order to make it runnable. With bug#522332 we are enhancing PDE's DependencyManager to compute all bundles that are required for a set of bundles that are selected. Let's say you have a simple project dependending on org.eclipse.ui. This won't be runnable without having an org.eclipse.equinox.event in the available bundles. The enhancement we implemented takes now also generic capabilities into account. This works for example for osgi.extender, because some UI plugins require that capability and org.apache.felix.scr is providing it. That dependency detection does not work for equinox.event, and launches would fail due to a missing IEventBroker. So equinox.event has to provide a capability which bundles using the IEventBroker are requiring, if that is not managed by other kind of dependencies. I that specific example bundle org.eclipse.ui.workbench is requiring an IEventBroker, so it has to declare some Require-Capability for that, and another bundle has to provide it. In the end, the BundleValidationOperation has to detect that org.eclipse.ui is requiring something that could be fulfilled when org.eclipse.equinox.event is present.
So your requirement is about bundle resolving. You are again mixing OSGi EventAdmin and E4 EventBroker. Equinox Event is already providing the OSGi capability aswell as e4.ui.services is requiring that capability. So that should already work. And it should even already work with Oxygen and p2 because I added the p2 capabilities aswell. That said, for OSGi EventAdmin in org.eclipse.equinox.event there shouldn't be any actions needed. Taking your example with org.eclipse.ui, there should be also no action necessary, because there is already a dependency defined via Require-Bundle or Import-Package. PDE should therefore already see that e4.ui.services is required. And if you look closely, org.eclipse.ui.workbench requires org.eclipse.e4.ui.services as it uses some interfaces from that bundle. The bundle dependency is already there. Therefore again, where is the issue? PDE should already find that dependency with the current configuration. If it doesn't, I suppose there is another issue in the PDE mechanism for resolving bundle dependencies. At least from what I see in the bundles. In theory I don't see an issue with adding a Provide-Capability. I am just not sure if you are clear about the issue you are currently facing and if adding the capability would solve it. But as long as nobody requires it, it is just an additional information in the MANIFEST header that does not hurt. If you want to add the Provide-Capability we first need to find some namespace for this. osgi.service would be incorrect as the EventBroker is not an OSGi service, it is something that is provided by a ContextFunction to make it available for E4 injection. Maybe something like "e4.context.object" or "e4.context.object.provider" and the value should be the full qualified name of the interface or object that is added to the context. Then it can be used in different places where the platform provides objects to the IEclipseContext or even from developers of Eclipse applications.
I have now looked again why org.eclipse.equinox.event is not selected and need some help to understand this. Bundle org.eclipse.equinox.event declares Provide-Capability: osgi.service; objectClass:List<String>="org.osgi.service.event.EventAdmin"; uses:="org.osgi.service.event" Bundle org.eclipse.e4.ui.services declares Require-Capability: [...] osgi.service; filter:="(objectClass=org.osgi.service.event.EventAdmin)"; effective:="active" In org.eclipse.osgi.internal.module.ResolverImpl.resolveBundle() there is this check: if (genericRequires[i].isEffective()) { ... } But isEffective() yields false, because in class GenericConstraint it is true only when the directive was not set, or it has value 'resolve'. Why is value 'active' excluded here? Why it is set, but the isEffective() value computed to false? If I change effective:="active" to effective:="resolve" the automatic selection of org.eclipse.equinox.event works. I guess the value 'active' is set with intention, but is the isEffective() computation correct?
The OSGi Core specification says the following about that: "effective - (resolve) Specifies the time a capability is available, either resolve (default) or another name. The OSGi framework resolver only considers Capabilities without an effective directive or effective:=resolve. Capabilities with other values for the effective directive can be considered by an external agent." OSGi Core 6 - Module Layer Version 1.8 - page 42 So the resolver works as specified in the spec. I had a short discussion with Thomas Watson on this in Bug 416047. IIRC the main thing is that for resolving the bundles EventAdmin is not needed. It is needed to be active in the runtime. But effective:=active is not taken into account as there is no "external agent" that checks for this. To be honest, I am not sure about a correct solution. I guess bndtools is checking the capabilities without the effective value to be able to provide the developer the option to select the bundles on "Resolve". But I am only a bndtools user and never looked in the implementation there. Maybe someone from that side could give some more insights.
I recognized that the resolver is running in development mode, i.e. ResolverImpl#developmentMode is true in PDE. Is it valid to leverage that property? More specifically: if (genericRequires[i].isEffective() || developmentMode) { ... }
New Gerrit change created: https://git.eclipse.org/r/116097
The proposed change will also consider non-effective generic constraints when running in development mode. This would solve my use case, i.e. org.eclipse.equinox.event will be selected when its generic capabilty is required. Is this valid?
With that patch this ticket needs to be assigned to Equinox and not Platform UI. Maybe we then get responses from people with deeper knowledge in the OSGi implementation of equinox.
(In reply to Dirk Fauth from comment #15) > With that patch this ticket needs to be assigned to Equinox and not Platform > UI. Maybe we then get responses from people with deeper knowledge in the > OSGi implementation of equinox. Tom (Watson) is cc'ed in the bug.
(In reply to Lars Vogel from comment #16) > (In reply to Dirk Fauth from comment #15) > > With that patch this ticket needs to be assigned to Equinox and not Platform > > UI. Maybe we then get responses from people with deeper knowledge in the > > OSGi implementation of equinox. > > Tom (Watson) is cc'ed in the bug. See gerrit for my comments on the change.
I have updated the patch according to Thomas' review comments. The development mode is now considered while constructing GenericCapabilty / GenericConstraint instances. By doing this, PDE will be able to resolve generic capabilities which declare effective:="active".
Gerrit change https://git.eclipse.org/r/116097 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=58ead3e6dcd337c9596894b1a8b779c26e759844
Thanks for the detailed reviews :) This will be helpful for PDE.
