AutoRcc: Don't use cmQtAutoGenerator::FileSystem methods

`cmQtAutoGenerator::FileSystem` is only required for concurrent file system
access, but `cmQtAutoGeneratorRcc` isn't concurrent.  Therefore this patch
replaces all `cmQtAutoGenerator::FileSystem` uses in `cmQtAutoGeneratorRcc`.

4 files changed, 153 insertions, 97 deletions

diff --git a/Source/cmQtAutoGenerator.cxx b/Source/cmQtAutoGenerator.cxx
index 27afe48..f56f7c9 100644
--- a/Source/cmQtAutoGenerator.cxx
+++ b/Source/cmQtAutoGenerator.cxx
@@ -152,6 +152,87 @@ void cmQtAutoGenerator::Logger::ErrorCommand(
   }
 }
 
+bool cmQtAutoGenerator::MakeParentDirectory(std::string const& filename)
+{
+  bool success = true;
+  std::string const dirName = cmSystemTools::GetFilenamePath(filename);
+  if (!dirName.empty()) {
+    success = cmSystemTools::MakeDirectory(dirName);
+  }
+  return success;
+}
+
+bool cmQtAutoGenerator::FileRead(std::string& content,
+                                 std::string const& filename,
+                                 std::string* error)
+{
+  content.clear();
+  if (!cmSystemTools::FileExists(filename, true)) {
+    if (error != nullptr) {
+      error->append("Not a file.");
+    }
+    return false;
+  }
+
+  unsigned long const length = cmSystemTools::FileLength(filename);
+  cmsys::ifstream ifs(filename.c_str(), (std::ios::in | std::ios::binary));
+
+  // Use lambda to save destructor calls of ifs
+  return [&ifs, length, &content, error]() -> bool {
+    if (!ifs) {
+      if (error != nullptr) {
+        error->append("Opening the file for reading failed.");
+      }
+      return false;
+    }
+    content.reserve(length);
+    typedef std::istreambuf_iterator<char> IsIt;
+    content.assign(IsIt{ ifs }, IsIt{});
+    if (!ifs) {
+      content.clear();
+      if (error != nullptr) {
+        error->append("Reading from the file failed.");
+      }
+      return false;
+    }
+    return true;
+  }();
+}
+
+bool cmQtAutoGenerator::FileWrite(std::string const& filename,
+                                  std::string const& content,
+                                  std::string* error)
+{
+  // Make sure the parent directory exists
+  if (!cmQtAutoGenerator::MakeParentDirectory(filename)) {
+    if (error != nullptr) {
+      error->assign("Could not create parent directory.");
+    }
+    return false;
+  }
+  cmsys::ofstream ofs;
+  ofs.open(filename.c_str(),
+           (std::ios::out | std::ios::binary | std::ios::trunc));
+
+  // Use lambda to save destructor calls of ofs
+  return [&ofs, &content, error]() -> bool {
+    if (!ofs) {
+      if (error != nullptr) {
+        error->assign("Opening file for writing failed.");
+      }
+      return false;
+    }
+    ofs << content;
+    if (!ofs.good()) {
+      if (error != nullptr) {
+        error->assign("File writing failed.");
+      }
+      return false;
+    }
+    return true;
+  }();
+}
+
 std::string cmQtAutoGenerator::FileSystem::GetRealPath(
   std::string const& filename)
 {
@@ -267,33 +348,8 @@ bool cmQtAutoGenerator::FileSystem::FileRead(std::string& content,
                                              std::string const& filename,
                                              std::string* error)
 {
-  bool success = false;
-  if (FileExists(filename, true)) {
-    unsigned long const length = FileLength(filename);
-    {
-      std::lock_guard<std::mutex> lock(Mutex_);
-      cmsys::ifstream ifs(filename.c_str(), (std::ios::in | std::ios::binary));
-      if (ifs) {
-        content.reserve(length);
-        content.assign(std::istreambuf_iterator<char>{ ifs },
-                       std::istreambuf_iterator<char>{});
-        if (ifs) {
-          success = true;
-        } else {
-          content.clear();
-          if (error != nullptr) {
-            error->append("Reading from the file failed.");
-          }
-        }
-      } else if (error != nullptr) {
-        error->append("Opening the file for reading failed.");
-      }
-    }
-  } else if (error != nullptr) {
-    error->append(
-      "The file does not exist, is not readable or is a directory.");
-  }
-  return success;
+  std::lock_guard<std::mutex> lock(Mutex_);
+  return cmQtAutoGenerator::FileRead(content, filename, error);
 }
 
 bool cmQtAutoGenerator::FileSystem::FileRead(GenT genType,
@@ -312,34 +368,8 @@ bool cmQtAutoGenerator::FileSystem::FileWrite(std::string const& filename,
                                               std::string const& content,
                                               std::string* error)
 {
-  bool success = false;
-  // Make sure the parent directory exists
-  if (MakeParentDirectory(filename)) {
-    std::lock_guard<std::mutex> lock(Mutex_);
-    cmsys::ofstream outfile;
-    outfile.open(filename.c_str(),
-                 (std::ios::out | std::ios::binary | std::ios::trunc));
-    if (outfile) {
-      outfile << content;
-      // Check for write errors
-      if (outfile.good()) {
-        success = true;
-      } else {
-        if (error != nullptr) {
-          error->assign("File writing failed");
-        }
-      }
-    } else {
-      if (error != nullptr) {
-        error->assign("Opening file for writing failed");
-      }
-    }
-  } else {
-    if (error != nullptr) {
-      error->assign("Could not create parent directory");
-    }
-  }
-  return success;
+  std::lock_guard<std::mutex> lock(Mutex_);
+  return cmQtAutoGenerator::FileWrite(filename, content, error);
 }
 
 bool cmQtAutoGenerator::FileSystem::FileWrite(GenT genType,
@@ -399,12 +429,8 @@ bool cmQtAutoGenerator::FileSystem::MakeDirectory(GenT genType,
 bool cmQtAutoGenerator::FileSystem::MakeParentDirectory(
   std::string const& filename)
 {
-  bool success = true;
-  std::string const dirName = cmSystemTools::GetFilenamePath(filename);
-  if (!dirName.empty()) {
-    success = MakeDirectory(dirName);
-  }
-  return success;
+  std::lock_guard<std::mutex> lock(Mutex_);
+  return cmQtAutoGenerator::MakeParentDirectory(filename);
 }
 
 bool cmQtAutoGenerator::FileSystem::MakeParentDirectory(
diff --git a/Source/cmQtAutoGenerator.h b/Source/cmQtAutoGenerator.h
index b55bebc..17170d7 100644
--- a/Source/cmQtAutoGenerator.h
+++ b/Source/cmQtAutoGenerator.h
@@ -62,6 +62,14 @@ public:
     bool ColorOutput_ = false;
   };
 
+  // -- File system methods
+  static bool MakeParentDirectory(std::string const& filename);
+  static bool FileRead(std::string& content, std::string const& filename,
+                       std::string* error = nullptr);
+  static bool FileWrite(std::string const& filename,
+                        std::string const& content,
+                        std::string* error = nullptr);
+
   /// @brief Thread safe file system interface
   class FileSystem
   {
diff --git a/Source/cmQtAutoGeneratorRcc.cxx b/Source/cmQtAutoGeneratorRcc.cxx
index 792e6fd..add84de 100644
--- a/Source/cmQtAutoGeneratorRcc.cxx
+++ b/Source/cmQtAutoGeneratorRcc.cxx
@@ -52,7 +52,7 @@ bool cmQtAutoGeneratorRcc::Init(cmMakefile* makefile)
 
   // -- Read info file
   if (!makefile->ReadListFile(InfoFile())) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "File processing failed");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "File processing failed.");
     return false;
   }
 
@@ -63,13 +63,13 @@ bool cmQtAutoGeneratorRcc::Init(cmMakefile* makefile)
   // - Directories
   AutogenBuildDir_ = InfoGet("ARCC_BUILD_DIR");
   if (AutogenBuildDir_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "Build directory empty");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "Build directory empty.");
     return false;
   }
 
   IncludeDir_ = InfoGetConfig("ARCC_INCLUDE_DIR");
   if (IncludeDir_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "Include directory empty");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "Include directory empty.");
     return false;
   }
 
