Community
Participate
Working Groups
IncludeEntry sometimes stores include paths in a slightly different case than the paths the indexer constructs. (Why? I didn't find out yet.) Due to some parts of Eclipse's Path implementation being case-sensitive on all platforms -- esp. in #isPrefixOf() -- IncludeReference#isOnIncludeEntry() fails to detect that a file is on the include paths. So the indexer could have totally complete and correct data, but CModelManager#createTranslationUnitFrom() would fail to create a TU for the file. (There seem to be other problems with this method due to the lack of a fallback or any notification when trying to construct a TU for a non-includeable file.) It's simpler not to touch Path or uses of #isPrefixOf(), so a solution is to store a canonical path in IncludeEntry and canonicalize the input path on entry to IncludeReference#isOnIncludeEntry(). My quick'n'dirty "canonicalization" just lowercases paths when on the Win32 platform, though going through java.io.File#getCanonicalFile() would be best (and would preserve the case displayed in the UI), though with an unknown performance hit. I didn't search for other places where this might be a problem. Notable changes below (a patch will follow later): IncludeEntry.java: public IncludeEntry(IPath resourcePath, IPath basePath, IPath baseRef, IPath includePath, boolean isSystemInclude, IPath[] exclusionPatterns, boolean isExported) { super(IPathEntry.CDT_INCLUDE, basePath, baseRef, resourcePath, exclusionPatterns, isExported); this.includePath = (includePath == null) ? Path.EMPTY : Util.getCanonicalPath(includePath); this.isSystemInclude = isSystemInclude; } ... IncludeReference.java: /* (non-Javadoc) * @see org.eclipse.cdt.core.model.IIncludeReference#isOnIncludeEntry(org.eclipse.core.runtime.IPath) */ public boolean isOnIncludeEntry(IPath path) { path = Util.getCanonicalPath(path); if (fIncludeEntry.getIncludePath().isPrefixOf(path) && !CoreModelUtil.isExcluded(path, fIncludeEntry.fullExclusionPatternChars())) { return true; } return false; } ... Util.java: private static boolean hasCaseInsensitiveFileSystem; static { hasCaseInsensitiveFileSystem = Platform.getOS().equals(Platform.OS_WIN32); } /** * Get a canonical IPath, for use in IPath#isPrefixOf, #equals, etc. * to overcome a bug in Path that doesn't account for * case-insensitive filesystems. */ public static IPath getCanonicalPath(IPath path) { if (hasCaseInsensitiveFileSystem) { // be stupid for now (instead of using File#toCanonicalFile()) return new Path(path.toOSString().toLowerCase()); } return path; }
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.
