Include required plugins automatically seem not consider already added capabilties

[laeubi]

If I use the "Include required plugins automatically" option I noticed that it does seem to add more than required in some situations.

For example assume a product hat has slf4j.api and two slf4j provider implementations are in the target then PDE adds another slf4j provider even if one was already chosen in the product, I noticed that when working on

• Create an SLF4j provider implementation for the Equinox Log eclipse-equinox/equinox#740

with the product here:

<?xml version="1.0" encoding="UTF-8"?>
<?pde version="3.5"?>

<product uid="testslf4j" version="1.0.0.qualifier" type="bundles" includeLaunchers="true" autoIncludeRequirements="false">

   <configIni use="default">
   </configIni>

   <launcherArgs>
      <programArgs>-console
      </programArgs>
      <vmArgs>-Declipse.ignoreApp=true
-Dosgi.noShutdown=true
      </vmArgs>
      <vmArgsMac>-XstartOnFirstThread -Dorg.eclipse.swt.internal.carbon.smallFonts
      </vmArgsMac>
   </launcherArgs>

   <launcher>
      <win useIco="false">
         <bmp/>
      </win>
   </launcher>

   <vm>
   </vm>

   <plugins>
      <plugin id="my.logging.bundle"/>
      <plugin id="org.apache.aries.spifly.dynamic.bundle"/>
      <plugin id="org.apache.felix.gogo.command"/>
      <plugin id="org.apache.felix.gogo.runtime"/>
      <plugin id="org.apache.felix.gogo.shell"/>
      <plugin id="org.apache.felix.scr"/>
      <plugin id="org.eclipse.equinox.common"/>
      <plugin id="org.eclipse.equinox.console"/>
      <plugin id="org.eclipse.osgi"/>
      <plugin id="org.objectweb.asm"/>
      <plugin id="org.objectweb.asm.commons"/>
      <plugin id="org.objectweb.asm.tree"/>
      <plugin id="org.objectweb.asm.tree.analysis"/>
      <plugin id="org.objectweb.asm.util"/>
      <plugin id="org.osgi.service.component"/>
      <plugin id="org.osgi.util.function"/>
      <plugin id="org.osgi.util.promise"/>
      <plugin id="slf4j.api"/>
   </plugins>

   <configurations>
      <plugin id="org.apache.aries.spifly.dynamic.bundle" autoStart="true" startLevel="1" />
      <plugin id="org.apache.felix.scr" autoStart="true" startLevel="2" />
      <plugin id="org.eclipse.core.runtime" autoStart="true" startLevel="0" />
      <plugin id="org.eclipse.equinox.common" autoStart="true" startLevel="2" />
      <plugin id="org.eclipse.equinox.event" autoStart="true" startLevel="2" />
      <plugin id="org.eclipse.equinox.simpleconfigurator" autoStart="true" startLevel="1" />
   </configurations>

</product>

The product itself is "complete" so works with autoIncludeRequirements="false" already, no missing requirements are reported when validated. If one starts the product it is getting the expected message:

SLF4J(W): No SLF4J providers were found.
SLF4J(W): Defaulting to no-operation (NOP) logger implementation
SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details.

Now I add <plugin id="org.eclipse.equinox.slf4j"/> and logging works and I get:

id	State       Bundle
7	ACTIVE      org.eclipse.equinox.slf4j_1.0.0.qualifier
17	RESOLVED    slf4j.api_2.0.16

if now one activates autoIncludeRequirements="true" one suddenly gets

id	State       Bundle
6	RESOLVED    slf4j.simple_2.0.16
8	ACTIVE      org.eclipse.equinox.slf4j_1.0.0.qualifier
18	RESOLVED    slf4j.api_2.0.16

@HannesWell can you take a look as this feature is extremely useful but might confusing in some situations like this.

[HannesWell]

This is probably because DependencyManager considers all kind of requirements:

eclipse.pde/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java

Line 199 in 975f72b

List<BundleWire> requiredWires = wiring.getRequiredWires(null);

And IIRC the slf4j.api bundle has a (OSGi) requirement to all it's providers through the Service-Loader mediator configuration.

In this specific case the life-line that prevents a real problem is that the Aries SPI-fly service-loader mediator only considers bundles in state active, which isn't usually the case (like in your example) as long as one has not configured it as auto-started in the product configuration (at least in my use-cases this was the case).

But of course in general it would be cleaner to not have the other providers in the product at all. I wanted to add meant to filter the considered required-wires so that the DependencyManager can be used in more places in PDE.
Then the service-loader requirements could be filtered too.

[laeubi]

And IIRC the slf4j.api bundle has a (OSGi) requirement to all it's providers through the Service-Loader mediator configuration.

Should it not only require one?

[laeubi]

@HannesWell I have now further checked this and the spec mentions

cardinality
String ['multiple' | 'single']
Allow the requirement to be satisfied by just one capability or provide wires to any capability that satisfies the requirement.

where multiple seems the default, I think slf4j should use single here to not pull in multiple providers (what is not supported). As you have made changes to slf4j in the past would you mind proposing such thing so that hopefully with a new release of slf4j the issue is resolved?

[laeubi]

According to the javadoc single is the default...

The requirement directive used to specify the cardinality for a requirement. The default value is single

So something seems to be odd here...

[laeubi]

I was now able to debug it and it is as suspected in the initial description, PDE does only consider requirements on the bundle level (based on the wiring in the target state), in this case the requirement is fulfilled by two bundles:

slf4j-simple (that is wired in the target state) and slf4j-equinox and thus it can't detect that as already being present.

[HannesWell]

I was now able to debug it and it is as suspected in the initial description, PDE does only consider requirements on the bundle level (based on the wiring in the target state), in this case the requirement is fulfilled by two bundles:

slf4j-simple (that is wired in the target state) and slf4j-equinox and thus it can't detect that as already being present.

Yes, after checking the required and provided capabilities in the MANIFEST.MF I also think that's what happens (sorry for not writing it earlier).
For clarification: slf4j.simple is wired to the slf4j.api bundle to satisfy it's service-loader requirement in the Target-Platform and therefore added to the launch. And the equinox.slf4j bundle is obviously added because it's listed in the product configuration.
So we end up with two providers in the runtime, while one would be sufficient.
But of course the TP doesn't know what the product needs.

@HannesWell I have now further checked this and the spec mentions

cardinality
String ['multiple' | 'single']
Allow the requirement to be satisfied by just one capability or provide wires to any capability that satisfies the requirement.

where multiple seems the default, I think slf4j should use single here to not pull in multiple providers (what is not supported). As you have made changes to slf4j in the past would you mind proposing such thing so that hopefully with a new release of slf4j the issue is resolved?

As you've already described later, that's what is already happening.
But since it seems to be common to specify a type attribute in the provider Manifests (e.g. type=simple), the other way round could be interesting in combination with a way to specify the provider to use explicitly.

For a flat class-path it's already possible to specify the factory explicitly.

https://github.com/qos-ch/slf4j/blob/4b4d533771e0501ffe1eb5e248e456f285357737/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java#L233-L244

For OSGi runtimes the existing property could actually be re-used and the selection could iterate through available providers and test their class name.

[laeubi]

the other way round could be interesting in combination with a way to specify the provider to use explicitly.

That is what I currently do, select one provider in the product configuration explicitly, the slf4j-simple is added additionally to what I already have selected :-)