Adds Feature #1568 - [OSGi] Opting in to Service Loader Mediator

[fipro78]

• Update bnd-maven-plugin to 6.4.0 (not directly to 7.0.0 as that requires Java 17 for the build)
• Change the Automatic-Module-Name to jpms-module-info
  With this the module-info.class gets generated by Bndtools to the bundle file. This should resolve #1451 then.
• Added the @aQute.bnd.annotation.spi.ServiceProvider annotation to the factories. This annotation triggers the generation of
  • The Require-Capability header for the Service Loader Mediator
  • The Provide-Capability header for all provided services
  • The service provider configuration files in META-INF/services
• Remove the static service provider configuration files and the generation of those files for the primitive collections, as they are now generated and don't need to be maintained manually anymore. This way the META-INF/services don't contain provider configuration files of services that do not exist anymore (see in a comment below)
• Change the p2 update site build (should fix #1501)
  • disable p2-repository module which uses EBR.
  • reactivate p2-feature to build a p2 update site via Tycho.
    • The p2 update site build is not included in the main build anymore and needs to be triggered separated.
    • It uses the artifacts from Maven as dependencies and provide them via p2 update site, instead of generating a new wrapped bundle of API and Impl. This makes it necessary to include a Service Loader Mediator Implementation when using Eclipse Collections in the future.
• Minor update in the parent pom.xml
  • Updated some versions
  • Removed the indentation in the description which partly makes PR #1468 obsolete. Only thing that is missing for #1468 is the warning about missing developer id's. Where actually I would suggest to add the id to the developers in the parent pom.xml to avoid the warning. But that is a project decision how they want to deal with this. The suggestion from @Bananeweizen to have a general Bundle-Developer in the manifest is also suitable.
• Minor updates in the generator files for the primitive factories, to get rid of the warnings in the Javadoc creation process.

[fipro78]

@timothyjward As an OSGi expert, could you please at least verify if my assumption and approach for the fix is correct? I tested it locally in an Maven based OSGi-application, and it works fine. But getting feedback from you would be very beneficial.

[timothyjward]

@timothyjward As an OSGi expert, could you please at least verify if my assumption and approach for the fix is correct?

Hi - I think the extender requirements that you have added look fine, but the implementation seems to be missing osgi.serviceloader capabilities for the implementations that it wants to register. This is described in the specification at https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.loader.html#d0e106889 and has an example at https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.loader.html#d0e107018

For reference I think you would need entries for each of the implementations registered in https://github.com/eclipse/eclipse-collections/tree/master/eclipse-collections/src/main/resources/META-INF/services

There are a lot of these, so you may wish to consider having bnd generate all this for you (including META-INF/services files) using the @ServiceProvider annotation https://bnd.bndtools.org/chapters/240-spi-annotations.html#serviceprovider

[fipro78]

@timothyjward Thanks for the feedback. From the specification point of view, you are totally right that the osgi.serviceloader capabilities need to be added. Interestingly it doesn't need to be required at runtime. SPI Fly seems to inspect META-INF/services and process everthing in there, even without the capabilities.

I will check if I can use the @ServiceProvider annotation. That would even reduce the maintenance effort on the project side, as the provider configuration files don't need to be maintained manually anymore.

[fipro78]

I adapted the request from @timothyjward and use @ServiceProvider for the generation of the SPI resources. This way also the provider configuration files are generated. I therefore also removed the generation of those resources.

This also revealed a current issue in the eclipse-collections bundle. The following files are currently included in META-INF/services but the related implementations do not exist anymore. I suppose they were dropped in the meanwhile, but the services files are still generated:

org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanBooleanMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanByteMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanCharMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanDoubleMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanFloatMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanIntMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanLongMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanShortMapFactory

org.eclipse.collections.api.factory.map.primitive.MutableBooleanBooleanMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanByteMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanCharMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanDoubleMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanFloatMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanIntMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanLongMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanShortMapFactory

org.eclipse.collections.api.factory.set.primitive.ImmutableBooleanSetFactory

[fipro78]

@nikhilnanivadekar Sorry that this PR now got that big. But it was necessary to be able to build an update site with Tycho once the meta information in the MANIFEST files get generated.

[nikhilnanivadekar]

@fipro78 trying to understand the PR. Is it possible to logically squash the commits? Multiple commits are ok, but some of them are overwritten and reviewing would be cumbersome.

Also, if you wouldn't mind explaining what will be the new process for p2 update site and to contribute to the Simrel repo? I am not an expert in this area. So, I will need (a lot of) help as I learn. Thanks!

[fipro78]

@fipro78 trying to understand the PR. Is it possible to logically squash the commits? Multiple commits are ok, but some of them are overwritten and reviewing would be cumbersome.

After some struggles, I hope I have correctly squashed the commits.

Also, if you wouldn't mind explaining what will be the new process for p2 update site and to contribute to the Simrel repo? I am not an expert in this area. So, I will need (a lot of) help as I learn. Thanks!

Well, with this PR the p2 update site is created from the p2-feature project, and not the p2-repository anymore. And the p2 update site is not created on every build, but needs to be triggered manually. That means, the Jenkins job that publishes the p2 update site needs to be changed to build the p2-feature and publish the results from the p2-feature/org.eclipse.collections.repository/target.

With regards to contribute to the Simrel repo, this PR actually has no impact. What this PR does is, it ensures that the artifacts, that are published to Maven Central, can also be used in an OSGi context. This was not really possible as of now, because the API retrieves services from the Impl, but they reside in different bundles, and therefore we have classloader issues. That is why the previous EBR approach in p2-repository creates a new wrapped bundle that combines API and Impl. The wrapping should be not needed anymore, if a Service Loader Mediator implementation is available in the runtime.

Whether Eclipse Collections become part of Simrel or will be included in Eclipse Orbit, is out of the scope of this PR. But without additional build steps, it should be easy to include the artifacts from Maven Central there. It would be only needed to consume those artifacts, as they are OSGi bundles.

Of course that need to be validated again, to be sure that it is really working. I suppose @merks asked whether there is a Maven repository to consume the artefacts before they are actually released, so it can be verified how easy it would be. And maybe he can then also give some more information with regards to including Eclipse Collections to Orbit or Simrel. In the end, the question is, who is responsible for the publishing of the p2 update site. And IMHO that depends on how easy it becomes to consume the artifacts from Maven Central, rather then needing to create a wrapped bundle.

@merks Have I explained that correctly? Do you want to add something? Because Simrel and Orbit is definitely your domain. ;-)

[merks]

PDE, with m2e's help, as well as Tycho, support specifying Maven dependencies in a *.target. Orbit uses that specifically here:

https://github.com/eclipse-orbit/orbit-simrel/blob/main/maven-osgi/tp/Maven.target

As background, this file is generated/updated automatically by looking at the *.target of various SimRel projects and looking at Maven Central for new versions. As part of that analysis, the following target is also considered:

https://github.com/eclipse-orbit/orbit-simrel/blob/main/maven-osgi/tp/other/MavenSupplement.target

So if Eclipse Collections publishes well-formed OSGi artifacts to Maven Central we can trivially include it in MavenSupplement.target so that it's generated into Maven.target and in the end, is available in these Orbit p2 repositories:

https://download.eclipse.org/tools/orbit/simrel/maven-osgi/ https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/

Note that is is a convenience for the consumers because the consumers can include such dependencies directly in their own *.target, in which case they are also responsible for PGP signing those artifacts, which Orbit also does as an additional convenience.

Once that's in place, the Eclipse Collection's SimRel contribution should be removed and any other project relying on those bundles, must contribute them via the p2 repository of their own SimRel contribution.

So I'm happy with this path forward. The overhead for Orbit is trivial and is just done once, after which updates will be automatically generated.

[donraab]

Hi @fipro78, thanks for spotting this. I don't believe these classes ever existed. IIRC, we had a discussion a long time ago around the benefit of having boolean to anything Map and decided it wasn't worth coding or generating. These must have been mistakenly added to the services.

I adapted the request from @timothyjward and use @ServiceProvider for the generation of the SPI resources. This way also the provider configuration files are generated. I therefore also removed the generation of those resources.

This also revealed a current issue in the eclipse-collections bundle. The following files are currently included in META-INF/services but the related implementations do not exist anymore. I suppose they were dropped in the meanwhile, but the services files are still generated:

org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanBooleanMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanByteMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanCharMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanDoubleMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanFloatMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanIntMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanLongMapFactory
org.eclipse.collections.api.factory.map.primitive.ImmutableBooleanShortMapFactory

org.eclipse.collections.api.factory.map.primitive.MutableBooleanBooleanMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanByteMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanCharMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanDoubleMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanFloatMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanIntMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanLongMapFactory
org.eclipse.collections.api.factory.map.primitive.MutableBooleanShortMapFactory

org.eclipse.collections.api.factory.set.primitive.ImmutableBooleanSetFactory

[fipro78]

@motlin @nikhilnanivadekar @donraab The @aQute.bnd.annotation.spi.ServiceProvider is an annotation with class time retention. That means there is not additional runtime dependency by using this annotation. It is only used to tell bndtools to generate the SPI resources and the necessary headers in the MANIFEST.

And is there a build result for the PR that could be consumed by @merks to verify if the changes can be consumed easily in the Eclipse environment?

@motlin I have no idea why the Javadoc build fails now. I tested it locally and there it works. Maybe you have an idea on this?

@timothyjward @merks I know you already had a look. But could you please have a second look at this PR to give your opinion on the changes? The project team has limited knowledge about OSGi and bndtools, and therefore it is hard for them to verify this PR.

[motlin]

The @aQute.bnd.annotation.spi.ServiceProvider is an annotation with class time retention.

I'm familiar with com.google.auto.service.AutoService which seems similar and also has class time retention. What's the difference between then? Does ServiceProvider also power the generation of manifest entries?

[motlin]

I have no idea why the Javadoc build fails now. I tested it locally and there it works. Maybe you have an idea on this?

The error message is:

Caused by: org.eclipse.aether.transfer.ArtifactNotFoundException: Could not find artifact org.eclipse.collections:eclipse-collections-code-generator-maven-plugin:jar:12.0.0-SNAPSHOT in repo.eclipse.org

There are two things I find odd about this.

• We ran mvn install on that module, so it ought to exist in .m2 and we shouldn't be looking for it in on the internet at all
• Even if we were looking for it on the internet, why are we looking in repo.eclipse.org rather than maven central?

Actually looking at the diffs, it seems that you changed mvn install to mvn verify in pull_request.yml and that could definitely be the cause. Do you remember the reason for that change?