[clang] [llvm] [Support][Modules] Ensure parent directory exists when creating lock file (PR #189888)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Aadarsh Keshri (Aadarsh-Keshri)

<details>
<summary>Changes</summary>

Following #<!-- -->187372 

---
Full diff: https://github.com/llvm/llvm-project/pull/189888.diff


3 Files Affected:

- (modified) clang/lib/Serialization/ModuleCache.cpp (-5) 
- (modified) llvm/lib/Support/LockFileManager.cpp (+24-3) 
- (modified) llvm/unittests/Support/LockFileManagerTest.cpp (+28) 


``````````diff
diff --git a/clang/lib/Serialization/ModuleCache.cpp 
b/clang/lib/Serialization/ModuleCache.cpp
index 658da6e3b7145..7c57d143d2e03 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -109,11 +109,6 @@ class CrossProcessModuleCache : public ModuleCache {
   void prepareForGetLock(StringRef ModuleFilename) override {
     // This is a compiler-internal input/output, let's bypass the sandbox.
     auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
-
-    // FIXME: Do this in LockFileManager and only if the directory doesn't
-    // exist.
-    StringRef Dir = llvm::sys::path::parent_path(ModuleFilename);
-    llvm::sys::fs::create_directories(Dir);
   }
 
   std::unique_ptr<llvm::AdvisoryLock>
diff --git a/llvm/lib/Support/LockFileManager.cpp 
b/llvm/lib/Support/LockFileManager.cpp
index 1a98ec03486db..205339f9366bd 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/IOSandbox.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
@@ -185,10 +186,30 @@ Expected<bool> LockFileManager::tryLock() {
   UniqueLockFileName = LockFileName;
   UniqueLockFileName += "-%%%%%%%%";
   int UniqueLockFileID;
-  if (std::error_code EC = sys::fs::createUniqueFile(
-          UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
-    return createStringError(EC, "failed to create unique file " +
+
+  {
+    std::error_code EC = sys::fs::createUniqueFile(
+        UniqueLockFileName, UniqueLockFileID, UniqueLockFileName);
+    if (EC == errc::no_such_file_or_directory) {
+      SmallString<128> Dir = sys::path::parent_path(UniqueLockFileName);
+      if (!Dir.empty()) {
+        if (std::error_code DirEC = sys::fs::create_directories(Dir))
+          return createStringError(DirEC,
+                                   "failed to create lock directory " + Dir);
+      }
+
+      // Retry creating lock file
+      UniqueLockFileName = LockFileName;
+      UniqueLockFileName += "-%%%%%%%%";
+
+      EC = sys::fs::createUniqueFile(UniqueLockFileName, UniqueLockFileID,
                                      UniqueLockFileName);
+    }
+
+    if (EC)
+      return createStringError(EC, "failed to create unique file " +
+                                       UniqueLockFileName);
+  }
 
   // Clean up the unique file on signal or scope exit.
   RemoveUniqueLockFileOnSignal RemoveUniqueFile(UniqueLockFileName);
diff --git a/llvm/unittests/Support/LockFileManagerTest.cpp 
b/llvm/unittests/Support/LockFileManagerTest.cpp
index bd61b6c36efb3..ce5fd4d5063ce 100644
--- a/llvm/unittests/Support/LockFileManagerTest.cpp
+++ b/llvm/unittests/Support/LockFileManagerTest.cpp
@@ -104,4 +104,32 @@ TEST(LockFileManagerTest, RelativePath) {
   ASSERT_FALSE(chdir(OrigPath));
 }
 
+TEST(LockFileManagerTest, NonExistentDirectory) {
+  TempDir LockFileManagerTestDir("LockFileManagerTestDir", /*Unique*/ true);
+
+  SmallString<64> NonExistentDir(LockFileManagerTestDir.path());
+  sys::path::append(NonExistentDir, "does_not_exist");
+
+  SmallString<64> LockedFile(NonExistentDir);
+  sys::path::append(LockedFile, "file");
+
+  SmallString<64> FileLock(LockedFile);
+  FileLock += ".lock";
+
+  // Ensure directory truly does not exist before test
+  EXPECT_FALSE(sys::fs::exists(NonExistentDir));
+
+  {
+    LockFileManager Locked(LockedFile);
+    EXPECT_THAT_EXPECTED(Locked.tryLock(), HasValue(true));
+
+    // Directory and lock file should now exist
+    EXPECT_TRUE(sys::fs::exists(NonExistentDir));
+    EXPECT_TRUE(sys::fs::exists(FileLock));
+  }
+
+  // After destruction, lock file should be removed
+  EXPECT_FALSE(sys::fs::exists(FileLock));
+}
+
 } // end anonymous namespace

``````````

</details>


https://github.com/llvm/llvm-project/pull/189888
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits