[VirtualOutputBackend] Support OF_Append output files

Add support for appending output file types with both atomic and
non-atomic update to the output file. Atomic append is implemented with
a `llvm::LockFileManager` that locks the output file while appending to
it.

Author: cachemeifyoucan

llvm/include/llvm/Support/VirtualOutputConfig.def

@@ -16,6 +16,7 @@
 
 HANDLE_OUTPUT_CONFIG_FLAG(Text, false) // OF_Text.
 HANDLE_OUTPUT_CONFIG_FLAG(CRLF, false) // OF_CRLF.
+HANDLE_OUTPUT_CONFIG_FLAG(Append, false) // OF_Append.
 HANDLE_OUTPUT_CONFIG_FLAG(DiscardOnSignal, true) // E.g., RemoveFileOnSignal.
 HANDLE_OUTPUT_CONFIG_FLAG(AtomicWrite, true) // E.g., use temporaries.
 HANDLE_OUTPUT_CONFIG_FLAG(ImplyCreateDirectories, true)

llvm/include/llvm/Support/VirtualOutputConfig.h

@@ -53,8 +53,8 @@ struct OutputConfig : detail::EmptyBaseClass {
   constexpr OutputConfig &setTextWithCRLF(bool Value) {
     return Value ? setText().setCRLF() : setBinary();
   }
-  constexpr bool getTextWithCRLF() { return getText() && getCRLF(); }
-  constexpr bool getBinary() { return !getText(); }
+  constexpr bool getTextWithCRLF() const { return getText() && getCRLF(); }
+  constexpr bool getBinary() const { return !getText(); }
 
   /// Updates Text and CRLF flags based on \a sys::fs::OF_Text and \a
   /// sys::fs::OF_CRLF in \p Flags. Rejects CRLF without Text (calling

llvm/lib/Support/VirtualOutputBackends.cpp

@@ -13,6 +13,8 @@
 #include "llvm/Support/VirtualOutputBackends.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/LockFileManager.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
@@ -306,6 +308,8 @@ Error OnDiskOutputFile::initializeFD(Optional<int> &FD) {
       OF |= sys::fs::OF_TextWithCRLF;
     else if (Config.getText())
       OF |= sys::fs::OF_Text;
+    if (Config.getAppend())
+      OF |= sys::fs::OF_Append;
     if (std::error_code EC = sys::fs::openFileForWrite(
             OutputPath, NewFD, sys::fs::CD_CreateAlways, OF))
       return convertToOutputError(OutputPath, EC);
@@ -448,6 +452,64 @@ Error OnDiskOutputFile::keep() {
   if (!TempPath)
     return Error::success();
 
+  // See if we should append instead of move.
+  if (Config.getAppend() && OutputPath != "-") {
+    // Read TempFile for the content to append.
+    auto Content = MemoryBuffer::getFile(*TempPath);
+    if (!Content)
+      return convertToTempFileOutputError(*TempPath, OutputPath,
+                                          Content.getError());
+    while (1) {
+      // Attempt to lock the output file.
+      // Only one process is allowed to append to this file at a time.
+      llvm::LockFileManager Locked(OutputPath);
+      switch (Locked) {
+      case llvm::LockFileManager::LFS_Error: {
+        // If we error acquiring a lock, we cannot ensure appends
+        // to the trace file are atomic - cannot ensure output correctness.
+        Locked.unsafeRemoveLockFile();
+        return convertToOutputError(
+            OutputPath, std::make_error_code(std::errc::no_lock_available));
+      }
+      case llvm::LockFileManager::LFS_Owned: {
+        // Lock acquired, perform the write and release the lock.
+        std::error_code EC;
+        llvm::raw_fd_ostream Out(OutputPath, EC, llvm::sys::fs::OF_Append);
+        if (EC)
+          return convertToOutputError(OutputPath, EC);
+        Out << (*Content)->getBuffer();
+        Out.close();
+        Locked.unsafeRemoveLockFile();
+        if (Out.has_error())
+          return convertToOutputError(OutputPath, Out.error());
+        // Remove temp file and done.
+        (void)sys::fs::remove(*TempPath);
+        return Error::success();
+      }
+      case llvm::LockFileManager::LFS_Shared: {
+        // Someone else owns the lock on this file, wait.
+        switch (Locked.waitForUnlock(256)) {
+        case llvm::LockFileManager::Res_Success:
+          LLVM_FALLTHROUGH;
+        case llvm::LockFileManager::Res_OwnerDied: {
+          continue; // try again to get the lock.
+        }
+        case llvm::LockFileManager::Res_Timeout: {
+          // We could error on timeout to avoid potentially hanging forever, but
+          // it may be more likely that an interrupted process failed to clear
+          // the lock, causing other waiting processes to time-out. Let's clear
+          // the lock and try again right away. If we do start seeing compiler
+          // hangs in this location, we will need to re-consider.
+          Locked.unsafeRemoveLockFile();
+          continue;
+        }
+        }
+        break;
+      }
+      }
+    }
+  }
+
   if (Config.getOnlyIfDifferent()) {
     auto Result = areFilesDifferent(*TempPath, OutputPath);
     if (!Result)

llvm/lib/Support/VirtualOutputConfig.cpp

@@ -17,7 +17,9 @@ using namespace llvm::vfs;
 OutputConfig &OutputConfig::setOpenFlags(const sys::fs::OpenFlags &Flags) {
   // Ignore CRLF on its own as invalid.
   using namespace llvm::sys::fs;
-  return Flags & OF_Text ? setText().setCRLF(Flags & OF_CRLF) : setBinary();
+  return Flags & OF_Text
+             ? setText().setCRLF(Flags & OF_CRLF).setAppend(Flags & OF_Append)
+             : setBinary().setAppend(Flags & OF_Append);
 }
 
 void OutputConfig::print(raw_ostream &OS) const {

llvm/unittests/Support/VirtualOutputBackendsTest.cpp

@@ -807,6 +807,47 @@ TEST(OnDiskBackendTest, OnlyIfDifferent) {
   EXPECT_NE(Status1.getUniqueID(), Status3.getUniqueID());
 }
 
+TEST(OnDiskBackendTest, Append) {
+  OnDiskOutputBackendProvider Provider;
+  auto Backend = Provider.createBackend();
+  std::string FilePath = Provider.getFilePathToCreate();
+  OutputConfig Config = OutputConfig().setAppend();
+
+  OutputFile O1, O2, O3;
+  // Write first file.
+  EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O1),
+                    Succeeded());
+  O1 << "some data\n";
+  EXPECT_THAT_ERROR(O1.keep(), Succeeded());
+  EXPECT_FALSE(O1.isOpen());
+
+  OnDiskFile File1(*Provider.D, FilePath);
+  EXPECT_TRUE(File1.equalsCurrentContent("some data\n"));
+
+  // Append same data.
+  EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O2),
+                    Succeeded());
+  O2 << "more data\n";
+  EXPECT_THAT_ERROR(O2.keep(), Succeeded());
+  EXPECT_FALSE(O2.isOpen());
+
+  // Check data is appended.
+  OnDiskFile File2(*Provider.D, FilePath);
+  EXPECT_TRUE(File2.equalsCurrentContent("some data\nmore data\n"));
+
+  // Non atomic append.
+  EXPECT_THAT_ERROR(
+      Backend->createFile(FilePath, Config.setNoAtomicWrite()).moveInto(O3),
+      Succeeded());
+  O3 << "more more\n";
+  EXPECT_THAT_ERROR(O3.keep(), Succeeded());
+  EXPECT_FALSE(O3.isOpen());
+
+  // Check data is appended.
+  OnDiskFile File3(*Provider.D, FilePath);
+  EXPECT_TRUE(File3.equalsCurrentContent("some data\nmore data\nmore more\n"));
+}
+
 TEST(HashingBackendTest, HashOutput) {
   HashingOutputBackend<BLAKE3> Backend;
   OutputFile O1, O2, O3, O4, O5;

llvm/unittests/Support/VirtualOutputConfigTest.cpp

@@ -23,6 +23,7 @@ TEST(VirtualOutputConfigTest, construct) {
   EXPECT_TRUE(OutputConfig().getAtomicWrite());
   EXPECT_TRUE(OutputConfig().getImplyCreateDirectories());
   EXPECT_FALSE(OutputConfig().getOnlyIfDifferent());
+  EXPECT_FALSE(OutputConfig().getAppend());
 
   // Test inverted defaults.
   EXPECT_TRUE(OutputConfig().getNoText());
@@ -31,6 +32,7 @@ TEST(VirtualOutputConfigTest, construct) {
   EXPECT_FALSE(OutputConfig().getNoAtomicWrite());
   EXPECT_FALSE(OutputConfig().getNoImplyCreateDirectories());
   EXPECT_TRUE(OutputConfig().getNoOnlyIfDifferent());
+  EXPECT_TRUE(OutputConfig().getNoAppend());
 }
 
 TEST(VirtualOutputConfigTest, set) {
@@ -124,7 +126,6 @@ TEST(VirtualOutputConfigTest, OpenFlags) {
 
   // Most flags are not supported / have no effect.
   EXPECT_EQ(OutputConfig(), OutputConfig().setOpenFlags(OF_None));
-  EXPECT_EQ(OutputConfig(), OutputConfig().setOpenFlags(OF_Append));
   EXPECT_EQ(OutputConfig(), OutputConfig().setOpenFlags(OF_Delete));
   EXPECT_EQ(OutputConfig(), OutputConfig().setOpenFlags(OF_ChildInherit));
   EXPECT_EQ(OutputConfig(), OutputConfig().setOpenFlags(OF_UpdateAtime));
@@ -134,6 +135,7 @@ TEST(VirtualOutputConfigTest, OpenFlags) {
            OutputConfig(),
            OutputConfig().setText(),
            OutputConfig().setTextWithCRLF(),
+           OutputConfig().setAppend(),
 
            // Should be overridden despite being invalid.
            OutputConfig().setCRLF(),
@@ -143,6 +145,7 @@ TEST(VirtualOutputConfigTest, OpenFlags) {
     EXPECT_EQ(OutputConfig().setText(), Init.setOpenFlags(OF_Text));
     EXPECT_EQ(OutputConfig().setTextWithCRLF(),
               Init.setOpenFlags(OF_TextWithCRLF));
+    EXPECT_EQ(OutputConfig().setAppend(), Init.setOpenFlags(OF_Append));
   }
 }