@@ -92,27 +92,27 @@ bool cmQtAutoGeneratorRcc::Init(cmMakefile* makefile)
 
   // - Validity checks
   if (LockFile_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "Lock file name missing");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "Lock file name missing.");
     return false;
   }
   if (SettingsFile_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "Settings file name missing");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "Settings file name missing.");
     return false;
   }
   if (AutogenBuildDir_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "Autogen build directory missing");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "Autogen build directory missing.");
     return false;
   }
   if (RccExecutable_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "rcc executable missing");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "rcc executable missing.");
     return false;
   }
   if (QrcFile_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "rcc input file missing");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "rcc input file missing.");
     return false;
   }
   if (RccFileName_.empty()) {
-    Log().ErrorFile(GenT::RCC, InfoFile(), "rcc output file missing");
+    Log().ErrorFile(GenT::RCC, InfoFile(), "rcc output file missing.");
     return false;
   }
 
@@ -153,9 +153,6 @@ bool cmQtAutoGeneratorRcc::Process()
   }
   // Generate on demand
   if (generate) {
-    if (!GenerateParentDir()) {
-      return false;
-    }
     if (!GenerateRcc()) {
       return false;
     }
@@ -210,17 +207,21 @@ bool cmQtAutoGeneratorRcc::SettingsFileRead()
   }
 
   // Make sure the settings file exists
