Downstream compiler location fix. (#1866)

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

* Move logic to determine how to load a downstream shared library to DownstreamCompilerUtil::loadSharedLibrary.
Handle if the given path is directory or a file.

* Improve comment.

* Special handling for a downstream compiler loading SharedLibrary detection - take into account filename may be different.

* Handle cases if path is set but paths can't be determined.

* Fix typo.

Co-authored-by: T. Foley <tfoleyNV@users.noreply.github.com>

5 files changed, 136 insertions, 44 deletions

diff --git a/source/compiler-core/slang-downstream-compiler.cpp b/source/compiler-core/slang-downstream-compiler.cpp
index f5f1b84c9..936c9402a 100644
--- a/source/compiler-core/slang-downstream-compiler.cpp
+++ b/source/compiler-core/slang-downstream-compiler.cpp
@@ -737,6 +737,125 @@ const DownstreamCompiler::Desc& DownstreamCompilerUtil::getCompiledWithDesc()
     outFuncs[int(SLANG_PASS_THROUGH_GLSLANG)] = &GlslangDownstreamCompilerUtil::locateCompilers;
 }
 
+static String _getParentPath(const String& path)
+{
+    // If we can get the canonical path, we'll do that before getting the parent
+    String canonicalPath;
+    if (SLANG_SUCCEEDED(Path::getCanonical(path, canonicalPath)))
+    {
+        return Path::getParentDirectory(canonicalPath);
+    }
+    else
+    {
+        return Path::getParentDirectory(path);
+    }
+}
+
+static SlangResult _findPaths(const String& path, const char* libraryName, String& outParentPath, String& outLibraryPath)
+{
+    // Try to determine what the path is by looking up the path type
+    SlangPathType pathType;
+    if (SLANG_SUCCEEDED(Path::getPathType(path, &pathType)))
+    {
+        if (pathType == SLANG_PATH_TYPE_DIRECTORY)
+        {
+            outParentPath = path;
+            outLibraryPath = Path::combine(outParentPath, libraryName);
+        }
+        else
+        {
+            SLANG_ASSERT(pathType == SLANG_PATH_TYPE_FILE);
+
+            outParentPath = _getParentPath(path);
+            outLibraryPath = path;
+        }
+
+        return SLANG_OK;
+    }
+
+    // If this failed the path could be to a shared library, but we may need to convert to the shared library filename first
+    const String sharedLibraryFilePath = SharedLibrary::calcPlatformPath(path.getUnownedSlice());
+    if (SLANG_SUCCEEDED(Path::getPathType(sharedLibraryFilePath, &pathType)) && pathType == SLANG_PATH_TYPE_FILE)
+    {
+        // We pass in the shared library path, as canonical paths can sometimes only apply to pre-existing objects. 
+        outParentPath = _getParentPath(sharedLibraryFilePath);
+        // The original path should work as is for the SharedLibrary load. Notably we don't use the sharedLibraryFilePath
+        // as this is the wrong name to do a SharedLibrary load with.
+        outLibraryPath = path;
+
+        return SLANG_OK;
+    }
+
+    return SLANG_FAIL;
+}
+
+/* static */SlangResult DownstreamCompilerUtil::loadSharedLibrary(const String& path, ISlangSharedLibraryLoader* loader, const char*const* dependentNames, const char* inLibraryName, ComPtr<ISlangSharedLibrary>& outSharedLib)
+{
+    String parentPath;
+    String libraryPath;
+
+    // If a path is passed in lets, try and determine what kind of path it is.
+    if (path.getLength())
+    {
+        if (SLANG_FAILED(_findPaths(path, inLibraryName, parentPath, libraryPath)))
+        {
+            // We have a few scenarios here.
+            // 1) The path could be the shared library/dll filename, that will be found through some operating system mechanism
+            // 2) That the shared library is *NOT* on the filesystem directly (the loader does something different)
+            // 3) Permissions or some other mechanism stops the lookup from working
+
+            // We should probably assume that the path means something, else why set it.
+            // It's probably less likely that it is a directory that we can't detect - as if it's a directory as part of an app
+            // it's permissions should allow detection, or be made to allow it.
+
+            // All this being the case we should probably assume that it is the shared library name.
+            libraryPath = path;
+
+            // Attempt to get a parent. If there isn't one this will be empty, which will mean it will be ignored, which is probably
+            // what we want if path is just a shared library name
+            parentPath = Path::getParentDirectory(libraryPath);
+        }
+    }
+
+    // Keep all dependent libs in scope, before we load the library we want
+    List<ComPtr<ISlangSharedLibrary>> dependentLibs;
+
+    // Try to load any dependent libs from the parent path
+    if (dependentNames)
+    {
+        for (const char*const* cur = dependentNames; *cur; ++cur)
+        {
+            const char* dependentName = *cur;
+            ComPtr<ISlangSharedLibrary> lib;
+            if (parentPath.getLength())
+            {
+                String dependentPath = Path::combine(parentPath, dependentName);
+                loader->loadSharedLibrary(dependentPath.getBuffer(), lib.writeRef());
+            }
+            else
+            {
+                loader->loadSharedLibrary(dependentName, lib.writeRef());
+            }
+
+            if (lib)
+            {
+                dependentLibs.add(lib);
+            }
+        }
+    }
+
+    if (libraryPath.getLength())
+    {
+        // If we hare a library path use that
+        return loader->loadSharedLibrary(libraryPath.getBuffer(), outSharedLib.writeRef());
+    }
+    else
+    {
+        // Else just use the name that was passed in.
+        return loader->loadSharedLibrary(inLibraryName, outSharedLib.writeRef());
+    }
+}
+
 /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! DownstreamCompilerSet !!!!!!!!!!!!!!!!!!!!!!*/
 
 void DownstreamCompilerSet::getCompilerDescs(List<DownstreamCompiler::Desc>& outCompilerDescs) const
