Bug 343437 - Add support for UNC include directories
Summary: Add support for UNC include directories
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-core (show other bugs)
Version: 8.0   Edit
Hardware: PC Mac OS X - Carbon (unsup.)
: P3 enhancement (vote)
Target Milestone: 8.0   Edit
Assignee: Markus Schorn CLA
QA Contact: Doug Schaefer CLA
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks: 346883 349204
  Show dependency tree
 
Reported: 2011-04-20 12:48 EDT by Greg Watson CLA
Modified: 2011-08-09 09:36 EDT (History)
3 users (show)

See Also:


Attachments
Add UNC include directories (27.73 KB, patch)
2011-04-20 12:48 EDT, Greg Watson CLA
no flags Details | Diff
Revised patch to add UNC include directories (29.14 KB, patch)
2011-04-22 15:17 EDT, Greg Watson CLA
mschorn.eclipse: iplog+
Details | Diff
suggested changes (37.65 KB, patch)
2011-04-27 07:08 EDT, Markus Schorn CLA
mschorn.eclipse: iplog-
Details | Diff
updated UNCPathConverterImpl (3.48 KB, patch)
2011-04-28 08:50 EDT, Greg Watson CLA
mschorn.eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Greg Watson CLA 2011-04-20 12:48:58 EDT
Created attachment 193721 [details]
Add UNC include directories

We would like to add support for include directories that conform to the UNC format (//host/path). This would allow us to support remote environments (where the system include files are located on a remote machine) with minimal disruption to CDT core. 

While URI's would be the ideal way to implement this, CDT uses strings and IPath objects interchangeably, so moving to URI's would require significant changes to the core CDT code. Since IPath already supports UNC paths, this approach leaves the majority of code unchanged.

The use of UNC's does require an additional extension point that enables the UNC to be translated into a valid URI that is understood by the EFS system. An extension is only invoked if the path is in UNC format, otherwise the default behavior is preserved.

A patch with the required changes is attached. I realize this is late in the 8.0 release cycle, but would appreciate it if you could consider this for 8.0.

Thanks!
Comment 1 Markus Schorn CLA 2011-04-22 05:49:48 EDT
Thanks Greg, I have reviewed the patch and see basically one issue, which is the misuse of the 'full path' of an IIndexFileLocation. This full path, always denotes a full-path as returned by IResource.getFullPath(). See the see the documentation of IIndexFileLocation.getFullPath(). Therefore the following parts of the patch need to be changed.

IndexLocationFactory.getIFLExpensive(...): 
The treatment of an UNC path should not differ from another absolute path, i.e. you need to search for a workspace file and create a workspace IFL or external IFL depending on the result of this search.

InternalParserUtil.createFileContent(...):
When the full-path is set, the resource can be found via the resources plugin independent of whether the location for the file is an UNC path or not.
The UNC specific treatment has do be done dependent on the URI. I guess you can do the same check as in TranslationUnit.getLocation().

ProjectIndexerInputAdapter.getASTPath(...):
The full path of the IFL must not be misused for storing a location. The UNC specific treatment should be done within IndexLocationFactory.getAbsoluteFile(ifl).
Comment 2 Greg Watson CLA 2011-04-22 15:16:25 EDT
Hi Markus, thanks for taking the time to look at this. I've attached a new patch that (I hope) addresses all your issues. Please let me know if there is anything else you would like me to do.
Comment 3 Greg Watson CLA 2011-04-22 15:17:15 EDT
Created attachment 193941 [details]
Revised patch to add UNC include directories
Comment 4 Markus Schorn CLA 2011-04-27 07:08:36 EDT
Created attachment 194141 [details]
suggested changes

Hi Greg,
I suggest the following changes (see my patch):
* I have separated the UNCPathConverter that can be extended by clients from 
  the special implementation that combines the contributed converters.
* I have made sure that the combined path converter is thread-safe.
* isUNC() is potentially called very ofter, I have replace the implementation 
  to simply check for leading // or \\, rather than creating a Path object.
* The FileCache needs an extra null-ptr check (in the case where the cache is
  by-passed.
* Added since-tag, externalized strings.

Please check, whether the modified patch works for you.
Comment 5 Greg Watson CLA 2011-04-28 08:49:06 EDT
Hi Marcus, your changes look fine, however I did need to change UNCPathConverterImpl slightly to avoid a class cast exception. See attached patch. 

Otherise it looks great. Thanks!
Comment 6 Greg Watson CLA 2011-04-28 08:50:04 EDT
Created attachment 194263 [details]
updated UNCPathConverterImpl
Comment 7 Markus Schorn CLA 2011-04-28 10:11:35 EDT
Fixed in 8.0 > 20110428.
Comment 8 CDT Genie CLA 2011-04-28 10:23:02 EDT
*** cdt cvs genie on behalf of mschorn ***
Bug 343437: Support for UNC include directories, by  Greg Watson.

[*] EditorUtility.java 1.73 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/util/EditorUtility.java?root=Tools_Project&r1=1.72&r2=1.73

[*] OpenIncludeAction.java 1.37 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/OpenIncludeAction.java?root=Tools_Project&r1=1.36&r2=1.37

[*] plugin.properties 1.60 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/plugin.properties?root=Tools_Project&r1=1.59&r2=1.60
[*] plugin.xml 1.155 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/plugin.xml?root=Tools_Project&r1=1.154&r2=1.155

[*] IndexLocationFactory.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/index/IndexLocationFactory.java?root=Tools_Project&r1=1.15&r2=1.16

[+] UNCPathConverterImpl.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/utils/org/eclipse/cdt/internal/core/UNCPathConverterImpl.java?root=Tools_Project&revision=1.1&view=markup

[+] UNCPathConverter.exsd  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/schema/UNCPathConverter.exsd?root=Tools_Project&revision=1.1&view=markup

[*] ScannerUtility.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/scanner/ScannerUtility.java?root=Tools_Project&r1=1.18&r2=1.19

[*] IncludeReference.java 1.25 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/IncludeReference.java?root=Tools_Project&r1=1.24&r2=1.25
[*] TranslationUnit.java 1.119 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/TranslationUnit.java?root=Tools_Project&r1=1.118&r2=1.119

[*] InternalParserUtil.java 1.23 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/InternalParserUtil.java?root=Tools_Project&r1=1.22&r2=1.23

[*] StandaloneIndexerInputAdapter.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/indexer/StandaloneIndexerInputAdapter.java?root=Tools_Project&r1=1.15&r2=1.16

[*] ProjectIndexerInputAdapter.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/indexer/ProjectIndexerInputAdapter.java?root=Tools_Project&r1=1.18&r2=1.19
[*] FileExistsCache.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/indexer/FileExistsCache.java?root=Tools_Project&r1=1.6&r2=1.7

[+] UNCPathConverter.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/UNCPathConverter.java?root=Tools_Project&revision=1.1&view=markup