Handling of temporary files (#2299)

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

* Work around windows issue with temporary file clash.

* Handle the temporary file path actually creates a file.

* Fix typo.

* Fix typo in linux for temporary file.

* Add unit test for io. Tests generateTemporary operation.

5 files changed, 40 insertions, 12 deletions

diff --git a/source/compiler-core/slang-artifact.cpp b/source/compiler-core/slang-artifact.cpp
index 5e5b93578..bf61763c8 100644
--- a/source/compiler-core/slang-artifact.cpp
+++ b/source/compiler-core/slang-artifact.cpp
@@ -58,10 +58,16 @@ void* Artifact::getInterface(const Guid& uuid)
 
 Artifact::~Artifact()
 {
+    // Remove the temporary
     if (m_pathType == PathType::Temporary)
     {
         File::remove(m_path);
     }
+    // If there is a temporary lock path, remove that
+    if (m_temporaryLockPath.getLength())
+    {
+        File::remove(m_temporaryLockPath);
+    }
 }
 
 bool Artifact::exists()
@@ -192,8 +198,10 @@ SlangResult Artifact::requireFile(Keep keep)
     // TODO(JS): NOTE! This isn't strictly correct, as the generated filename is not guarenteed to be unique
     // if we change it with an extension (or prefix).
     // This doesn't change the previous behavior though.
-    String path;
-    SLANG_RETURN_ON_FAIL(File::generateTemporary(nameBase, path));
+    String temporaryLockPath;
+    SLANG_RETURN_ON_FAIL(File::generateTemporary(nameBase, temporaryLockPath));
+
+    String path = temporaryLockPath;
 
     if (ArtifactInfoUtil::isCpuBinary(m_desc) && 
         (m_desc.kind == ArtifactKind::SharedLibrary ||
@@ -221,7 +229,6 @@ SlangResult Artifact::requireFile(Keep keep)
     }
 
     // If there is an extension append it
-
     const UnownedStringSlice ext = ArtifactInfoUtil::getDefaultExtension(m_desc);
 
     if (ext.getLength())
@@ -230,6 +237,13 @@ SlangResult Artifact::requireFile(Keep keep)
         path.append(ext);
     }
 
+    // If the final path is different from the lock path save that path
+    if (path != temporaryLockPath)
+    {
+        m_temporaryLockPath = temporaryLockPath;
+    }
+
+    // Write the contents
     SLANG_RETURN_ON_FAIL(File::writeAllBytes(path, blob->getBufferPointer(), blob->getBufferSize()));
 
     // Okay we can now add this as temporary path too
@@ -244,9 +258,6 @@ SlangResult Artifact::loadBlob(Keep keep, ISlangBlob** outBlob)
 
     if (!blob)
     {
-        // TODO(JS): 
-        // Strictly speaking we could *potentially* convert some other representation into
-        // a blob by serializing it, but we don't worry about any of that here
         if (m_pathType != PathType::None)
         {
             // Read into a blob
@@ -258,6 +269,7 @@ SlangResult Artifact::loadBlob(Keep keep, ISlangBlob** outBlob)
         }
         else
         {
+            // Look for a representation that we can serialize into a blob
             for (const auto& element : m_elements)
             {
                 ISlangUnknown* intf = element.value;
@@ -275,7 +287,6 @@ SlangResult Artifact::loadBlob(Keep keep, ISlangBlob** outBlob)
         }
 
         // Wasn't able to construct
-
         if (!blob)
         {
             return SLANG_E_NOT_FOUND;
diff --git a/source/compiler-core/slang-artifact.h b/source/compiler-core/slang-artifact.h
index 91f8bec37..362dac16f 100644
--- a/source/compiler-core/slang-artifact.h
+++ b/source/compiler-core/slang-artifact.h
@@ -176,8 +176,8 @@ SLANG_INLINE ArtifactKeep getIntermediateKeep(ArtifactKeep keep) { return (keep
 enum ArtifactPathType
 {
     None,
-    Temporary,
-    Existing,
+    Temporary,                  ///< Is a temporary file
+    Existing,                   ///< Is an existing file
 };
 
 /* The IArtifactInstance interface represents a single instance of a type that can be part of an artifact. It's special in so far 
@@ -383,6 +383,7 @@ protected:
 
     PathType m_pathType = PathType::None;       ///< What the path indicates
     String m_path;                              ///< The path 
+    String m_temporaryLockPath;                 ///< The temporary lock path
 
     ComPtr<ISlangBlob> m_blob;                  ///< Blob to store result in memory
 
diff --git a/source/compiler-core/slang-downstream-compiler.cpp b/source/compiler-core/slang-downstream-compiler.cpp
index 5011b42a0..85726557e 100644
--- a/source/compiler-core/slang-downstream-compiler.cpp
+++ b/source/compiler-core/slang-downstream-compiler.cpp
@@ -428,6 +428,7 @@ SlangResult CommandLineDownstreamCompiler::compile(const CompileOptions& inOptio
     // Find all the files that will be produced
     RefPtr<TemporaryFileSet> productFileSet(new TemporaryFileSet);
     
+    
     if (options.modulePath.getLength() == 0 || options.sourceContents.getLength() != 0)
     {
         String modulePath = options.modulePath;
@@ -435,7 +436,15 @@ SlangResult CommandLineDownstreamCompiler::compile(const CompileOptions& inOptio
         // If there is no module path, generate one.
         if (modulePath.getLength() == 0)
         {
-            SLANG_RETURN_ON_FAIL(File::generateTemporary(UnownedStringSlice::fromLiteral("slang-generated"), modulePath));
+            // Holds the temporary lock path, if a temporary path is used
+            String temporaryLockPath;
+
+            // Generate a unique module path name
+            SLANG_RETURN_ON_FAIL(File::generateTemporary(UnownedStringSlice::fromLiteral("slang-generated"), temporaryLockPath));
+            productFileSet->add(temporaryLockPath);
+
+            modulePath = temporaryLockPath;
+
             options.modulePath = modulePath;
         }
 
@@ -511,7 +520,6 @@ SlangResult CommandLineDownstreamCompiler::compile(const CompileOptions& inOptio
     DownstreamDiagnostics diagnostics;
     SLANG_RETURN_ON_FAIL(parseOutput(exeRes, diagnostics));
 
-    
     out = new CommandLineDownstreamCompileResult(diagnostics, moduleFilePath, productFileSet);
     
     return SLANG_OK;
diff --git a/source/core/slang-io.cpp b/source/core/slang-io.cpp
index c11ba85e0..17cd60160 100644
--- a/source/core/slang-io.cpp
+++ b/source/core/slang-io.cpp
@@ -101,7 +101,8 @@ namespace Slang
             char* chars = tempFileName.prepareForAppend(count);
 
             // https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettempfilenamea
-            //  Generates a temporary file name. 
+            // Generates a temporary file name.
+            // Will create a file with this name.
             DWORD ret = ::GetTempFileNameA(tempPath.getBuffer(), prefix.getBuffer(), 0, chars);
 
             if (ret == 0)
@@ -111,6 +112,8 @@ namespace Slang
             tempFileName.appendInPlace(chars, ::strlen(chars));
         }
 
+        SLANG_ASSERT(File::exists(tempFileName));
+
         outFileName = tempFileName;
         return SLANG_OK;
     }
@@ -135,6 +138,8 @@ namespace Slang
         close(handle);
 
         outFileName = buffer.getBuffer();
+        SLANG_ASSERT(File::exists(outFileName));
+
         return SLANG_OK;
     }
 #endif
diff --git a/source/core/slang-io.h b/source/core/slang-io.h
index 51b1d45e5..43812aaf9 100644
--- a/source/core/slang-io.h
+++ b/source/core/slang-io.h
@@ -27,6 +27,9 @@ namespace Slang
 
         static SlangResult makeExecutable(const String& fileName);
 
+            /// Creates a temporary file typically in some way based on the prefix
+            /// The file will be *created* with the outFileName, on success.
+            /// It's creation in necessary to lock that particular name.  
         static SlangResult generateTemporary(const UnownedStringSlice& prefix, String& outFileName);
     };