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

Replace embedded osgi-sources by osgi-bundles from Maven-Central #26

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

HannesWell
Copy link
Member

@HannesWell HannesWell commented Apr 25, 2022

As discussed in eclipse-equinox/equinox#18, this replaces the embedded osgi-sources of org.eclipse.equinox.app, org.eclipse.equinox.coordinator and org.eclipse.equinox.log.stream by corresponding osgi-bundles from Maven-Central.

@tjwatson as you suggested, for org.eclipse.equinox.coordinator and org.eclipse.equinox.log.stream the removed osgi packages are just imported.

For org.eclipse.equinox.app the bundle org.osgi.service.application is also required and reexported, because as you predicted org.eclipse.equinox.app itself is required and reexport by org.eclipse.core.runtime. Just removing the org.osgi.service.application package will break many bundles even directly in the Eclipse SDK so this will require some more coordinated effort to get rid of.
Maybe this could be the start to remove all reexports from org.eclipse.core.runtime.

@HannesWell
Copy link
Member Author

This again requires eclipse-platform/eclipse.platform.releng.aggregator#235 to be submitted first.

@HannesWell
Copy link
Member Author

After some debugging I found the cause of the test-failure to be that the System property "org.osgi.vendor.application.ApplicationDescriptor" is not set to the Equinox implementation of org.osgi.service.application.ApplicationDescriptor. Without that property set ApplicationDescriptor.Delegate failed in its static-inializer.
Unfortunately setting that property in the initializer of EclipseAppDescriptor to its fully qualified name does not work either. ApplicationDescriptor.Delegate then tries to load the class by its fully qualified name via Class.forName(), which delegates to the classloader of the calling class. But then OSGi's classloader encapsulation does its duty and does not find the class, since the bundle org.osgi.service.application does not have access to org.eclipse.equinox.app.
The same applies for the ApplicationHandle respectively EclipseAppHandle.

I wonder how is that supposed to work respectivly how can I make the equinox implementation accessible for org.osgi.service.application? I didn't find anything in the class or the OSGi spec that could help me here.
I start to question if we can use the org.osgi.service.application bundle or if the sources have to remain embedded into org.eclipse.equinox.app so that they can be modified to use the Equinox-implementation as suggested in the NOTE at the beginning of the class.

@HannesWell
Copy link
Member Author

Furthermore it looks like the Application Admin Specification was removed from the OSGi compendium with Release 8.
Chapter 116 Application Admin Specification , which is available up until Release 7 has vanished in 8. But I didn't find any removal announcement either.

@tjwatson
Copy link
Contributor

Yes, this is a dead specification. I suggest we just leave the source here (embedded). I had forgotten about the delegate stuff in there. I never did like the design of that API for this reason. The only way I could see to make it work reasonably was to modify the source to fit our implementation. This has worked up to now. I don't see us needing to evolve this API at this point and I see no need to spin our wheels trying to consume the classes from the OSGi artifact in maven central.

@HannesWell
Copy link
Member Author

Understand. Yes then consuming that jar from Maven-Central would not make our life easier. And given the require-bundle problem with that Plugin I agree that we should leave the OSGi sources in org.eclipse.equinox.app just as they are.
I have reverted that part of the change.

Just for curiosity is there a way to use the unmodified jars from Maven-Central? To me it looks like this is not possible without some hacks/magic within an OSGi application?

And update to latest version of
- org.osgi.service.coordinator
- org.osgi.service.log.stream
- org.osgi.util.pushstream

Discussed in issue:
https://github.com/eclipse-equinox/equinox.framework/issues/40
@HannesWell
Copy link
Member Author

Are you then fine with this change?
The build failure was just due to API-tool errors because it only want's a micro-version bump (but is questionable why it does not require a major one).
Nevertheless due to the nature of this change (we remove exported packages without replacement) I suggest perform a minor-version bump on org.eclipse.equinox.coordinator and org.eclipse.equinox.log.stream.
I added an api-filter to suppress that warning/error, so that the build hopefully passes.

@tjwatson
Copy link
Contributor

Just for curiosity is there a way to use the unmodified jars from Maven-Central? To me it looks like this is not possible without some hacks/magic within an OSGi application?

Here you are asking about the org.osgi.service.application? If so then then my answer is essentially "no". You could think of some way to attach the implementation as a fragment to org.osgi.service.application. But at that point we loose the actual org.eclipse.equinox.app bundle. I guess you could split the impl into two things. A fragment that has the delegate implementation and then adds an import to the host so that the delegate can get back to the actual implementing bundle. But this is all way overly complicated. The only way I saw to do this nicely was to modify the source and put it directly in the implementation. As you say the NOTE at the beginning of the class indicates this is allowed.

In retrospect it may have been best to not model the Eclipse application model on the OSGi application admin specification. At the time we were separating this bit out of the org.eclipse.core.runtime bundle this new specification was being developed at OSGi and it seemed like a natural fit. But I am not convinced we gained anything at the Eclipse platform level by allowing Eclipse IApplication stuff stuff to be managed by the OSGi Application Admin specification.

@HannesWell HannesWell merged commit 2070005 into eclipse-equinox:master Apr 29, 2022
@HannesWell HannesWell deleted the osgiBundles branch April 29, 2022 16:04
@HannesWell
Copy link
Member Author

Just for curiosity is there a way to use the unmodified jars from Maven-Central? To me it looks like this is not possible without some hacks/magic within an OSGi application?

Here you are asking about the org.osgi.service.application? If so then then my answer is essentially "no". You could think of some way to attach the implementation as a fragment to org.osgi.service.application. But at that point we loose the actual org.eclipse.equinox.app bundle. I guess you could split the impl into two things. A fragment that has the delegate implementation and then adds an import to the host so that the delegate can get back to the actual implementing bundle. But this is all way overly complicated. The only way I saw to do this nicely was to modify the source and put it directly in the implementation. As you say the NOTE at the beginning of the class indicates this is allowed.

Yes, using a fragment should work, but as you said would thinks more complicated compared to the current one. Leaving it as it is IMHO the better way. Especially since the OSGi Application Admin specification is not developed further.

In retrospect it may have been best to not model the Eclipse application model on the OSGi application admin specification. At the time we were separating this bit out of the org.eclipse.core.runtime bundle this new specification was being developed at OSGi and it seemed like a natural fit. But I am not convinced we gained anything at the Eclipse platform level by allowing Eclipse IApplication stuff stuff to be managed by the OSGi Application Admin specification.

Too bad, but as long as it does not hinder us it is Okey.

Because it is not developed further by OSGi is it then OK to fix the warnings in those osgi-application source file that occur when you enable them? Usually we should use a 1:1 copy of the originals but in this case I think an exception would be OK. I'm asking because I noticed there is some potential for clean ups in o.e.equinox.app (and probably the entire equinox-bundles repo) and I have the intention to do that.

@laeubi
Copy link
Member

laeubi commented Apr 30, 2022

Just for curiosity is there a way to use the unmodified jars from Maven-Central? To me it looks like this is not possible without some hacks/magic within an OSGi application?

If the jar from central is a bundle you can use it in OSGi, no hacks required. Beside that, pax-url offers a wrap protocol to wrap non-bundle jars, together with the mvn protocol you can even consume directly from maven central any jar...

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

Successfully merging this pull request may close these issues.

None yet

4 participants