Summary: | [Regression] ExternalLibraryCache causes false classpath problem markers. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] PDE | Reporter: | Carlin Rogers <carlin.rogers> | ||||||
Component: | UI | Assignee: | Curtis Windatt <curtis.windatt.public> | ||||||
Status: | RESOLVED FIXED | QA Contact: | |||||||
Severity: | normal | ||||||||
Priority: | P3 | CC: | ankur_sharma, curtis.windatt.public, daniel_megert, Michael_Rennie, praby, raghunathan.srinivasan | ||||||
Version: | 3.7 | Keywords: | contributed | ||||||
Target Milestone: | 3.8 M2 | Flags: | daniel_megert:
review+
|
||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Whiteboard: | |||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 354557 | ||||||||
Attachments: |
|
Description
Carlin Rogers
2011-06-01 20:39:21 EDT
Created attachment 197193 [details]
patch - checks if the ZipEntry is a directory.
I think this patch should fix the issue.
> Seems like ExternalLibraryCache.getExtractedLibraries() should check if the
> ZipEntry is not a directory (at least until there's support for directory
> entries in the bundle classpath).
The proposed change is in extractJar(), called by getExtractedLibraries().
I need to do some more investigation, but this is likely a regression from 3.6. As Carlin points out, the fix to bug 147831 does not account for directory entries inside jarred bundles. Consider for RC4 inclusion. I do not know how widespread this problem is. I don't know of any Eclipse bundles that follow this structure (jarred bundle with a directory entry other than '.' on the bundle-classpath). Created attachment 197239 [details]
Bundle to reproduce
1) Change your target platform to be the attached bundle
2) Create a new plug-in project
3) Add the NestedJars bundle to your dependencies
4) (Optional) import class A (from the nested folder) and B (from the nested jar) into your own class.
The error will show up in the log whenever the classpath is changed.
How often will you be encountering this Carlin? The code from both the nested jar and the nested folder is available on the classpath. The only consequence of this bug appears to be the logged error. I'm leaning towards pushing this to 3.7.1. Carlin please tell ASAP, how hard/important this hits you - we're close of shutting down 3.7 development. Curtis, does the proposed fix makes sense? If so, please provide a detailed risk assessment of the fix, including performance implications so that we (I ;-) can make an educated decision. If not, please provide another fix with assessment. +1 for 3.7.1, since the classpath is actually correct and the code will still compile. I'm not convinced this needs to go into RC4, unless you have a better assessment of the impact as Dani requested. (In reply to comment #6) > Curtis, does the proposed fix makes sense? If so, please provide a detailed > risk assessment of the fix, including performance implications so that we (I > ;-) can make an educated decision. If not, please provide another fix with > assessment. Yes the proposed patch makes sense. The only problem is that if you have are using the same workspace you reproduced it on the patch will not immediately fix your problem. This is because PDE caches the exploded jar entries in our metadata. Deleting the .external_libraries folder in PDE metadata or starting from a clean workspace will demonstrate that the patch fixes the problem. This is very low risk and has no impact on performance. Thanks for the quick response Curtis. I think we would be OK with a 3.7.1 release. We can work around the issue. We work on an adopter product that includes a plugin with a directory in the bundle classpath so we see this in our development environment. For us the error is more than just a log entry. The classpath problem marker means that compilation does not continue on the plugin that is dependent on the other plugin in question. To work around it we need to import the other plugin as well into the dev workspace. >Thanks for the quick response Curtis. I think we would be OK with a 3.7.1
>release. We can work around the issue.
Thanks for the update. +1 for 3.7.1.
Cloned as 354557 for 3.7.1 inclusion Fixed in HEAD. |