From d1e740d8b25e03a734093bd84b792eaf969649d1 Mon Sep 17 00:00:00 2001
From: jsmall-nvidia <jsmall@nvidia.com>
Date: Thu, 6 Oct 2022 10:27:50 -0400
Subject: Improvements around absolute paths and file systems (#2433)

* #include an absolute path didn't work - because paths were taken to always be relative.

* Add handling for root paths.

* Fixes around absolute paths.

* Add SimplifyStyle

* Remove unrequire include.

* Fix some details around RelativeFileSystem canonical paths.

* For MemoryFileSystem make sure "/a" and "a" maps to same canonical path.

* Add test for canonicalPath.

* Improve comment.

* More testing around canonical paths.
---
 source/core/slang-file-system.cpp | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

(limited to 'source/core/slang-file-system.cpp')

diff --git a/source/core/slang-file-system.cpp b/source/core/slang-file-system.cpp
index 0697810f4..0369d0a02 100644
--- a/source/core/slang-file-system.cpp
+++ b/source/core/slang-file-system.cpp
@@ -871,19 +871,35 @@ SlangResult RelativeFileSystem::_calcCombinedPathInner(SlangPathType fromPathTyp
     }
 }
 
-SlangResult RelativeFileSystem::_getCanonicalPath(const char* path, StringBuilder& outPath)
+SlangResult RelativeFileSystem::_getCanonicalPath(const char* path, String& outPath)
 {
     if (m_stripPath)
     {
         // We are just using the filename. There is no path that could go outside of the the relative path so we can use as is
-        auto fileName = Path::getFileName(path);
-
-        outPath.swapWith(fileName);
+        outPath = Path::getFileName(path);
     }
     else
     {
+        // NOTE that we don't want the canonical path to be absolute with a leading "/"
+        // because paths specified which aren't absolute, would produce a different path.
+        // 
+        // Ie we want (and get with these options)
+        // "a" -> "a"
+        // "/a" -> "a".
+        //
+        // If we allowed the root to be included then...
+        // "a" -> "a"
+        // "/a" -> "/a"
+        // 
+        // Two identical paths would match to different paths, which wouldn't be canonical.
+        // 
+        // This could be fixed by making all paths absolute with '/' too, but it's easier to just make all not 
+        // have "/"
+
+        StringBuilder canonicalPath;
         // We want the input path to be local to this file system
-        SLANG_RETURN_ON_FAIL(Path::simplifyAbsolute(path, outPath));
+        SLANG_RETURN_ON_FAIL(Path::simplify(path, Path::SimplifyStyle::AbsoluteOnlyAndNoRoot, canonicalPath));
+        outPath = canonicalPath;
     }
     return SLANG_OK;
 }
@@ -892,11 +908,12 @@ SlangResult RelativeFileSystem::_getFixedPath(const char* path, String& outPath)
 {
     ComPtr<ISlangBlob> blob;
 
-    StringBuilder canonicalPath;
+    String canonicalPath;
     SLANG_RETURN_ON_FAIL(_getCanonicalPath(path, canonicalPath));
+  
     SLANG_RETURN_ON_FAIL(_calcCombinedPathInner(SLANG_PATH_TYPE_DIRECTORY, m_relativePath.getBuffer(), canonicalPath.getBuffer(), blob.writeRef()));
-
     outPath = StringUtil::getString(blob);
+    
     return SLANG_OK;
 }
 
@@ -957,8 +974,8 @@ SlangResult RelativeFileSystem::getPath(PathKind kind, const char* path, ISlangB
         }
         case PathKind::Canonical:
         {   
-            StringBuilder canonicalPath;
-            SLANG_RETURN_ON_FAIL(_getCanonicalPath(path, canonicalPath));
+            String canonicalPath;
+            SLANG_RETURN_ON_FAIL(_getCanonicalPath(path, canonicalPath));            
             *outPath = StringBlob::moveCreate(canonicalPath).detach();
             return SLANG_OK;
         }
-- 
cgit v1.2.3