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

Add EMF-packages declared in MANIFEST.MF to EPackage-registry too #52

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HannesWell
Copy link
Contributor

This is adds a OSGi Bundle-Tracker that adds the EMF-packages declared in a MANIFEST.MF as provides OSGi capabilities (which EMF can already generated) to the EPackage.Registry automatically at runtime. This makes it obsolete to declared EPackages in the plugin.xml (although the latter has precedence for compatibility).
Outside of a OSGi runtime the corresponding header of all MANIFEST.MF files discoverable by the provided classloader are read 'manually' to ensure EPackages are then registered too.

@merks that's the change I talked to about before Christmas and you said that @juergen-albert and @maho7791 are porbably interested in this as well.

This is currently a draft that needs more testing, although the part for OSGi runtimes has already been tested in our application, and ideally also tests. Ed, can you recommend a place for that?

@maho7791
Copy link

maho7791 commented Jan 8, 2025

Hi Hannes,

for a handling of EMF in a OSGi environment without a need of an extension registry or Equinox as framework we already have that:

https://github.com/geckoprojects-org/org.gecko.emf

It is a service based EPackage- and ResourceFactory -Registry, where you can inject Epackages as service as well as a ResourceSet.

This project is currently in move to the new Eclipse Fennec project. It also contains an extender, that reads models described in the Manifest. I dont know from scratch, if we already support the capabilities, but its not that hard to add this feature.

Maybe this goes in a similar direction?

@HannesWell
Copy link
Contributor Author

This project is currently in move to the new Eclipse Fennec project. It also contains an extender, that reads models described in the Manifest. I dont know from scratch, if we already support the capabilities, but its not that hard to add this feature.

Maybe this goes in a similar direction?

Hi Mark,
that's an interesting project, Ed also mentioned it. Yes it looks like a similar direction, but a much smaller change doing much less. My goal is (currently) just to avoid the need for a plugin.xml just to register EPackages. And since a MANIFEST.MF is necessary for all bundles, considering the capabilities was a great fit for that goal. In fact I have first developed the proposed Tracker internally as an immediate DS-componented. But it seems to be something that could be interesting for more users and thought that it when integrating it directly in EMF the tracker can be opened by the activator, reducing the number of services and increasing predictability.

Would adding this directly to EMF be a benefit for Gecko-EMF/Eclipse Fennec or could it even be harmful?
When implementing this I tried to be backwards-compatible, i.e. packages registered through the plugin.xml always should take precedence. Subsequently we could consider to promote this more e.g. by not generating the package-registration in the plugin.xml when the option to generate the capabilities is enabled or even make that option the default for new models.

@HannesWell HannesWell force-pushed the manifest-epackage-registration branch from eb6e412 to 670203e Compare January 8, 2025 10:57
@maho7791
Copy link

maho7791 commented Jan 8, 2025

In general I support this idea. We have it in https://github.com/geckoprojects-org/org.gecko.emf/tree/main/org.gecko.emf.osgi.extender
It would be nice, if this feature can be deactivated in the genmodel. Then we would be absolutely fine. I wanted to comment the our challenge with the dynamic service approach was, dealing with dynamic, when bundles are becoming active.

I suggest looking into the extension registry implementation. When does it start and track bundles for plugin.xml's. I believe they use the resolved state.
Your bundle tracker should track bundles in state resolved, to register the EPackages as early as possible. If you track e.g. active bundles, remember that these bundles can possibly run code (in activator or components). This in return means they could access the EPackageRegistry, where an expected EPackage isn't already registered, because to registering bundle the contains the ecore isn't already in state active. Starting this BundleTracker as early as possible is a good option. I think, this happens with the extension registry. That would result in an required early start for EMF ecore.

In general we should't have to care about start ordering.

Just as a comment

@HannesWell HannesWell force-pushed the manifest-epackage-registration branch from 670203e to 853a828 Compare January 8, 2025 13:33
@HannesWell
Copy link
Contributor Author

It would be nice, if this feature can be deactivated in the genmodel. Then we would be absolutely fine. I wanted to comment the our challenge with the dynamic service approach was, dealing with dynamic, when bundles are becoming active.

The tracker and automatic registration can only be active globally or not at all. At least I cannot tell how to selectively disabled it. Of course one can simply not have the corresponding capabilities in the MANIFEST.MF respectively not let EMF generate it.

