Merge topic 'update-kwsys'

3ef5dab0 cmSystemTools: Simplify using KWSys Status
ec1b6157 Update CMake code using KWSys to account for Status return values
eef585ef Merge branch 'upstream-KWSys' into update-kwsys
a7901671

 KWSys 2021-04-14 (7ab1c601)
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Ben Boeckel <ben.boeckel@kitware.com>
Merge-request: !6004

Source/CPack/cmCPackSTGZGenerator.cxx

@@ -51,15 +51,16 @@ int cmCPackSTGZGenerator::PackageFiles()
    * so we must iterate over generated packages.
    */
   for (std::string const& pfn : this->packageFileNames) {
-    retval &= cmSystemTools::SetPermissions(pfn.c_str(),
+    retval &= static_cast<bool>(
+      cmSystemTools::SetPermissions(pfn.c_str(),
 #if defined(_MSC_VER) || defined(__MINGW32__)
-                                            S_IREAD | S_IWRITE | S_IEXEC
+                                    S_IREAD | S_IWRITE | S_IEXEC
 #else
-                                            S_IRUSR | S_IWUSR | S_IXUSR |
-                                              S_IRGRP | S_IWGRP | S_IXGRP |
-                                              S_IROTH | S_IWOTH | S_IXOTH
+                                    S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP |
+                                      S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH |
+                                      S_IXOTH
 #endif
-    );
+                                    ));
   }
   return retval;
 }

Source/CTest/cmCTestScriptHandler.cxx

@@ -921,7 +921,7 @@ bool cmCTestScriptHandler::TryToRemoveBinaryDirectoryOnce(
     }
   }
 
-  return cmSystemTools::RemoveADirectory(directoryPath);
+  return static_cast<bool>(cmSystemTools::RemoveADirectory(directoryPath));
 }
 
 cmDuration cmCTestScriptHandler::GetRemainingTimeAllowed()

Source/CTest/cmCTestTestHandler.cxx

@@ -1881,7 +1881,7 @@ void cmCTestTestHandler::ExpandTestsToRunInformationForRerunFailed()
   std::string dirName = this->CTest->GetBinaryDir() + "/Testing/Temporary";
 
   cmsys::Directory directory;
