Bugzilla – Full Text Bug Listing
| Summary: | It is possible to import package from fragment, but classes are invisible | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Krzysztof Daniel <krzysztof.daniel> | ||||||||
| Component: | UI | Assignee: | Curtis Windatt <curtis.windatt.public> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | curtis.windatt.public, daniel_megert, krzysztof.daniel | ||||||||
| Version: | 3.7 | Keywords: | contributed | ||||||||
| Target Milestone: | 3.6.2+ | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Krzysztof Daniel
Created attachment 191101 [details]
Fix proposition
Here is a log of what packages and fragments are not allowed to be in the package selection dialog (fragments without parent with extensible API): fragment org.eclipse.core.filesystem.win32.x86 has no parent with extensible API fragment org.eclipse.core.net.win32.x86 has no parent with extensible API fragment org.eclipse.core.resources.win32.x86 has no parent with extensible API fragment org.eclipse.core.runtime.compatibility.registry org.eclipse.equinox.registry has extensible API fragment org.eclipse.ecf.provider.filetransfer.httpclient.ssl has no parent with extensible API fragment org.eclipse.ecf.provider.filetransfer.ssl has no parent with extensible API fragment org.eclipse.ecf.ssl has no parent with extensible API fragment org.eclipse.equinox.launcher.win32.win32.x86 has no parent with extensible API fragment org.eclipse.equinox.security.win32.x86 has no parent with extensible API fragment org.eclipse.jdt.compiler.apt org.eclipse.jdt.core has extensible API fragment org.eclipse.jdt.compiler.tool org.eclipse.jdt.core has extensible API fragment org.eclipse.swt.win32.win32.x86 org.eclipse.swt has extensible API fragment org.eclipse.ui.win32 has no parent with extensible API fragment org.eclipse.ui.workbench.compatibility has no parent with extensible API fragment org.eclipse.update.core.win32 has no parent with extensible API fragment Fragment has no parent with extensible API fragment org.eclipse.core.filesystem.win32.x86 has no parent with extensible API fragment org.eclipse.core.net.win32.x86 has no parent with extensible API fragment org.eclipse.core.resources.win32.x86 has no parent with extensible API fragment org.eclipse.core.runtime.compatibility.registry org.eclipse.equinox.registry has extensible API fragment org.eclipse.ecf.provider.filetransfer.httpclient.ssl has no parent with extensible API fragment org.eclipse.ecf.provider.filetransfer.ssl has no parent with extensible API fragment org.eclipse.ecf.ssl has no parent with extensible API fragment org.eclipse.equinox.launcher.win32.win32.x86 has no parent with extensible API fragment org.eclipse.equinox.security.win32.x86 has no parent with extensible API fragment org.eclipse.jdt.compiler.apt org.eclipse.jdt.core has extensible API fragment org.eclipse.jdt.compiler.tool org.eclipse.jdt.core has extensible API fragment org.eclipse.swt.win32.win32.x86 org.eclipse.swt has extensible API fragment org.eclipse.ui.win32 has no parent with extensible API fragment org.eclipse.ui.workbench.compatibility has no parent with extensible API fragment org.eclipse.update.core.win32 has no parent with extensible API At first glance the patch looks good, marking for M7. Created attachment 191917 [details]
Proposed Fix
Christopher, please confirm that this fix works correctly for you. It takes the same approach as your patch, just rewords/reorders some of it.
I am not sure about the comment in line 485.
I thought it is a host who has to have an extensible api directive set.
I made a typo in line 581.
It should be written "their packages" not "they packages".
I am a little bit afraid of the inversion you have made in the loop in line 583.
for (int i = 0; i < hosts.length; i++) {
if (ClasspathUtilCore.hasExtensibleAPI(PluginRegistry.findModel(hosts[i])))
return false;
}
is not an equivalent of negated
for (int i = 0; i < hosts.length; i++) {
if (ClasspathUtilCore.hasExtensibleAPI(PluginRegistry.findModel(hosts[i])))
return true;
}
In the latter case we need to find at least one host with an extensibleAPI, the first approach says that all hosts need to have extensibleAPI. I am not sure what is expected here.
The part about the inversion in my last comment is wrong - and the new code is right. The code hides successfully packages from the fragment. I went one step further, and checked how actually Eclipse behaves when the directive is set - and it looks like only the existing packages from the host can be extended, and not those declared only in the fragment. Is this expected behavior? (In reply to comment #6) I forgot to export the package. Everything works as expected. Only comments in the patch require some attention. (In reply to comment #7) > I forgot to export the package. Everything works as expected. Only comments in > the patch require some attention. Which comments would you like to see changed? Created attachment 192251 [details]
Patch with altered comments
Excellent, applied the patch to HEAD. Patch released to 3.6.x stream for backporting. |