Regarding dynamics: The Eclipse extension registry is static. I have to admit I cannot tell what happens if bundles are installed into or uninstalled from an Equinox runtime at a later point of time. At least I haven't found any code in EMF that removes any registered packages from the registry when a bundle is 'removed'.

... at least until now. The new Tracker, will also remove registrations of EPackages if the originate from a OSGi-capability in the MANIFEST.MF and a bundle is 'un-resolved', although to be exact a bundle can only have the state STOPPING or UNINSTALLED: https://docs.osgi.org/javadoc/osgi.core/8.0.0/org/osgi/framework/Bundle.html
But this means a registration is only removed if a bundle is uninstalled.

Btw. @maho7791 can you tell if a bundle gets a new classloader when it's stopped and (re-)started or only if it is uninstalled and (re-)installed? Since a new Classloader means a new class object I think the registration should be removed in that case, also to avoid keeping references to 'dead' classloaders or bundles.

I suggest looking into the extension registry implementation. When does it start and track bundles for plugin.xml's. I believe they use the resolved state. Your bundle tracker should track bundles in state resolved, to register the EPackages as early as possible. If you track e.g. active bundles, remember that these bundles can possibly run code (in activator or components). This in return means they could access the EPackageRegistry, where an expected EPackage isn't already registered,

Yes, absolutely. Agree and the tracker tracking all bundles in state Bundle.RESOLVED | Bundle.STARTING | Bundle.ACTIVE | Bundle.STOPPING, so everything except installed and uninstalled.

because to registering bundle the contains the ecore isn't already in state active. Starting this BundleTracker as early as possible is a good option. I think, this happens with the extension registry. That would result in an required early start for EMF ecore.

o.e.emf.ecore has Bundle-ActivationPolicy: lazy and opens the described Bundle-Tracker in it's activator with this change. This means as soon as e.g. the EPackage.Registry interface is loaded o.e.emf.ecore is started and the tracker is opened and tracking should start.
Is that what you suggest?

@merks
Copy link
Contributor

merks commented Jan 8, 2025

else
{
EPackage.Registry.INSTANCE.remove(packageURI);
if (ePackageNsURIToGenModelLocationMap != null)
{
ePackageNsURIToGenModelLocationMap.remove(packageURI);
}
return true;
}
}

@juergen-albert
Copy link

Btw. @maho7791 can you tell if a bundle gets a new classloader when it's stopped and (re-)started or only if it is uninstalled and (re-)installed? Since a new Classloader means a new class object I think the registration should be removed in that case, also to avoid keeping references to 'dead' classloaders or bundles.

You have to assume that you get a new Classloader, as some rewiring might happen, which may result in a new classloader.

@maho7791
Copy link

maho7791 commented Jan 8, 2025

It would be nice, if this feature can be deactivated in the genmodel. Then we would be absolutely fine. I wanted to comment the our challenge with the dynamic service approach was, dealing with dynamic, when bundles are becoming active.

The tracker and automatic registration can only be active globally or not at all. At least I cannot tell how to selectively disabled it. Of course one can simply not have the corresponding capabilities in the MANIFEST.MF respectively not let EMF generate it.

Globally deactivation is fine as well.

Btw. @maho7791 can you tell if a bundle gets a new classloader when it's stopped and (re-)started or only if it is uninstalled and (re-)installed? Since a new Classloader means a new class object I think the registration should be removed in that case, also to avoid keeping references to 'dead' classloaders or bundles.

A bundle gets a new Classloader instance when it reaches the resolved state. Stopping and starting a bundle may cause a re-resolve, So there might be a new Classloader instance after a new resolve was triggered. In any case the BundleContext will get a new instance after stopping and starting again.

o.e.emf.ecore has Bundle-ActivationPolicy: lazy and opens the described Bundle-Tracker in it's activator with this change. This means as soon as e.g. the EPackage.Registry interface is loaded o.e.emf.ecore is started and the tracker is opened and tracking should start. Is that what you suggest?

I just want you to keep in mind, not to end up in situations where some consumers of packages cannot find them, because they are not already registered.

When you use the static accessors for generated factories like PersonFactory.eInstance.createPerson(), the EPackage is registered automatically into the static EPackageRegistry. This is done by the generated code.

