Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 335298

Summary: IncludePaths are canonicalised on creation
Product: [Tools] CDT Reporter: James Blackburn <jamesblackburn+eclipse>
Component: cdt-coreAssignee: James Blackburn <jamesblackburn+eclipse>
Status: RESOLVED FIXED QA Contact: Doug Schaefer <cdtdoug>
Severity: normal    
Priority: P3 CC: andrew.ferguson, ed.swartz
Version: 8.0   
Target Milestone: 8.0   
Hardware: PC   
OS: Linux-GTK   
Whiteboard:
Attachments:
Description Flags
patch 1 jamesblackburn+eclipse: iplog-, jamesblackburn+eclipse: review?

Description James Blackburn CLA 2011-01-25 06:44:03 EST
In the core.model: IncludeEntry; IncludeFileEntry; IncludeReference; MacroFileEntry all have their paths canonicalised in their constructor.
Apparently this is to handle case-sensitivity issues on Windows (bug 158190).

On Linux we use symlinks and see that the canonicalised paths are different from the paths entered (and in fact some managedbuild tests fail as /usr/local/... which is under a symlink here).

There's also the issue that File#getCanonicalPath performs I/O and is likely unnecessarily expensive on platforms other than Windows.

Is there a good reason to canonicalize the path on Linux?
Comment 1 James Blackburn CLA 2011-01-25 06:55:03 EST
Created attachment 187510 [details]
patch 1

Potential fix for the issue: only canonicalise paths on Windows.
Comment 2 James Blackburn CLA 2011-01-25 08:58:33 EST
Tests seem happy having disabled path canonicalization on !WINDOWS.
Comment 3 CDT Genie CLA 2011-01-25 09:23:02 EST
*** cdt cvs genie on behalf of jblackburn ***
Bug 335298 - IncludePaths are canonicalised on creation. Only need to do this on Windows.

[*] PathUtil.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/PathUtil.java?root=Tools_Project&r1=1.11&r2=1.12
Comment 4 Ed Swartz CLA 2011-01-25 09:25:46 EST
I agree, this is only necessary for Windows.  Thanks!
Comment 5 Andrew Gvozdev CLA 2011-01-25 10:25:23 EST
>	public static IPath getCanonicalPath(IPath fullPath) {
>		if (!WINDOWS || !fullPath.isAbsolute())
>			return fullPath;
>		...
> }

Is PathUtil.getCanonicalPath() a right place to do that? You are changing the semantics of the function. Now it does not return canonical path on Linux although its name "getCanonicalPath" implies that. And this in a public API.
Comment 6 James Blackburn CLA 2011-01-25 10:42:46 EST
Fair enough, added a #getCanonicalPathWindows and update the call-sites appropriately.