Bugzilla – Full Text Bug Listing
| Summary: | Consider packages "exported" via x-friends as API | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Matthias Becker <ma.becker> | ||||
| Component: | API Tools | Assignee: | PDE API Tools Inbox <pde-apitools-inbox> | ||||
| Status: | RESOLVED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | curtis.windatt.public, daniel_megert, markus.kell.r, Michael_Rennie, Vikas.Chandra | ||||
| Version: | 4.5 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X | ||||||
| Whiteboard: | stalebug | ||||||
| Attachments: |
|
||||||
This cannot change the default behaviour of the platform. This will be a significant hole in our test coverage unless you are putting significant effort into writing new tests (perhaps some existing tests could be copied and changed to use x-friends). Committer time is very limited (Vikas is the only person who might have time to review). So if this becomes a large code change there may not be time in 4.5. Dear Curtis, I don't want to change the default behaviour of the API tools - I want to make this behaviour switchable (see treatFriendPackagesAsApi in the patch). And yes, tests for this are missing. I would also provide tests with the patch. Regards, Matthias (In reply to Matthias Becker from comment #2) Where do you want to make it switchable? I guess it has to be on the exporting plug-in. Dear Markus, maybe this is a miss-understanding: I want the make the changed behaviour of the PDE API-Tools (the check that is done in the ANT tasks) switchable (see ANT-Taks parameter treatFriendPackagesAsApi in the patch). I don't want to change anything in the logic how manifests are handled in the "rest of" the PDE tools. Or do I oversee some side-effect to may proposed changes? Regards, Matthias I understand that adding a parameter to the Ant task would solve your immediate problem. But there's more: API Tools can be used via Ant tasks or as builder in PDE-enabled projects. These two usages should stay functionally equivalent, so that the Eclipse IDE always shows the same results as an automated external build system. Therefore, adding this setting to the Ant task means it needs to be made available in the IDE as well. A global setting cannot be shared with a project, that's why it should be a project-specific setting. Ok now I got it. I just had a look in to code. There are already some project specific settings for the API tools. Couldn't we simply add an option to the Project specific properties page that can be reached via "Plug-in Development" -> "API Errors / Warning" (File /apitools/org.eclipse.pde.api.tools.ui/.settings/org.eclipse.pde.api.tools.prefs)? The "API Erros / Warnings" currently only handles settings how errors are reported (as Error/Warning/Ingored). The setting I am proposing is of another type so it does not fit into this page perfect. We could add an "General" Section above the ApiErrorWarningsConfigurationBlock. We even could introduce a new page - but it seems to be a bit oversized to create a complete new page for only one checkbox. What do you think? How should I proceed? On the API Errors/Warnings property page there is a tab called API compatibility. There is already a checkbox option to report breakage regardless of version increment. Another option here to "Report breaking changes on internal x-friend packages" sounds appropriate. Sounds reasonable. I will follow up on this. Dear all,
I am still working on this topic when I have some spare time.
In the meantime I did a first implementation that I want to sketch here:
I both extended ApiFileGenerationTask.execute() and BundleComponent.annotateExportedPackages() to treat internal packages that have x-friends just as it would be in a public package - the actual comparison logic was not changed at all. For sure I made this behaviour switchable via a new property in the ANT task (treatFriendPackagesAsApi) / a new preferences in the IDE ("Report API breakage for internal x-friends packages").
For the generation of an API-Baseline (with the ANT Task) everything is fine but I currently have some issues with the comparison step:
The comparison can:
(a) either be done with the IDE-Integration of the API-Tools
(b) or be done with the APIToolsAnalysisTask.
In both cases the new IDE-preference "Report API breakage for internal x-friends packages" must be considered when creating the ApiDescription (e.g. from instances of BundleComponent/ ProjectComponent).
The call stack to my extended code in the "annotateExportedPackages" method looks like the following:
BundleComponent.annotateExportedPackages()
BundleComponent.initializeApiDescription()
BundleComponent.createLocalApiDescription()
BundleComponent.createApiDescription()
BundleComponent(Component).getApiDescription()
ApiComparator.internalCompare()
ApiComparator.compare()
ApiComparator.compare()
BaseApiAnalyzer.checkCompatibility()
BaseApiAnalyzer.analyzeComponent()
ApiAnalysisBuilder.buildAll()
I now have the issue that the code in annotateExportedPackages has to know if this new feature is switched on or off. I cannot access the eclipse preference store directly there, because this is not available in the ANT-Task use case.
I am now wondering how to hand over this setting. I considered of adding a member to the BundleComponent class that carries the information if the feature is on or off. This member could be filled with an appropriate setter and be evaluated in the createApiDescription method.
What do you think? Some advice from you side would be great.
Kind regards,
Matthias
I would look at BaseAPIAnalyzer where we check whether we are running with an Eclipse instance or not before checking the preference setting. org.eclipse.pde.api.tools.internal.builder.BaseApiAnalyzer.ignoreMajorVersionCheckWithoutBreakingChange() 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. Please reopen if the problem still persists. |
Created attachment 249462 [details] draft patch Our team has the following situation: We have two products (set of plugins) that share some common functionality. It should be possible to install these products into the same eclipse installation. We have put the common functionality into a set of plugins. These plugins have there own common development / release cycle. The two products consume these plugins (binary). As there is still some movement in the common plugins we don't want to expose the functionality to the complete outside by completely exporting the relevant packages of the common plugins. We only want to expose the functionality two our (own) two products. We have chosen to do this by "exporting" the packages in the common plugin via x-friends directives. The consuming pluigns do then something like Require-Bundle: com.acme.foo.bar;bundle-version="[x.yy.zz,x+1.0.0)", in their manifests. With this we don't have an "official" API but be have an "internal" API. But we have to keep this (internal) API binary compatible if we don't want to break a shared installation of both products into a common eclipse installation. So we would like to use the API Tools in our central build to check if changes in the shared plugins would break the (internal) API. As the API tools today consider packages that are "exported" via x-friends as internal / not-API this is currently not possible - at least I found no way to use the API tools in our use-case. I debugged through the API tools. To me it seems that our use-case could be supported with relatively few changes to the code. My idea is to provide a setting / switch that allows the user to tell the API tools to consider packages that have x-friends as API. Would you we willing to accept such a patch? I simply attach my first draft of that patch to this bug. This change is for sure not finished it should simply function as a sketch. Regards, Matthias