If you want to load an EObject using ResourceSet.getEObject(URI.createURI("/tmp/test.person") you will sooner or later need the ResourceFactory for file extension person and the PersonPackage from the corresponding registries. If they aret not already registered, the load will fail.

This means, this way of registration of the EPackages can cause start order problems. You can avoid this with starting o.e.e.ecore a early as possible, in the best case before any code can call e.g. ResourceSet.getEObject. But we dont want that.

Assume you have bundles in that order: A, Ecore, M1 with model 1, M2 with model person. Let A call 'ResourceSet.getEObject(URI.createURI("/tmp/test.person")' whereas bundle M2 contains the person.ecore. What happens:

  • A gets activated and calls 'ResourceSet.getEObject(URI.createURI("/tmp/test.person")'
  • Ecore is started, because of the class access of the EMF stuff.
  • The static Epackage and ResourceFactoryRegistries are instantiated
  • Your BundleTracker starts
  • In worst case, A gets no EPackage or ResourceFactory for person
  • You cannot lock the Registries until all models are regsitered, because you dont know how many models are there
  • Your tracker registers model 1 from bundle M1
  • Your tracker registers model person from bundle M2

This cannot happen with the Extension Registry. Because any access to it is based on the IExtensionRegistryService. Now the same with the extension registry:

  • A gets activated and calls ResourceSet.getEObject(URI.createURI("/tmp/test.person")
  • Ecore is started, because of the class access of the EMF stuff.
  • The static Epackage and ResourceFactoryRegistries are instantiated
  • To get EPackages and ResourceFActories from the extension registry, it wants all registrations, it calls for the IExtensionREgistryService over CorePlugin
  • Equinox Registry is started,
  • The activator creates a Registry and scans all at least resolved bundles for their plugin.xml
  • Bundle M1 will be scanned
  • Bundle M2 will be scanned
  • It registers a bundle listener
  • Now it registers the extension registry as service
  • Ecore gets the registry istance and all EPackages and ResourceFactories estensionhs and puts them in the EPackage/ResourceFactoryRegistry
  • A gets it Factories or EPackages -> happy, happy

After some thinking, It feels like the BundleTracker is not the way to solve it.

We can use it with our approach, because, we can delay an ResourceSet injection into an DS component until the requested EPackage is registered.

@HannesWell HannesWell force-pushed the manifest-epackage-registration branch from 853a828 to b5d7425 Compare January 16, 2025 14:03
@HannesWell HannesWell force-pushed the manifest-epackage-registration branch from b5d7425 to 8c5da88 Compare January 16, 2025 14:27
@HannesWell
Copy link
Contributor Author

The tracker and automatic registration can only be active globally or not at all. At least I cannot tell how to selectively disabled it. Of course one can simply not have the corresponding capabilities in the MANIFEST.MF respectively not let EMF generate it.

Globally deactivation is fine as well.

I actually think that it should work well by default without interfering negatively without the need to disable it at all.

Btw. @maho7791 can you tell if a bundle gets a new classloader when it's stopped and (re-)started or only if it is uninstalled and (re-)installed? Since a new Classloader means a new class object I think the registration should be removed in that case, also to avoid keeping references to 'dead' classloaders or bundles.

A bundle gets a new Classloader instance when it reaches the resolved state. Stopping and starting a bundle may cause a re-resolve, So there might be a new Classloader instance after a new resolve was triggered. In any case the BundleContext will get a new instance after stopping and starting again.

If we would do it absolutely correct we would probably have to remove and re-register all EPackages from that bundle in this case since otherwise the old classloader and class-object would leak in that case? OTOH as far as I can tell, this also doesn't happen for EPackages registered via the Eclipse Extension Registry respectively a plugin.xml. So currently this doesn't seem to be a big problem.

I just want you to keep in mind, not to end up in situations where some consumers of packages cannot find them, because they are not already registered.

I have checked the behavior of BundleTracker in detail and indeed the tracker doesn't process all existing bundles before its open() method returns (but it seem to process some 🫤). To avoid the problem you have described I have updated the code to read all bundles in the runtime after the tracker was opened. This way it's ensured that all bundles are processed before the Activator's start() method returns and the first scenario you have described doesn't happen. Since registrations are only added in case of absence the second asynchronous attempt to register packages isn't a problem either.

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.

4 participants