Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 335298 - IncludePaths are canonicalised on creation
Summary: IncludePaths are canonicalised on creation
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-core (show other bugs)
Version: 8.0   Edit
Hardware: PC Linux-GTK
: P3 normal (vote)
Target Milestone: 8.0   Edit
Assignee: James Blackburn CLA
QA Contact: Doug Schaefer CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-25 06:44 EST by James Blackburn CLA
Modified: 2011-01-25 11:23 EST (History)
2 users (show)

See Also:

Attachments
patch 1 (2.25 KB, patch)
2011-01-25 06:55 EST, James Blackburn CLA 		jamesblackburn+eclipse: iplog-
jamesblackburn+eclipse: review?
Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.