Summary: | failure to open declaration/definition due to case-sensitive path checks on Win32 | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Tools] CDT | Reporter: | Ed Swartz <ed.swartz> | ||||||||||||||
Component: | cdt-core | Assignee: | Doug Schaefer <cdtdoug> | ||||||||||||||
Status: | RESOLVED FIXED | QA Contact: | |||||||||||||||
Severity: | normal | ||||||||||||||||
Priority: | P3 | CC: | acharnp, andrew.ferguson, norbert.ploett | ||||||||||||||
Version: | 3.1.1 | Keywords: | contributed | ||||||||||||||
Target Milestone: | 4.0 M5 | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | Windows XP | ||||||||||||||||
Whiteboard: | |||||||||||||||||
Attachments: |
|
Description
Ed Swartz
2006-09-21 14:20:55 EDT
Created attachment 50656 [details]
patch described in summary
This is a testing patch, most likely needs changes.
looking at this patch, I'm wondering if your comment about the problem being in Path.isPrefix is a better place to fix this i.e. by providing a PathUtil.isPrefixIgnoreCase for comparing paths on windows machines. Particularly for isOnIncludeEntry it makes more sense to me to have a case-insensitive comparison than canonicalize both paths before comparing (especially if the canonicalisation routine invokes I/O in the future). since isPrefix is used in many places I'd guess there are more than a few failure routes - do you have a testcase that covers the bugzilla? (In reply to comment #2) > looking at this patch, I'm wondering if your comment about the problem being in > Path.isPrefix is a better place to fix this i.e. by providing a > PathUtil.isPrefixIgnoreCase for comparing paths on windows machines. Yes, that would be a better place. > Particularly for isOnIncludeEntry it makes more sense to me to have a > case-insensitive comparison than canonicalize both paths before comparing > (especially if the canonicalisation routine invokes I/O in the future). Is IPath guaranteed to handle other aspects of canonicalization, like removing ".."? If so, then yes, it looks like case-insensitive comparison suffices. > since isPrefix is used in many places I'd guess there are more than a few > failure routes - do you have a testcase that covers the bugzilla? The only testcase I know of is (1) make a standard make project, (2) edit C/C++ Include Paths and Symbols and enter an incorrectly capitalized path, and (3) open a source file from that project and invoke Open Declaration for a symbol found in an external file. Since the include path is not canonical, then IncludeEntry#isOnIncludePath() fails, and the fallback logic is to invoke a generic text editor. (Due to other fixes for 158193, this no longer causes a CoreException.) I guess the correct fix is to ensure the paths entered into the C/C++ Include Paths and Symbols dialog are canonical (or maybe at a lower level in CModel?). Patch attached. Created attachment 55044 [details]
patch to canonicalize include paths from C/C++ Include Paths and Symbols UI
The last patch (for the UI) is not sufficient. Projects upgraded from older releases can still have miscapitalized entries. Also, other lookups of files (e.g. from the OpenIncludeAction) can have non-canonical capitalization. So I think it's a good idea still to canonicalize paths whenever they are looked up. I will attach a patch that addresses the known issues I've encountered so far in CDT 4.0. Still no tests :( Created attachment 57829 [details]
4.0 patches for org.eclipse.cdt.internal.code.model
Created attachment 57831 [details]
4.0 patch to org.eclipse.cdt.internal.ui.editor to ensure full paths and miscapitalized #includes can be looked up
Created attachment 57832 [details]
4.0 patch to org.eclipse.cdt.utils.PathUtil to fix canonicalization not to destroy relative paths or to add device unexpectedly
Created attachment 58135 [details]
Another patch for OpenIncludeAction to handle miscapitalized names in #include
Patch applied. Thanks, Ed. |