Community
Participate
Working Groups
All the metadata related interfaces should have the notice that they should not be implemented and they should also have the '@noimplement' API tooling tag (http://127.0.0.1:49685/help/index.jsp?topic=/org.eclipse.pde.doc.user/reference/api-tooling/preferences/ref-errorswarnings.htm)
Great point. I think we talked about this a bit and then the topic got dropped. We are expecting people to implement the interfaces, that's why they were put in there :-) However, the implementors are to be SPI folks not regular clients. I don't recall what convention we decided to use in this cases but the @noimplement tag seems a little harsh.
The primary goal of the java doc is to guide typical users (e.g. someone willing to use p2 API to install software) which is why I'm recommending the addition of this particular tag. I do understand the duality in the role of IInstallableUnit but as you said this is for SPI implementors, and in this particular case I don't really expect this to be used as a main mechanism to extend p2. As I expressed previously in bug #256359 this should only be used as a last resort mechanism and should not be recommended as a general mechanism. We have to remember that what motivated the addition is really the lack of consistency around the API if installable unit (some classes were interfaces some classes). If we believe that this will cause confusion, then we may have to review the addition of this API.
did you have a specific comment in mind from bug 256359? AFAIK the reason for putting the interfaces in was precisely so we could replace the implementation. Ian and the Cloudsmith folks both saw usecases where this was important. What alternative implementation strategy are you thinking of exposing? As for causing confusion, I don't think we'll need to resort to removing the interfaces. The question at hand is what convention we should use for this sort of case. Eventually having some SPI related @ tags for the API tools would be good but in the mean time, perhaps some textual softening of the @noimplement tag would work? For example: @noimplement - this type/method/... is SPI and not intended to be implemented by typical clients. Should we also have @noextend? certainly for normal clients they are just supposed to use the type. Should SPI users be extending the interfaces? BTW, the API doc (http://wiki.eclipse.org/SPI_description) has some thoughts on this issue. Perhaps @spi should be used as well?
@spi makes sense, however, the document also suggests moving spi classes / interfaces to a special package. I don't think this makes sense in our case as these interfaces are API (provisional) for callers and SPI for implementors. Is something like the SWT widget class a good example? Maybe the wording in widget is too harsh.
SWT is not a good example as they have interfaces. It is often ideal to put the SPI in a package of its own but as you see that is not always possible. I hesitate to introduce @spi but if we are putting in random canned text to say that the thing is SPI then it is no worse and possibly better to also have a tag convention
Here is a scenario which may help to solidify (and exemplify) the problem. Say we have a bug fix for a service release, and the best fix involves adding a new method to one of these interfaces. * This definitely breaks API if we consider implementors API * This arguably breaks SPI, as the document Jeff pointed to suggested that SPI changes should increase the minor segment * This clearly breaks binary compatibility What makes the most sense in this case: 1. Make the change (and the notice in the header should state that this is break is likely) 2. Feel shame, but make the change (announce it, make people aware, but realize in the end this was to be expected) 3. Don't make the change (find some other way to fix the bug, and then do it properly when the minor segment is increased) {i.e. follow the "rules" of SPI} 4. Never make the change {i.e. follow the "rules" of API} I think we can all agree that (4) is out. Thoughts?
In framework we commonly have API that everyone uses but does not implement. That API is binary compatible and users are never broken. Under that there is SPI that people implement/use. We change that from time to time and break the SPI folks. The amount of change is decreasing and we don't break people lightheartedly but it happens. at some point we will declare it to be supported but given the number of poeple affected and the impact of not being able to change these fundamental bits, the tradeoff is reasonable IMHO. I suppose this is #2 without the shame. In the end this comes down to the wording of the "SPI" statement.
I think our standard boilerplate for @noimplement applies here (my emphasis): This interface is not intended to be implemented by *clients* If you are implementing a service you are not a client, and thus our binary compatibility contract *for clients* isn't applicable. This is consistent with OSGi conventions. For example BundleContext has had methods added over the years, which is not a binary-compatible change for framework implementers, but is fine for clients who simply reference it. Implementers can live without cross-release binary compatibility by closely specifying the version of the bundle/package they implement. I think of SPI as real API, but distinguished/separated from other API to be used by a particular specialized group of clients. I don't think this applies here, since APIs like IInstallableUnit, IRequiredCapability, etc, would eventually be used (but not implemented) by a wide range of clients when it becomes API. Re comment #6: historically we have never added API in service releases, since bundles in service releases should be completely interchangeable and this wouldn't be the case if we added API. I agree this case could hypothetically occur but I'm not sure it's too important. I agree with Jeff that approach #2 would be the way to go here.
I'm fine with the @noimplement and it client oriented messaging. For SPI however, we are actually expecting repository implementors (i.e., Service Providers, the SP in SPI) to, if they choose, provide their own implementations of the metadata interfaces. This is the same as someone implementing the Team repository interfaces or Resources builder interfaces or... Seems it would be worthwhile pointing out to SPI folks that they could be interested in this stuff. In any event, I'm no longer feeling particularly strongly about this beyond having @noimplement so...
The only problem I see with @noimplement is that it may appear as if p2 expects the default implementation of these classes, that is IMHO @noimplement means "don't implement this interface, p2 may downcast to a particular concrete type". I think we need to clearly state that we don't expect (nor encourage) the implementation of these interfaces, but it can be done (although if you do this, you may be broken even between builds).
I am adding the notices (@noimplement and @noextend) to these interfaces and I have a few questions: 1. Can somebody enable API tooling for the metadata bundle? I noticed it was not enabled. I could do it, but I'm not sure how the baseline stuff works for us. 2. Should I add @since tags?
Feel free to add the @since tag as well. Let's do the enablement of the API tooling in another bug.
Created attachment 122494 [details] Adds the notice to the headers of the metadata interfaces Let me know if you want more than this.
I opened bug 260966 for the enablement of API tools.
Created attachment 122501 [details] Updated notice I talked to Jeff off-line and he suggested we not add the @since for two reasons. 1. We don't have API yet, so it doesn't really make sense to be specifying this (and we don't want people to think this is API) 2. We don't do this anywhere else in p2. This patch adds the API tooling messages without the @since tags.
Patch applied. thanks Ian.
