Community
Participate
Working Groups
BundleErrorReporter currently has a fixed set of headers/properties it checks. But as OSGi is extensible (e.g. JPA Service Spec offers a Meta Persistence Header [1]) and thus PDE should offer some means for plugins to contribute to these checks. The extension interface could be as simple as public interface IBundleHeaderValidator { void validate(IFile file, IPluginModelBase model, Map<String, JarManifestHeader> headers); } [1] https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.jpa.html#i3088219
A direct consumer of this function could be E4 as it recently introduced a custom header value. Instead of implementing logic in PDE it could simply contribute a new checker.
+1 for the functionality for the signature: do you think it is reasonable to use something that returns IStatus?
And thank you Christoph for starting this effort! To be honest I also need this functionality to validate the header that defines licensed functionality: ``` Provide-Capability: licensing.feature;licensing.feature="org.eclipse.passage.example.e4ff.product";name="Eclipse Passage Example E4FF";version="1.0.0.qualifier";provider="Passage E4FF Template";level="warn",licensing.feature;licensing.feature="antimagic-shield";name="Antimagic protection";version="2.72";provider="Passage E4FF Template";level="warn" ``` here we have a number of attributes to check for consistency with "licensing operator" data. Please add me to the gerrit if you plan to create it - I will check if it fits my case as well.
(In reply to Alexander Fedorov from comment #2) > for the signature: do you think it is reasonable to use something that > returns IStatus? I first though of something like this, on the other hand last time I used markers it was very individual (marker type, custom attributes,...) that I'm not sure if it fits to be implemented in a generic way and maybe better not restrict to just return a status here. And as the current BundleErrorReporter don't use something generic here but has specific check methods for several aspects with individual handling I can't simply plug into there so need to invent something new in PDE. Thats why I'd like to pass the File (maybe just a resource is even better), so it is easy to create markers on them, but if you (or anyone else) is aware of a generic "StatusMarker" in the platform that fits here let me know.
I think it might be possible to map IStatus to the internal "VirtualMarker, and finally map those into "real" markers but currently the validator uses mainly IHeader but on some places IManifestHeader are used, both are not public API but the later seems semantically more useful if it comes to validation. As IManifestHeader does not extend IHeader it seems to be different on the other hand... I think the major 'blocker' here is that there is no public common API, the question is: should could we provide an extension point based on internal API? Another way I can think of is, one could use the BundleDescription from the model for the validation extension (but then I don't see a way for generic IStatus handling). Then the signature might looks like: public interface IBundleHeaderValidator { void validateHeader(IResource resource, BundleDescription description); }
> I think the major 'blocker' here is that there is no public common API, the question is: should could we provide an extension point based on internal API? I think we can suggest a public API for what we need. I'm sceptic about moving existing big PDE classes with vintage implementation to public packages, but well focused new interfaces that could be implemented internally with the current code seems to be a realistic way to go.
(In reply to Christoph Laeubrich from comment #5) > I think the major 'blocker' here is that there is no public common API, the > question is: should could we provide an extension point based on internal > API? No, all API used in the extension point must be public. > I think it might be possible to map IStatus to the internal "VirtualMarker, > and finally map those into "real" markers but currently the validator uses > mainly IHeader but on some places IManifestHeader are used, both are not > public API but the later seems semantically more useful if it comes to > validation. As IManifestHeader does not extend IHeader it seems to be > different on the other hand... What API would you need from IManifestHeader? IHeader seems to be the one designed for validation and can safely be made public API. IManifestHeader on the other side is part of the bundle model and should not be published on its own. > Another way I can think of is, one could use the BundleDescription from the > model for the validation extension (but then I don't see a way for generic > IStatus handling). Is the OSGi state guaranteed to be updated before the builder runs? (In reply to Christoph Laeubrich from comment #4) > I first though of something like this, on the other hand last time I used > markers it was very individual (marker type, custom attributes,...) that I'm > not sure if it fits to be implemented in a generic way and maybe better not > restrict to just return a status here. Yes, we should not limit the API to severity + message. That would not even allow an accurate positioning of the marker. I think we should create an IMarkerFactory interface { void createMarker(String type, Map<String, ? extends Object> attributes) } and pass that to the extension. The implementation can then be transparently hooked into the VirtualMarkers. To summarize, I propose the following API: public interface IBundleHeaderValidator { void validate(IFile manifestFile, Map<String, IHeader> headers, IMarkerFactory markerFactory); }
(In reply to Julian Honnen from comment #7) > What API would you need from IManifestHeader? IHeader seems to be the one > designed for validation and can safely be made public API. I don't feel anything missing (and if it might be possible to add) I'm just not very familiar with PDE API and try to guess what best fits here. IManifestHeader just reads more suitable than a generic 'IHeader' but in the end it does not mater much. > I think we should create an IMarkerFactory interface { > void createMarker(String type, Map<String, ? extends Object> attributes) > } > and pass that to the extension. The implementation can then be transparently > hooked into the VirtualMarkers. > > > To summarize, I propose the following API: > > public interface IBundleHeaderValidator { > void validate(IFile manifestFile, Map<String, IHeader> headers, > IMarkerFactory markerFactory); > } That sounds good, if you have a clear understanding on how it could best be designed it, would it be possible to add the API for it?
(In reply to Christoph Laeubrich from comment #8) > That sounds good, if you have a clear understanding on how it could best be > designed it, would it be possible to add the API for it? I could quickly start with the API and integration if you finish the extension point and test?
(In reply to Julian Honnen from comment #9) > (In reply to Christoph Laeubrich from comment #8) > > That sounds good, if you have a clear understanding on how it could best be > > designed it, would it be possible to add the API for it? > I could quickly start with the API and integration if you finish the > extension point and test? That sounds reasonable when the API is there I should be able to created the an extension point from it and integrate it with BundleErrorReporter.
New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/179506
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/179506 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=65e2088a1ed8bc721a6b79cfcc1b41716841e77c
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
