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

Support usage of internal PDE code #601

Open
gireeshpunathil opened this issue May 23, 2023 · 6 comments
Open

Support usage of internal PDE code #601

gireeshpunathil opened this issue May 23, 2023 · 6 comments

Comments

@gireeshpunathil
Copy link
Contributor

A product, which conducted a code scanning reported that it uses a number of internal programming interfacing for lack of exported and documented APIs to meet their desired outcome. These are:

  • IBundlePluginModelBase.getBundle
  • Bundle.getHeader
  • PDECore.getDefault().acquireService

Are there better / external aletrnatives? If not, can we covert these into APIs? If cannot, can we make sure (by putting a comment in those APIs or so) so as not to deprecate/announce/remove them without giving a replacement?

@merks
Copy link
Contributor

merks commented May 23, 2023

You're not the only one using PDE internals. This is a search in Oomph's workspace:

image

There are often no good alternatives. Converting to APIs now, after the fact, will just tend break what's working now for others using internals. And who is going to create new APIs along with a promise to maintain them forever?

If someone breaks such an API later, very often you can work around that with reflection (which makes it even hard to notice a problem until runtime)....

@vik-chand
Copy link
Member

You're not the only one using PDE internals. This is a search in Oomph's workspace:

image

There are often no good alternatives. Converting to APIs now, after the fact, will just tend break what's working now for others using internals. And who is going to create new APIs along with a promise to maintain them forever?

If someone breaks such an API later, very often you can work around that with reflection (which makes it even hard to notice a problem until runtime)....

Agree with this assessment !

@HannesWell
Copy link
Member

* PDECore.getDefault().acquireService

Instead of this methods I suggest to use OSGi standard methods to obtain a OSGi service. For often used services I suggest to use an OSGi ServiceTracker, for only rarely fetched services something like eclipse-equinox/equinox#146 is probably more convenient (I hope to land it next release cycle).

* IBundlePluginModelBase.getBundle

I assume you are referring to IBundleModel getBundleModel()?
Depending on your exact use case IPluginModelBase.getBundleDescription() could be suitable as well.
But I want to mention that, if PDE moves away from using the old OSGi/Equinox Resolver API and use the OSGi wiring API instead that method will likely vanish to (of course with the default deprecation period). But in order to prepare for that you should not rely too much on BundleDescription but better on its super-interface BundleRevision.
However at the moment this has the downside that the implementation of BundleDescription using in PDE at the moment does not implement all methods of BundleRevision to the full extend. For example (IIRC) BundleRevision.getBundle() usually returns null.
I have the intention to do the mentioned migration one day, but this will be a bigger task and I have not concrete plan for that yet.

* Bundle.getHeader

I assume you mean IBundle.getHeader(String) respectively one of its two implementations?

Depending on which headers you are querying, IPluginModelBase.getBundleDescription() might again be an alternative. But again the recommendation to read the 'connections' to other Bundles by using the OSGi Wiring API mainly accessible through the BundleWiring returned by BundleRevision.getWiring().

@gireeshpunathil
Copy link
Contributor Author

thanks @HannesWell! this is really useful. I will get the product focal to get more information to gain clarity on your questions, or they might respond to those directly here.

@davenice
Copy link

davenice commented Jun 8, 2023

Thank you @HannesWell - essentially our code is given an IProject and we want to get more information from it.

So we use getBundleDescription to get the symbolic name through API, but we want the exact version string from the manifest, which we do roughly like this:

		IPluginModelBase manifestModel = PluginRegistry.findModel(aProject);
		IBundlePluginModelBase bundlePluginModel = (IBundlePluginModelBase)manifestModel;
		IBundleModel bundleModel = bundlePluginModel.getBundleModel();
		Bundle bundle = (Bundle) bundleModel.getBundle();
		result.version = bundle.getHeader(MANIFEST_MF_VERSION_KEY);

I've cut out quite a few intervening lines where we access other information.

@HannesWell
Copy link
Member

So we use getBundleDescription to get the symbolic name through API, but we want the exact version string from the manifest, which we do roughly like this:

For the version, can't you just use PluginRegistry.findModel(aProject).getBundleDescription().getVersion()?
This returns a Version object of the parsed version from the manifest. This gives you a semantically equivalent representation of the version string in the manifest. Of course you then cannot tell if one has specified 1.0 or 1.0.0, but this usually not relevant.
And AFAICT the OSGi-spec does not permit any attributes or directives on the value of the Bundle-Version header.

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

No branches or pull requests

5 participants