Bugzilla – Full Text Bug Listing
| Summary: | Error for API breakage when there is none (compare plug-in split) | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Michael Valenta <Michael.Valenta> | ||||||||||||
| Component: | API Tools | Assignee: | Olivier Thomann <Olivier_Thomann> | ||||||||||||
| Status: | RESOLVED DUPLICATE | QA Contact: | |||||||||||||
| Severity: | normal | ||||||||||||||
| Priority: | P3 | CC: | daniel_megert, darin.eclipse, markus.kell.r, Michael_Rennie, Olivier_Thomann, Szymon.Brandys, tomasz.zarna | ||||||||||||
| Version: | 3.4 | ||||||||||||||
| Target Milestone: | 3.5 M5 | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Windows XP | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 243136 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Michael Valenta
What happens if you restart Eclipse ? I could not reproduce so far. (In reply to comment #1) > What happens if you restart Eclipse ? In my case the error is still there. Doing Clean doesn't help either. I see the error too. As Mike said, some API was moved from org.eclipse.core and maybe API tooling doesn't see that the required plug-in (org.eclipse.compare.core) contains it now. I disabled the baseline (i.e. unchecked on the preference page) to get ride of the error. I then restarted and the error came back. I would have thought that the checked state of the baseline would be persisted. The problem should come from the workspace profile since it is reporting missing types. Michael, if you could provide your workpace (ideally a small one containing just the two compare projects), I could try to debug it. Ok, I know why I could not reproduce it. The setting for reporting missing type was set to ignore. Now I can easily reproduce it. The problem comes from the fact that we don't consider reexported bundles types as part of the types for the bundle we are checking. I think we should inject the class file containers from the reexported bundles inside the classfile containers of the original bundle. Then when searching for the type we could find it inside the reexported bundle containers. This also means that we should look for the api description inside the reexported bundle instead of inside the root bundle. I have a patch for the class file containers, but now I failed with errors saying that the same types are no longer API because I don't retrieve the right api description so the type is not seen as an API type. Created attachment 107666 [details]
Proposed fix
This should fix the class file containers part as well as the findClassFile with an origin.
(In reply to comment #8) > This should fix the class file containers part as well as the findClassFile > with an origin. Yes. It works. I use the patched pde plug-in within my Eclipse. While doing some changes in org.eclipse.compare plug-ins, the problem reoccurred again. So I'm taking back what I said in comment 9. With the patch the errors are slightly different. You need a fix for the api description part otherwise the moved types are reported as not being API anymore. playing around with the description side of things, I am not sure this will be as trivial a fix as I thought. I think the problem in this case is that the re-exported bundle has a package with the same name as one in the host bundle. So when we try to inject the package fragments from the re-exported bundle they are in essence tossed out since we already have a package fragment with the same name (and we currently do not have support for scoping package fragment names). A good place to start for a solution would be to emulate how we handle fragments, but instead have a 'reexported' section (or something similar) Perhaps we should iterate over API packages (exported packages) from each API component when doing the comparison, rather than iterating over class file containers. This way, it does not matter where the class file comes from, only if it is considered API for the component. This might also work better for fragment/host relationships. (In reply to comment #13) > Perhaps we should iterate over API packages (exported packages) from each API > component when doing the comparison, rather than iterating over class file > containers. This way, it does not matter where the class file comes from, only > if it is considered API for the component. The problem is that this would work only when getting API visibility deltas. We still need to iterate all types in order to get all the deltas. Consider the following scenario: Bundle XYZ version 1.0 contains these types: a.b.c.X a.b.c.Y d.e.f.M d.e.f.N Bundle XYZ gets split into two bundles: Bundle XYZ.core (version 1.0) a.b.c.X Bundle XYZ (version 1.1); re-exports XYZ.core a.b.c.Y d.e.f.M d.e.f.N When comparing version 1.1 of XYZ to version 1.0, we currently find the type a.b.c.X to be removed from XYZ (a breaking change). However, from an API view the type is still visible since it is re-exported from XYZ.core. The delta engine needs to enhanced to check for the type in re-exported bundles/packages. It should resolve a.b.c.X in the context of bundle XYZ, check if it comes from a re-exported bundle/package and then perform the compare. Any compatibility issues in the type itself need to be reported against the new bundle XYZ.core. Marking as M5 candidate Created attachment 120515 [details]
patch
Fix with a test. Should create more tests, and perhaps add different kinds of removed type flags to indicate types removed due to being moved but not re-exported.
Created attachment 120519 [details]
update
Updated patch.
Created attachment 120525 [details]
another update
Still needs more work & tests. Not reporting problems on correct component - and still finds problems with newly added restrictions in new API component.
Created attachment 120569 [details]
patch
There is an issue with the way problems are created. When org.eclipse.compare is built, we end up analyzing the files in org.eclipse.compare.core as well. Any problems found in org.eclipse.compare.core do not get created properly, since our builder is building org.eclipse.compare it attempts to locate resources in the org.eclipse.compare project on which to create markers. The resources are not found and the problems are not created. The IApiProblems have a project relative path to their resoruce, but we resolve the path relative to the incorrect project. This will be fixed as part of bug 258882. *** This bug has been marked as a duplicate of bug 258882 *** New api filters must be added for new restrictions: Restrictions have been added for type org.eclipse.compare.patch.IFilePatchResult Restrictions have been added for type org.eclipse.compare.patch.IHunk Restrictions have been added for type org.eclipse.compare.patch.PatchConfiguration (In reply to comment #23) > New api filters must be added for new restrictions: > Restrictions have been added for type > org.eclipse.compare.patch.IFilePatchResult > Restrictions have been added for type org.eclipse.compare.patch.IHunk > Restrictions have been added for type > org.eclipse.compare.patch.PatchConfiguration Done. Dani, thanks for reminding me about this. |