-  if (directory.Load(dirName) == 0) {
+  if (!directory.Load(dirName)) {
     cmCTestLog(this->CTest, ERROR_MESSAGE,
                "Unable to read the contents of " << dirName << std::endl);
     return;

Source/cmCTest.cxx

@@ -4,7 +4,6 @@
 
 #include <algorithm>
 #include <cctype>
-#include <cerrno>
 #include <chrono>
 #include <cstdio>
 #include <cstdlib>
@@ -2842,9 +2841,10 @@ int cmCTest::ExecuteTests()
       cmCTestLog(this, OUTPUT,
                  "Internal ctest changing into directory: " << workDir
                                                             << std::endl);
-      if (cmSystemTools::ChangeDirectory(workDir) != 0) {
+      cmsys::Status status = cmSystemTools::ChangeDirectory(workDir);
+      if (!status) {
         auto msg = "Failed to change working directory to \"" + workDir +
-          "\" : " + std::strerror(errno) + "\n";
+          "\" : " + status.GetString() + "\n";
         cmCTestLog(this, ERROR_MESSAGE, msg);
         return 1;
       }

Source/cmConditionEvaluator.cxx

@@ -654,10 +654,10 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList& newArgs,
       if (argP1 != newArgs.end() && argP2 != newArgs.end() &&
           this->IsKeyword(keyIS_NEWER_THAN, *argP1)) {
         int fileIsNewer = 0;
-        bool success = cmSystemTools::FileTimeCompare(
+        cmsys::Status ftcStatus = cmSystemTools::FileTimeCompare(
           arg->GetValue(), (argP2)->GetValue(), &fileIsNewer);
         this->HandleBinaryOp(
-          (!success || fileIsNewer == 1 || fileIsNewer == 0), reducible, arg,
+          (!ftcStatus || fileIsNewer == 1 || fileIsNewer == 0), reducible, arg,
           newArgs, argP1, argP2);
       }
 

Source/cmFileCommand.cxx

@@ -2956,9 +2956,12 @@ bool HandleCreateLinkCommand(std::vector<std::string> const& args,
 
   // Check if copy-on-error is enabled in the arguments.
   if (!completed && arguments.CopyOnError) {
-    completed = cmsys::SystemTools::CopyFileAlways(fileName, newFileName);
-    if (!completed) {
-      result = "Copy failed: " + cmSystemTools::GetLastSystemError();
+    cmsys::Status copied =
+      cmsys::SystemTools::CopyFileAlways(fileName, newFileName);
+    if (copied) {
+      completed = true;
+    } else {
+      result = "Copy failed: " + copied.GetString();
     }
   }
 

Source/cmQtAutoGenerator.cxx

@@ -116,7 +116,7 @@ bool cmQtAutoGenerator::MakeParentDirectory(std::string const& filename)
   bool success = true;
   std::string const dirName = cmSystemTools::GetFilenamePath(filename);
   if (!dirName.empty()) {
-    success = cmSystemTools::MakeDirectory(dirName);
+    success = static_cast<bool>(cmSystemTools::MakeDirectory(dirName));
   }
   return success;
 }

Source/cmSystemTools.cxx

@@ -153,27 +153,6 @@ static int cm_archive_read_open_file(struct archive* a, const char* file,
 #  define environ (*_NSGetEnviron())
 #endif
 
-namespace {
-void ReportError(std::string* err)
-{
-  if (!err) {
-    return;
-  }
-#ifdef _WIN32
-  LPSTR message = NULL;
-  DWORD size = FormatMessageA(
-    FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
-      FORMAT_MESSAGE_IGNORE_INSERTS,
-    NULL, GetLastError(), MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
-    (LPSTR)&message, 0, NULL);
-  *err = std::string(message, size);
-  LocalFree(message);
-#else
-  *err = strerror(errno);
-#endif
-}
-}
-
 bool cmSystemTools::s_RunCommandHideConsole = false;
 bool cmSystemTools::s_DisableRunCommandOutput = false;
 bool cmSystemTools::s_ErrorOccured = false;
@@ -1016,23 +995,31 @@ cmSystemTools::CopyResult cmSystemTools::CopySingleFile(
   }
 
   mode_t perm = 0;
-  bool perms = SystemTools::GetPermissions(oldname, perm);
+  cmsys::Status perms = SystemTools::GetPermissions(oldname, perm);
 
   // If files are the same do not copy
   if (SystemTools::SameFile(oldname, newname)) {
     return CopyResult::Success;
   }
 
-  if (!cmsys::SystemTools::CloneFileContent(oldname, newname)) {
+  cmsys::Status status;
+  status = cmsys::SystemTools::CloneFileContent(oldname, newname);
+  if (!status) {
     // if cloning did not succeed, fall back to blockwise copy
-    if (!cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname)) {
-      ReportError(err);
-      return CopyResult::Failure;
+    status = cmsys::SystemTools::CopyFileContentBlockwise(oldname, newname);
+  }
+  if (!status) {
+    if (err) {
+      *err = status.GetString();
     }
+    return CopyResult::Failure;
   }
   if (perms) {
-    if (!SystemTools::SetPermissions(newname, perm)) {
-      ReportError(err);
+    status = SystemTools::SetPermissions(newname, perm);
+    if (!status) {
+      if (err) {
+        *err = status.GetString();
+      }
       return CopyResult::Failure;
     }
   }
@@ -1090,7 +1077,9 @@ cmSystemTools::RenameResult cmSystemTools::RenameFile(
       if (replace == Replace::No && move_last_error == ERROR_ALREADY_EXISTS) {
         return RenameResult::NoReplace;
       }
-      ReportError(err);
+      if (err) {
+        *err = cmsys::Status::Windows(move_last_error).GetString();
+      }
       return RenameResult::Failure;
     }
 
@@ -1121,7 +1110,9 @@ cmSystemTools::RenameResult cmSystemTools::RenameFile(
   if (replace == Replace::No && GetLastError() == ERROR_ALREADY_EXISTS) {
     return RenameResult::NoReplace;
   }
-  ReportError(err);
+  if (err) {
+    *err = cmsys::Status::Windows_GetLastError().GetString();
+  }
   return RenameResult::Failure;
 #else
   // On UNIX we have OS-provided calls to create 'newname' atomically.
@@ -1132,13 +1123,17 @@ cmSystemTools::RenameResult cmSystemTools::RenameFile(
     if (errno == EEXIST) {
       return RenameResult::NoReplace;
     }
-    ReportError(err);
+    if (err) {
+      *err = cmsys::Status::POSIX_errno().GetString();
+    }
     return RenameResult::Failure;
   }
   if (rename(oldname.c_str(), newname.c_str()) == 0) {
     return RenameResult::Success;
   }
-  ReportError(err);
+  if (err) {
+    *err = cmsys::Status::POSIX_errno().GetString();
+  }
   return RenameResult::Failure;
 #endif
 }
@@ -3130,7 +3125,7 @@ bool cmSystemTools::RepeatedRemoveDirectory(const std::string& dir)
   }
   return false;
 #else
-  return cmSystemTools::RemoveADirectory(dir);
+  return static_cast<bool>(cmSystemTools::RemoveADirectory(dir));
 #endif
 }
 

Source/cmSystemTools.h

@@ -12,6 +12,7 @@
 #include <cm/string_view>
 
 #include "cmsys/Process.h"
+#include "cmsys/Status.hxx"      // IWYU pragma: export
 #include "cmsys/SystemTools.hxx" // IWYU pragma: export
 
 #include "cmCryptoHash.h"

Source/cmWorkingDirectory.cxx

@@ -19,7 +19,7 @@ cmWorkingDirectory::~cmWorkingDirectory()
 
 bool cmWorkingDirectory::SetDirectory(std::string const& newdir)
 {
-  if (cmSystemTools::ChangeDirectory(newdir) == 0) {
+  if (cmSystemTools::ChangeDirectory(newdir)) {
     this->ResultCode = 0;
     return true;
   }

Source/cmcmd.cxx

@@ -1635,7 +1635,7 @@ bool cmcmd::SymlinkInternal(std::string const& file, std::string const& link)
     cmSystemTools::RemoveFile(link);
   }
 #if defined(_WIN32) && !defined(__CYGWIN__)
-  return cmSystemTools::CopyFileAlways(file, link);
+  return static_cast<bool>(cmSystemTools::CopyFileAlways(file, link));
 #else
   std::string linktext = cmSystemTools::GetFilenameName(file);
   return cmSystemTools::CreateSymlink(linktext, link);

Source/kwsys/CMakeLists.txt

@@ -142,6 +142,7 @@ if(KWSYS_STANDALONE OR CMake_SOURCE_DIR)
   set(KWSYS_USE_MD5 1)
   set(KWSYS_USE_Process 1)
   set(KWSYS_USE_RegularExpression 1)
+  set(KWSYS_USE_Status 1)
   set(KWSYS_USE_System 1)
   set(KWSYS_USE_SystemTools 1)
   set(KWSYS_USE_CommandLineArguments 1)
@@ -157,6 +158,7 @@ if(KWSYS_USE_SystemTools)
   set(KWSYS_USE_Directory 1)
   set(KWSYS_USE_FStream 1)
   set(KWSYS_USE_Encoding 1)
+  set(KWSYS_USE_Status 1)
 endif()
 if(KWSYS_USE_Glob)
   set(KWSYS_USE_Directory 1)
@@ -177,6 +179,7 @@ if(KWSYS_USE_System)
 endif()
 if(KWSYS_USE_Directory)
   set(KWSYS_USE_Encoding 1)
+  set(KWSYS_USE_Status 1)
 endif()
 if(KWSYS_USE_DynamicLoader)
   set(KWSYS_USE_Encoding 1)
@@ -630,7 +633,7 @@ set(KWSYS_HXX_FILES Configure String)
 # Add selected C++ classes.
 set(cppclasses
   Directory DynamicLoader Encoding Glob RegularExpression SystemTools
-  CommandLineArguments FStream SystemInformation ConsoleBuf
+  CommandLineArguments FStream SystemInformation ConsoleBuf Status
   )
 foreach(cpp ${cppclasses})
   if(KWSYS_USE_${cpp})
@@ -963,6 +966,7 @@ if(KWSYS_STANDALONE OR CMake_SOURCE_DIR)
     # C++ tests
     set(KWSYS_CXX_TESTS ${KWSYS_CXX_TESTS}
       testConfigure.cxx
+      testStatus.cxx
       testSystemTools.cxx
       testCommandLineArguments.cxx
       testCommandLineArguments1.cxx

Source/kwsys/Directory.cxx

@@ -94,7 +94,7 @@ void Directory::Clear()
 
 namespace KWSYS_NAMESPACE {
 
-bool Directory::Load(const std::string& name, std::string* errorMessage)
+Status Directory::Load(std::string const& name, std::string* errorMessage)
 {
   this->Clear();
   intptr_t srchHandle;
@@ -121,21 +121,11 @@ bool Directory::Load(const std::string& name, std::string* errorMessage)
   delete[] buf;
 
   if (srchHandle == -1) {
+    Status status = Status::POSIX_errno();
     if (errorMessage) {
-      if (unsigned int errorId = GetLastError()) {
-        LPSTR message = nullptr;
-        DWORD size = FormatMessageA(
-          FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
-            FORMAT_MESSAGE_IGNORE_INSERTS,
-          nullptr, errorId, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
-          (LPSTR)&message, 0, nullptr);
-        *errorMessage = std::string(message, size);
-        LocalFree(message);
-      } else {
-        *errorMessage = "Unknown error.";
-      }
+      *errorMessage = status.GetString();
     }
-    return false;
+    return status;
   }
 
   // Loop through names
@@ -143,7 +133,14 @@ bool Directory::Load(const std::string& name, std::string* errorMessage)
     this->Internal->Files.push_back(Encoding::ToNarrow(data.name));
   } while (_wfindnext(srchHandle, &data) != -1);
   this->Internal->Path = name;
-  return _findclose(srchHandle) != -1;
+  if (_findclose(srchHandle) == -1) {
+    Status status = Status::POSIX_errno();
+    if (errorMessage) {
+      *errorMessage = status.GetString();
+    }
+    return status;
+  }
+  return Status::Success();
 }
 
 unsigned long Directory::GetNumberOfFilesInDirectory(const std::string& name,
@@ -220,7 +217,7 @@ unsigned long Directory::GetNumberOfFilesInDirectory(const std::string& name,
 
 namespace KWSYS_NAMESPACE {
 
-bool Directory::Load(const std::string& name, std::string* errorMessage)
+Status Directory::Load(std::string const& name, std::string* errorMessage)
 {
   this->Clear();
 
@@ -231,7 +228,7 @@ bool Directory::Load(const std::string& name, std::string* errorMessage)
     if (errorMessage != nullptr) {
       *errorMessage = std::string(strerror(errno));
     }
-    return false;
+    return Status::POSIX_errno();
   }
 
   errno = 0;
@@ -242,12 +239,12 @@ bool Directory::Load(const std::string& name, std::string* errorMessage)
     if (errorMessage != nullptr) {
       *errorMessage = std::string(strerror(errno));
     }
-    return false;
+    return Status::POSIX_errno();
   }
 
   this->Internal->Path = name;
   closedir(dir);
-  return true;
+  return Status::Success();
 }
 
 unsigned long Directory::GetNumberOfFilesInDirectory(const std::string& name,

Source/kwsys/Directory.hxx.in

@@ -4,6 +4,7 @@
 #define @KWSYS_NAMESPACE@_Directory_hxx
 
 #include <@KWSYS_NAMESPACE@/Configure.h>
+#include <@KWSYS_NAMESPACE@/Status.hxx>
 
 #include <string>
 
@@ -32,10 +33,9 @@ public:
 
   /**
    * Load the specified directory and load the names of the files
-   * in that directory. 0 is returned if the directory can not be
-   * opened, 1 if it is opened.
+   * in that directory.
    */
-  bool Load(const std::string&, std::string* errorMessage = nullptr);
+  Status Load(std::string const&, std::string* errorMessage = nullptr);
 
   /**
    * Return the number of files in the current directory.

Source/kwsys/Status.cxx

+/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
+   file Copyright.txt or https://cmake.org/licensing#kwsys for details.  */
+#include "kwsysPrivate.h"
+#include KWSYS_HEADER(Status.hxx)
+
+// Work-around CMake dependency scanning limitation.  This must
+// duplicate the above list of headers.
+#if 0
+#  include "Status.hxx.in"
+#endif
+
+#include <cerrno>
+#include <cstring>
+#include <string>
+
+#if defined(_WIN32)
+#  include <windows.h>
+#endif
+
+namespace KWSYS_NAMESPACE {
+
+Status Status::POSIX_errno()
+{
+  return Status::POSIX(errno);
+}
+
+#ifdef _WIN32
+Status Status::Windows_GetLastError()
+{
+  return Status::Windows(GetLastError());
+}
+#endif
+
+std::string Status::GetString() const
+{
+  std::string err;
+  switch (this->Kind_) {
+    case Kind::Success:
+      err = "Success";
+      break;
+    case Kind::POSIX:
+      err = strerror(this->POSIX_);
+      break;
+#ifdef _WIN32
+    case Kind::Windows: {
+      LPSTR message = NULL;
+      DWORD size = FormatMessageA(
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
+          FORMAT_MESSAGE_IGNORE_INSERTS,
+        NULL, this->Windows_, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+        (LPSTR)&message, 0, NULL);
+      err = std::string(message, size);
+      LocalFree(message);
+    } break;
+#endif
+  };
+  return err;
+}
+
+} // namespace KWSYS_NAMESPACE

Source/kwsys/Status.hxx.in

+/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
+   file Copyright.txt or https://cmake.org/licensing#kwsys for details.  */
+#ifndef @KWSYS_NAMESPACE@_Status_hxx
+#define @KWSYS_NAMESPACE@_Status_hxx
+
+#include <@KWSYS_NAMESPACE@/Configure.hxx>
+
+#include <string>
+
+namespace @KWSYS_NAMESPACE@ {
+
+/** \class Status
+ * \brief OS-specific status of a system operation.
+ */
+class @KWSYS_NAMESPACE@_EXPORT Status
+{
+public:
+  enum class Kind
+  {
+    Success,
+    POSIX,
+#ifdef _WIN32
+    Windows,
+#endif
+  };
+
+  /** Construct with kind "Success".  */
+  Status() = default;
+
+  /** Construct with kind "Success".  */
+  static Status Success() { return Status(); }
+
+  /** Construct with kind "POSIX" using given errno-style value.  */
+  static Status POSIX(int e)
+  {
+    Status s(Kind::POSIX);
+    s.POSIX_ = e;
+    return s;
+  }
+
+  /** Construct with kind "POSIX" using errno.  */
+  static Status POSIX_errno();
+
+#ifdef _WIN32
+  /** Construct with kind "Windows" using given GetLastError()-style value.  */
+  static Status Windows(unsigned int e)
+  {
+    Status s(Kind::Windows);
+    s.Windows_ = e;
+    return s;
+  }
+
+  /** Construct with kind "Windows" using GetLastError().  */
+  static Status Windows_GetLastError();
+#endif
+
+  /** Return true on "Success", false otherwise.  */
+  explicit operator bool() const { return this->Kind_ == Kind::Success; }
+
+  /** Return the kind of status.  */
+  Kind GetKind() const { return this->Kind_; }
+
+  /** If the kind is "POSIX", returns the errno-style value.
+      Otherwise, returns 0.  */
+  int GetPOSIX() const
+  {
+    return this->Kind_ == Kind::POSIX ? this->POSIX_ : 0;
+  }
+
+#ifdef _WIN32
+  /** If the kind is "Windows", returns the GetLastError()-style value.
+      Otherwise, returns 0.  */
+  int GetWindows() const
+  {
+    return this->Kind_ == Kind::Windows ? this->Windows_ : 0;
+  }
+#endif
+
+  /** Return a human-readable description of the status.  */
+  std::string GetString() const;
+
+private:
+  Status(Kind kind)
+    : Kind_(kind)
+  {
+  }
+
+  Kind Kind_ = Kind::Success;
+
+  union
+  {
+    int POSIX_;
+#ifdef _WIN32
+    unsigned int Windows_;
+#endif
+  };
+};
+
+} // namespace @KWSYS_NAMESPACE@
+
+#endif

Source/kwsys/SystemTools.hxx.in

@@ -4,6 +4,7 @@
 #define @KWSYS_NAMESPACE@_SystemTools_hxx
 
 #include <@KWSYS_NAMESPACE@/Configure.hxx>
+#include <@KWSYS_NAMESPACE@/Status.hxx>
 
 #include <iosfwd>
 #include <map>
@@ -339,7 +340,7 @@ public:
   /**
      Change the modification time or create a file
   */
-  static bool Touch(const std::string& filename, bool create);
+  static Status Touch(std::string const& filename, bool create);
 
   /**
    *  Compare file modification times.
@@ -347,8 +348,8 @@ public:
    *  When true is returned, result has -1, 0, +1 for
    *  f1 older, same, or newer than f2.
    */
-  static bool FileTimeCompare(const std::string& f1, const std::string& f2,
-                              int* result);
+  static Status FileTimeCompare(std::string const& f1, std::string const& f2,
+                                int* result);
 
   /**
    *  Get the file extension (including ".") needed for an executable
@@ -507,7 +508,7 @@ public:
    * For windows return the short path for the given path,
    * Unix just a pass through
    */
-  static bool GetShortPath(const std::string& path, std::string& result);
+  static Status GetShortPath(std::string const& path, std::string& result);
 
   /**
    * Read line from file. Make sure to read a full line and truncates it if
@@ -553,16 +554,16 @@ public:
    * can make a full path even if none of the directories existed
    * prior to calling this function.
    */
-  static bool MakeDirectory(const char* path, const mode_t* mode = nullptr);
-  static bool MakeDirectory(const std::string& path,
-                            const mode_t* mode = nullptr);
+  static Status MakeDirectory(const char* path, const mode_t* mode = nullptr);
+  static Status MakeDirectory(std::string const& path,
+                              const mode_t* mode = nullptr);
 
   /**
    * Copy the source file to the destination file only
    * if the two files differ.
    */
-  static bool CopyFileIfDifferent(const std::string& source,
-                                  const std::string& destination);
+  static Status CopyFileIfDifferent(std::string const& source,
+                                    std::string const& destination);
 
   /**
    * Compare the contents of two files.  Return true if different
@@ -580,13 +581,13 @@ public:
   /**
    * Blockwise copy source to destination file
    */