@@ -813,4 +932,6 @@ void DownstreamCompilerSet::addCompiler(DownstreamCompiler* compiler)
     }
 }
 
+
+
 }
diff --git a/source/compiler-core/slang-downstream-compiler.h b/source/compiler-core/slang-downstream-compiler.h
index 53b378eb0..861960f10 100644
--- a/source/compiler-core/slang-downstream-compiler.h
+++ b/source/compiler-core/slang-downstream-compiler.h
@@ -512,6 +512,14 @@ struct DownstreamCompilerUtil: public DownstreamCompilerBaseUtil
     static void updateDefaults(DownstreamCompilerSet* set);
 
     static void setDefaultLocators(DownstreamCompilerLocatorFunc outFuncs[int(SLANG_PASS_THROUGH_COUNT_OF)]);
+
+        /// Attempts to determine what 'path' is and load appropriately. Is it a path to a shared library? Is it a directory holding the libraries?
+        /// Some downstream shared libraries need other shared libraries to be loaded before the main shared library, such that they are in the same directory
+        /// otherwise the shared library could come from some unwanted location.
+        /// dependentNames names shared libraries which should be attempted to be loaded in the path of the main shared library.
+        /// The list is optional (nullptr can be passed in), and the list is terminated by nullptr.
+    static SlangResult loadSharedLibrary(const String& path, ISlangSharedLibraryLoader* loader, const char*const* dependantNames, const char* libraryName, ComPtr<ISlangSharedLibrary>& outSharedLib);
+
 };
 
 }