-  if (!FileSys().FileExists(SettingsFile_, true)) {
+  if (!cmSystemTools::FileExists(SettingsFile_, true)) {
     // Touch the settings file to make sure it exists
-    FileSys().Touch(SettingsFile_, true);
+    if (!cmSystemTools::Touch(SettingsFile_, true)) {
+      Log().ErrorFile(GenT::RCC, SettingsFile_,
+                      "Settings file creation failed.");
+      return false;
+    }
   }
 
   // Lock the lock file
   {
     // Make sure the lock file exists
-    if (!FileSys().FileExists(LockFile_, true)) {
-      if (!FileSys().Touch(LockFile_, true)) {
-        Log().ErrorFile(GenT::RCC, LockFile_, "Lock file creation failed");
+    if (!cmSystemTools::FileExists(LockFile_, true)) {
+      if (!cmSystemTools::Touch(LockFile_, true)) {
+        Log().ErrorFile(GenT::RCC, LockFile_, "Lock file creation failed.");
         return false;
       }
     }
@@ -237,13 +238,18 @@ bool cmQtAutoGeneratorRcc::SettingsFileRead()
   // Read old settings
   {
     std::string content;
-    if (FileSys().FileRead(content, SettingsFile_)) {
+    if (FileRead(content, SettingsFile_)) {
       SettingsChanged_ = (SettingsString_ != SettingsFind(content, "rcc"));
       // In case any setting changed clear the old settings file.
       // This triggers a full rebuild on the next run if the current
       // build is aborted before writing the current settings in the end.
       if (SettingsChanged_) {
-        FileSys().FileWrite(GenT::RCC, SettingsFile_, "");
+        std::string error;
+        if (!FileWrite(SettingsFile_, "", &error)) {
+          Log().ErrorFile(GenT::RCC, SettingsFile_,
+                          "Settings file clearing failed. " + error);
+          return false;
+        }
       }
     } else {
       SettingsChanged_ = true;
@@ -264,11 +270,12 @@ bool cmQtAutoGeneratorRcc::SettingsFileWrite()
     std::string content = "rcc:";
     content += SettingsString_;
     content += '\n';
-    if (!FileSys().FileWrite(GenT::RCC, SettingsFile_, content)) {
+    std::string error;
+    if (!FileWrite(SettingsFile_, content, &error)) {
       Log().ErrorFile(GenT::RCC, SettingsFile_,
-                      "Settings file writing failed");
+                      "Settings file writing failed. " + error);
       // Remove old settings file to trigger a full rebuild on the next run
-      FileSys().FileRemove(SettingsFile_);
+      cmSystemTools::RemoveFile(SettingsFile_);
       return false;
     }
   }
@@ -398,21 +405,25 @@ bool cmQtAutoGeneratorRcc::TestInfoFile()
       Log().Info(GenT::RCC, reason);
     }
     // Touch build file
-    FileSys().Touch(RccFileOutput_);
+    if (!cmSystemTools::Touch(RccFileOutput_, false)) {
+      Log().ErrorFile(GenT::RCC, RccFileOutput_, "Build file touch failed");
+      return false;
+    }
     BuildFileChanged_ = true;
   }
 
   return true;
 }
 
-bool cmQtAutoGeneratorRcc::GenerateParentDir()
-{
-  // Make sure the parent directory exists
-  return FileSys().MakeParentDirectory(GenT::RCC, RccFileOutput_);
-}
-
 bool cmQtAutoGeneratorRcc::GenerateRcc()
 {
+  // Make parent directory
+  if (!MakeParentDirectory(RccFileOutput_)) {
+    Log().ErrorFile(GenT::RCC, RccFileOutput_,
+                    "Could not create parent directory");
+    return false;
+  }
+
   // Start a rcc process
   std::vector<std::string> cmd;
   cmd.push_back(RccExecutable_);
@@ -444,7 +455,7 @@ bool cmQtAutoGeneratorRcc::GenerateRcc()
       err += Quoted(RccFileOutput_);
       Log().ErrorCommand(GenT::RCC, err, cmd, rccStdOut + rccStdErr);
     }
-    FileSys().FileRemove(RccFileOutput_);
+    cmSystemTools::RemoveFile(RccFileOutput_);
     return false;
   }
 
@@ -470,15 +481,23 @@ bool cmQtAutoGeneratorRcc::GenerateWrapper()
     content += MultiConfigOutput();
     content += ">\n";
 
-    // Write content to file
-    if (FileSys().FileDiffers(RccFilePublic_, content)) {
+    // Compare with existing file content
+    bool fileDiffers = true;
+    {
+      std::string oldContents;
+      if (FileRead(oldContents, RccFilePublic_)) {
+        fileDiffers = (oldContents != content);
+      }
+    }
+    if (fileDiffers) {
       // Write new wrapper file
       if (Log().Verbose()) {
         Log().Info(GenT::RCC, "Generating RCC wrapper file " + RccFilePublic_);
       }
-      if (!FileSys().FileWrite(GenT::RCC, RccFilePublic_, content)) {
+      std::string error;
+      if (!FileWrite(RccFilePublic_, content, &error)) {
         Log().ErrorFile(GenT::RCC, RccFilePublic_,
-                        "RCC wrapper file writing failed");
+                        "RCC wrapper file writing failed. " + error);
         return false;
       }
     } else if (BuildFileChanged_) {
@@ -486,7 +505,11 @@ bool cmQtAutoGeneratorRcc::GenerateWrapper()
       if (Log().Verbose()) {
         Log().Info(GenT::RCC, "Touching RCC wrapper file " + RccFilePublic_);
       }
-      FileSys().Touch(RccFilePublic_);
+      if (!cmSystemTools::Touch(RccFilePublic_, false)) {
+        Log().ErrorFile(GenT::RCC, RccFilePublic_,
+                        "RCC wrapper file touch failed.");
+        return false;
+      }
     }
   }
   return true;
diff --git a/Source/cmQtAutoGeneratorRcc.h b/Source/cmQtAutoGeneratorRcc.h
index baa560c..d831f1f 100644
--- a/Source/cmQtAutoGeneratorRcc.h
+++ b/Source/cmQtAutoGeneratorRcc.h
@@ -36,7 +36,6 @@ private:
   bool TestResources(bool& generate);
   bool TestInfoFile();
   // -- Generation
-  bool GenerateParentDir();
   bool GenerateRcc();
   bool GenerateWrapper();