Bugzilla – Full Text Bug Listing
| Summary: | Bundles marked as x-friends should (optionally) not be flagged with API Usage errors | ||
|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Martin Oberhuber <mober.at+eclipse> |
| Component: | API Tools | Assignee: | PDE API Tools Inbox <pde-apitools-inbox> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | curtis.windatt.public, darin.eclipse, john.cortell, Michael_Rennie, stepper |
| Version: | 3.4 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
|
Description
Martin Oberhuber
Being a friend allows one to reference internals without warnings - i.e. one can use internal implementations. It's nore clear that this should allow one to break API rules of engagement. I think they could be different things. I agree with Darin. I think an example makes it obvious that a separate mechanism is needed: - I have a plugin com.acme.recorder.core that exports package com.acme.recorder.core.interfaces. - The package isn't internal. I.e., it's visible to all downstream plugins - In the package is interface com.acme.recorder.IRecord - IRecord is meant to be used by clients but not implemented by them. It's marked with @noextend and @noimplement. - I also have a plugin com.acme.recorder.digital. That plugin has implementations of IRecord. Currently that results in an API tooling violation. I can't use the existing the OSGi x-friends mechanism to allow the recorder.digital plugin to be excluded from the IRecord API restriction for the simple reason that IRecord is in a public package. You can't associate x-friends to a package that is visible to all downstream plugins. Everyone is automatically a friend of that package. I'm seeing in Eclipse 3.6M4 that API Usage restrictions warnings are being silenced by x-friends settings, which is troubling, as its hiding warnings that I don't want hidden. Before I explain, can someone tell me if the behavior has intentionally changed? Nevermind. Looks like a misdiagnosis. I *understand* the arguments from Darin in comment #1 and from John in comment #2 but I think we should be a bit more pragmatic as the current situation is really annoying. If someone is really insisting on x-friends being ignored can't we make that configurable? It would be possible to make this configurable. By default x-friends should be flagged with illegal API use - but there could be an option to allow illegal use between x-friends. Darin, do you have an idea on when it could be available? (In reply to comment #7) > Darin, do you have an idea on when it could be available? I'm not sure this will make the 3.7 plan based on available resources/priorities. It's possible that someone will get to implementing this (I don't think it's that large of a work item), but a patch is always welcome. I'm still very keen on this new optional setting/functionality. Would it be possible to get it soon in this new cycle? There are currently no plans to work on this in the 3.8 cycle I am inclined to mark this wontfix, the reason being that consumers of restricted API can simply add a filter for the illegal usage. To me it makes more sense to add filters as it forces consumers to explicitly acknowledge that they are breaking the rules, rather than all current and future illegal use not being reported at all because of x-friends use. (In reply to comment #11) > I am inclined to mark this wontfix, the reason being that consumers of > restricted API can simply add a filter for the illegal usage. To me it makes > more sense to add filters as it forces consumers to explicitly acknowledge > that they are breaking the rules, rather than all current and future illegal > use not being reported at all because of x-friends use. That's clearly the way with less (no) effort in API Tools. But I hoped that we could get something as cool as http://bndtools.org/whatsnew2-0-0.html#enhanced-semantic-versioning ;-( (In reply to comment #12) > That's clearly the way with less (no) effort in API Tools. But I hoped that > we could get something as cool as > http://bndtools.org/whatsnew2-0-0.html#enhanced-semantic-versioning ;-( The more I think about this, perhaps we could add this support in, but have it off by default. I ran into a very good use case for this working on the new platform UI APIs where one bundle provides the restricted interfaces and then many more provide the default implementations of those interfaces - in that use case it is time-consuming to add a pile of filters for each use, when all of them would be allowed (and the illegal access restrictions are also removed via the x-friends directive). We already have all of the framework hooks in API tools to support this, see: 1. IApiAccess 2. IApiDescription#resolveAccessLevel I would envision the solution being as simple as adding a check in our problem detectors' #considerReference callbacks. (In reply to comment #13) > The more I think about this, perhaps we could add this support in, but have > it off by default. I ran into a very good use case for this working on the > new platform UI APIs where one bundle provides the restricted interfaces and > then many more provide the default implementations of those interfaces - in > that use case it is time-consuming to add a pile of filters for each use, > when all of them would be allowed (and the illegal access restrictions are > also removed via the x-friends directive). In all the examples I can find in Platform UI we run into the case described by John in comment #2. The package declaring the interface is public API (exported with no restrictions). Adding the x-friend setting treats the package as internal. For example org.eclipse.ui.internal.themes.CascadingColorRegistry illegally extends org.eclipse.jface.resource.ColorRegistry We would need an entirely new mechanism to support this so I am not in support of this for Kepler. Two more possible solutions that I wouldn't recommend: 1) Reverse the x-friend lookup. In the example, org.eclipse.ui.internal.themese would x-friend jface. This would look very wrong in the manifest though because you are stating that cannot depend on you is allowed to use your internal code. 2) Look for other x-friend directives. If another package declares your bundle as an x-friend ignore all illegal api use from that bundle. I think this is a poor idea because it is opaque to the user, x-friending a single package would turn off all illegal use checks. (In reply to comment #14) > We would need an entirely new mechanism to support this so I am not in > support of this for Kepler. Have to agree with this. I'm not in favor of creating our own way to handle this. > Two more possible solutions that I wouldn't recommend: > 1) Reverse the x-friend lookup. In the example, > org.eclipse.ui.internal.themese would x-friend jface. This would look very > wrong in the manifest though because you are stating that cannot depend on > you is allowed to use your internal code. > 2) Look for other x-friend directives. If another package declares your > bundle as an x-friend ignore all illegal api use from that bundle. I think > this is a poor idea because it is opaque to the user, x-friending a single > package would turn off all illegal use checks. Agree again. Neither of those sound good. Since we have a perfectly good system of using filters (and commented filters), I am going to close this wontfix. |