diff --git a/source/compiler-core/slang-dxc-compiler.cpp b/source/compiler-core/slang-dxc-compiler.cpp
index 0b46bd09e..f5db290b1 100644
--- a/source/compiler-core/slang-dxc-compiler.cpp
+++ b/source/compiler-core/slang-dxc-compiler.cpp
@@ -397,30 +397,9 @@ SlangResult DXCDownstreamCompiler::disassemble(SlangCompileTarget sourceBlobTarg
 {
     ComPtr<ISlangSharedLibrary> library;
 
-    // If the user supplies a path to their preferred version of DXC
-    // we just use this.
-    if (path.getLength() != 0)
-    {
-        // We *assume* path is the path to d3dcompiler.
-        ComPtr<ISlangSharedLibrary> dxil;
-
-        // Attempt to load dxil from same path that d3dcompiler is located
-        const String parentPath = Path::getParentDirectory(path);
-        if (parentPath.getLength())
-        {
-            String dxilPath = Path::combine(parentPath, "dxil");
-            // Try to load dxil along this path first
-            // If it fails - then DXC may load from a different place and thats ok.
-            loader->loadSharedLibrary(dxilPath.getBuffer(), dxil.writeRef());
-        }
-
-        SLANG_RETURN_ON_FAIL(loader->loadSharedLibrary(path.getBuffer(), library.writeRef()));
-    }
-    else
-    {
-        SLANG_RETURN_ON_FAIL(loader->loadSharedLibrary("dxcompiler", library.writeRef()));
-    }
-     
+    const char* dependentNames[] = {"dxil", nullptr } ;
+    SLANG_RETURN_ON_FAIL(DownstreamCompilerUtil::loadSharedLibrary(path, loader, dependentNames, "dxcompiler", library));
+ 
     SLANG_ASSERT(library);
     if (!library)
     {
diff --git a/source/compiler-core/slang-fxc-compiler.cpp b/source/compiler-core/slang-fxc-compiler.cpp
index b0a117d1a..62113a629 100644
--- a/source/compiler-core/slang-fxc-compiler.cpp
+++ b/source/compiler-core/slang-fxc-compiler.cpp
@@ -326,16 +326,7 @@ SlangResult FXCDownstreamCompiler::disassemble(SlangCompileTarget sourceBlobTarg
 {
     ComPtr<ISlangSharedLibrary> library;
 
-    // If the user supplies a path to their preferred version of FXC,
-    // we just use this.
-    if (path.getLength() != 0)
-    {
-        SLANG_RETURN_ON_FAIL(loader->loadSharedLibrary(path.getBuffer(), library.writeRef()));
-    }
-    else
-    {
-        SLANG_RETURN_ON_FAIL(loader->loadSharedLibrary("d3dcompiler_47", library.writeRef()));
-    }
+    SLANG_RETURN_ON_FAIL(DownstreamCompilerUtil::loadSharedLibrary(path, loader, nullptr, "d3dcompiler_47", library));
 
     SLANG_ASSERT(library);
     if (!library)
diff --git a/source/compiler-core/slang-glslang-compiler.cpp b/source/compiler-core/slang-glslang-compiler.cpp
index e754cbdbc..e893ef0ea 100644
--- a/source/compiler-core/slang-glslang-compiler.cpp
+++ b/source/compiler-core/slang-glslang-compiler.cpp
@@ -256,20 +256,13 @@ SlangResult GlslangDownstreamCompiler::disassemble(SlangCompileTarget sourceBlob
     // On unix systems we need to ensure pthread is loaded first.
     // TODO(JS):
     // There is an argument that this should be performed through the loader....
+    // NOTE! We don't currently load through a dependent library, as it is *assumed* something as core as 'ptheads'
+    // isn't going to be distributed with the shader compiler. 
     ComPtr<ISlangSharedLibrary> pthreadLibrary;
     DefaultSharedLibraryLoader::load(loader, path, "pthread", pthreadLibrary.writeRef());
 #endif
 
-    // If the user supplies a path to their preferred version of FXC,
-    // we just use this.
-    if (path.getLength() != 0)
-    {
-        SLANG_RETURN_ON_FAIL(loader->loadSharedLibrary(path.getBuffer(), library.writeRef()));
-    }
-    else
-    {
-        SLANG_RETURN_ON_FAIL(loader->loadSharedLibrary("slang-glslang", library.writeRef()));
-    }
+    SLANG_RETURN_ON_FAIL(DownstreamCompilerUtil::loadSharedLibrary(path, loader, nullptr, "slang-glslang", library));
 
     SLANG_ASSERT(library);
     if (!library)