cmWorkingDirectory: Unify error messages

Source/CPack/cmCPackArchiveGenerator.cxx

@@ -2,7 +2,6 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmCPackArchiveGenerator.h"
 
-#include <cstring>
 #include <map>
 #include <ostream>
 #include <unordered_map>
@@ -238,10 +237,7 @@ int cmCPackArchiveGenerator::addOneComponentToArchive(
   // Change to local toplevel
   cmWorkingDirectory workdir(localToplevel);
   if (workdir.Failed()) {
-    cmCPackLogger(cmCPackLog::LOG_ERROR,
+    cmCPackLogger(cmCPackLog::LOG_ERROR, workdir.GetError() << std::endl);
-                  "Failed to change working directory to "
-                    << localToplevel << " : "
-                    << std::strerror(workdir.GetLastResult()) << std::endl);
     return 0;
   }
   std::string filePrefix;
@@ -448,10 +444,7 @@ int cmCPackArchiveGenerator::PackageFiles()
   DECLARE_AND_OPEN_ARCHIVE(packageFileNames[0], archive);
   cmWorkingDirectory workdir(this->toplevel);
   if (workdir.Failed()) {
-    cmCPackLogger(cmCPackLog::LOG_ERROR,
+    cmCPackLogger(cmCPackLog::LOG_ERROR, workdir.GetError() << std::endl);
-                  "Failed to change working directory to "
-                    << this->toplevel << " : "
-                    << std::strerror(workdir.GetLastResult()) << std::endl);
     return 0;
   }
   for (std::string const& file : this->files) {

Source/CPack/cmCPackGenerator.cxx

@@ -3,7 +3,6 @@
 #include "cmCPackGenerator.h"
 
 #include <algorithm>
-#include <cstring>
 #include <memory>
 #include <utility>
 
@@ -434,10 +433,7 @@ int cmCPackGenerator::InstallProjectViaInstalledDirectories(
         cmWorkingDirectory workdir(goToDir);
         if (workdir.Failed()) {
           cmCPackLogger(cmCPackLog::LOG_ERROR,
-                        "Failed to change working directory to "
+                        workdir.GetError() << std::endl);
-                          << goToDir << " : "
-                          << std::strerror(workdir.GetLastResult())
-                          << std::endl);
           return 0;
         }
         for (auto const& symlinked : symlinkedFiles) {

Source/CTest/cmCTestBuildAndTest.cxx

@@ -4,7 +4,6 @@
 
 #include <chrono>
 #include <cstdint>
-#include <cstring>
 #include <iostream>
 #include <ratio>
 #include <utility>
@@ -210,8 +209,7 @@ int cmCTestBuildAndTest::Run()
   }
   cmWorkingDirectory workdir(this->BinaryDir);
   if (workdir.Failed()) {
-    std::cout << "Failed to change working directory to " << this->BinaryDir
+    std::cout << workdir.GetError() << '\n';
-              << " : " << std::strerror(workdir.GetLastResult()) << '\n';
     return 1;
   }
 
@@ -311,8 +309,7 @@ int cmCTestBuildAndTest::Run()
   if (!this->BuildRunDir.empty()) {
     std::cout << "Run test in directory: " << this->BuildRunDir << "\n";
     if (!workdir.SetDirectory(this->BuildRunDir)) {
-      std::cout << "Failed to change working directory : "
+      std::cout << workdir.GetError() << '\n';
-                << std::strerror(workdir.GetLastResult()) << "\n";
       return 1;
     }
   }

Source/CTest/cmCTestCoverageHandler.cxx

@@ -6,7 +6,6 @@
 #include <chrono>
 #include <cstdio>
 #include <cstdlib>
-#include <cstring>
 #include <iomanip>
 #include <iterator>
 #include <memory>
@@ -1325,10 +1324,7 @@ int cmCTestCoverageHandler::HandleLCovCoverage(
     std::string fileDir = cmSystemTools::GetFilenamePath(f);
     cmWorkingDirectory workdir(fileDir);
     if (workdir.Failed()) {
-      cmCTestLog(this->CTest, ERROR_MESSAGE,
+      cmCTestLog(this->CTest, ERROR_MESSAGE, workdir.GetError() << std::endl);
-                 "Unable to change working directory to "
-                   << fileDir << " : "
-                   << std::strerror(workdir.GetLastResult()) << std::endl);
       cont->Error++;
       continue;
     }
@@ -1550,9 +1546,7 @@ bool cmCTestCoverageHandler::FindLCovFiles(std::vector<std::string>& files)
   std::string buildDir = this->CTest->GetCTestConfiguration("BuildDirectory");
   cmWorkingDirectory workdir(buildDir);
   if (workdir.Failed()) {
-    cmCTestLog(this->CTest, ERROR_MESSAGE,
+    cmCTestLog(this->CTest, ERROR_MESSAGE, workdir.GetError() << std::endl);
-               "Unable to change working directory to " << buildDir
-                                                        << std::endl);
     return false;
   }
 

Source/CTest/cmCTestHandlerCommand.cxx

@@ -4,7 +4,6 @@
 
 #include <algorithm>
 #include <cstdlib>
-#include <cstring>
 #include <sstream>
 
 #include <cm/string_view>
@@ -190,9 +189,7 @@ bool cmCTestHandlerCommand::InitialPass(std::vector<std::string> const& args,
   cmWorkingDirectory workdir(
     this->CTest->GetCTestConfiguration("BuildDirectory"));
   if (workdir.Failed()) {
-    this->SetError("failed to change directory to " +
+    this->SetError(workdir.GetError());
-                   this->CTest->GetCTestConfiguration("BuildDirectory") +
-                   " : " + std::strerror(workdir.GetLastResult()));
     if (captureCMakeError) {
       this->Makefile->AddDefinition(this->CaptureCMakeError, "-1");
       cmCTestLog(this->CTest, ERROR_MESSAGE,

Source/CTest/cmCTestMultiProcessHandler.cxx

@@ -8,7 +8,6 @@
 #include <cmath>
 #include <cstddef> // IWYU pragma: keep
 #include <cstdlib>
-#include <cstring>
 #include <iomanip>
 #include <iostream>
 #include <list>
@@ -279,9 +278,7 @@ void cmCTestMultiProcessHandler::StartTestProcess(int test)
   cmWorkingDirectory workdir(this->Properties[test]->Directory);
   if (workdir.Failed()) {
     cmCTestRunTest::StartFailure(std::move(testRun), this->Total,
-                                 "Failed to change working directory to " +
+                                 workdir.GetError(),
-                                   this->Properties[test]->Directory + " : " +
-                                   std::strerror(workdir.GetLastResult()),
                                  "Failed to change working directory");
     return;
   }

Source/CTest/cmCTestRunTest.cxx

@@ -7,7 +7,6 @@
 #include <cstddef> // IWYU pragma: keep
 #include <cstdint>
 #include <cstdio>
-#include <cstring>
 #include <iomanip>
 #include <ratio>
 #include <sstream>
@@ -398,10 +397,7 @@ bool cmCTestRunTest::StartAgain(std::unique_ptr<cmCTestRunTest> runner,
   // change to tests directory
   cmWorkingDirectory workdir(testRun->TestProperties->Directory);
   if (workdir.Failed()) {
-    testRun->StartFailure(testRun->TotalNumberOfTests,
+    testRun->StartFailure(testRun->TotalNumberOfTests, workdir.GetError(),
-                          "Failed to change working directory to " +
-                            testRun->TestProperties->Directory + " : " +
-                            std::strerror(workdir.GetLastResult()),
                           "Failed to change working directory");
     return true;
   }

Source/CTest/cmCTestTestHandler.cxx

@@ -8,7 +8,6 @@
 #include <cstddef> // IWYU pragma: keep
 #include <cstdio>
 #include <cstdlib>
-#include <cstring>
 #include <ctime>
 #include <functional>
 #include <iomanip>
@@ -97,8 +96,7 @@ bool ReadSubdirectory(std::string fname, cmExecutionStatus& status)
   {
     cmWorkingDirectory workdir(fname);
     if (workdir.Failed()) {
-      status.SetError("Failed to change directory to " + fname + " : " +
+      status.SetError(workdir.GetError());
-                      std::strerror(workdir.GetLastResult()));
       return false;
     }
     const char* testFilename;

Source/cmCTest.cxx

@@ -73,6 +73,7 @@
 #include "cmValue.h"
 #include "cmVersion.h"
 #include "cmVersionConfig.h"
+#include "cmWorkingDirectory.h"
 #include "cmXMLWriter.h"
 #include "cmake.h"
 
@@ -1480,31 +1481,14 @@ int cmCTest::GenerateDoneFile()
   return 0;
 }
 
-bool cmCTest::TryToChangeDirectory(std::string const& dir)
-{
-  cmCTestLog(this, OUTPUT,
-             "Internal ctest changing into directory: " << dir << std::endl);
-  cmsys::Status status = cmSystemTools::ChangeDirectory(dir);
-  if (!status) {
-    auto msg = "Failed to change working directory to \"" + dir +
-      "\" : " + status.GetString() + "\n";
-    cmCTestLog(this, ERROR_MESSAGE, msg);
-    return false;
-  }
-  return true;
-}
-
 std::string cmCTest::Base64GzipEncodeFile(std::string const& file)
 {
-  const std::string currDir = cmSystemTools::GetCurrentWorkingDirectory();
-  std::string parentDir = cmSystemTools::GetParentDirectory(file);
-
   // Temporarily change to the file's directory so the tar gets created
   // with a flat directory structure.
-  if (currDir != parentDir) {
+  cmWorkingDirectory workdir(cmSystemTools::GetParentDirectory(file));
-    if (!this->TryToChangeDirectory(parentDir)) {
+  if (workdir.Failed()) {
-      return "";
+    cmCTestLog(this, ERROR_MESSAGE, workdir.GetError() << std::endl);
-    }
+    return "";
   }
 
   std::string tarFile = file + "_temp.tar.gz";
@@ -1521,12 +1505,6 @@ std::string cmCTest::Base64GzipEncodeFile(std::string const& file)
   }
   std::string base64 = this->Base64EncodeFile(tarFile);
   cmSystemTools::RemoveFile(tarFile);
-
-  // Change back to the directory we started in.
-  if (currDir != parentDir) {
-    cmSystemTools::ChangeDirectory(currDir);
-  }
-
   return base64;
 }
 
@@ -2980,10 +2958,10 @@ int cmCTest::ExecuteTests()
     workDir = cmSystemTools::CollapseFullPath(this->Impl->TestDir);
   }
 
-  if (currDir != workDir) {
+  cmWorkingDirectory changeDir(workDir);
-    if (!this->TryToChangeDirectory(workDir)) {
+  if (changeDir.Failed()) {
-      return 1;
+    cmCTestLog(this, ERROR_MESSAGE, changeDir.GetError() << std::endl);
-    }
+    return 1;
   }
 
   if (!this->Initialize(workDir, nullptr)) {
@@ -2994,10 +2972,6 @@ int cmCTest::ExecuteTests()
     res = this->ProcessSteps();
   }
 
-  if (currDir != workDir) {
-    cmSystemTools::ChangeDirectory(currDir);
-  }
-
   if (res != 0) {
     cmCTestLog(this, DEBUG,
                "Running a test(s) failed returning : " << res << std::endl);

Source/cmCTest.h

@@ -493,9 +493,6 @@ private:
   int RunScripts(std::vector<std::pair<std::string, bool>> const& scripts);
   int ExecuteTests();
 
-  /** return true iff change directory was successful */
-  bool TryToChangeDirectory(std::string const& dir);
-
   struct Private;
   std::unique_ptr<Private> Impl;
 };

Source/cmFileCommand.cxx

@@ -3852,8 +3852,7 @@ bool HandleArchiveExtractCommand(std::vector<std::string> const& args,
 
     cmWorkingDirectory workdir(destDir);
     if (workdir.Failed()) {
-      status.SetError(
+      status.SetError(workdir.GetError());
-        cmStrCat("failed to change working directory to: ", destDir));
       cmSystemTools::SetFatalErrorOccurred();
       return false;
     }

Source/cmGlobalGenerator.cxx

@@ -2255,8 +2255,7 @@ int cmGlobalGenerator::Build(
   ostr << "Change Dir: '" << bindir << '\'' << std::endl;
   if (workdir.Failed()) {
     cmSystemTools::SetRunCommandHideConsole(hideconsole);
-    std::string err = cmStrCat("Failed to change directory: ",
+    std::string const& err = workdir.GetError();
-                               std::strerror(workdir.GetLastResult()));
     cmSystemTools::Error(err);
     ostr << err << std::endl;
     return 1;

Source/cmMakefile.cxx

@@ -3722,10 +3722,7 @@ int cmMakefile::TryCompile(const std::string& srcdir,
   // use the cmake object instead of calling cmake
   cmWorkingDirectory workdir(bindir);
   if (workdir.Failed()) {
-    this->IssueMessage(MessageType::FATAL_ERROR,
+    this->IssueMessage(MessageType::FATAL_ERROR, workdir.GetError());
-                       cmStrCat("Failed to set working directory to ", bindir,
-                                " : ",
-                                std::strerror(workdir.GetLastResult())));
     cmSystemTools::SetFatalErrorOccurred();
     this->IsSourceFileTryCompile = false;
     return 1;

Source/cmWorkingDirectory.cxx

@@ -2,8 +2,7 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmWorkingDirectory.h"
 
-#include <cerrno>
+#include "cmStringAlgorithms.h"
-
 #include "cmSystemTools.h"
 
 cmWorkingDirectory::cmWorkingDirectory(std::string const& newdir)
@@ -19,11 +18,13 @@ cmWorkingDirectory::~cmWorkingDirectory()
 
 bool cmWorkingDirectory::SetDirectory(std::string const& newdir)
 {
-  if (cmSystemTools::ChangeDirectory(newdir)) {
+  cmsys::Status status = cmSystemTools::ChangeDirectory(newdir);
-    this->ResultCode = 0;
+  if (status) {
+    this->Error.clear();
     return true;
   }
-  this->ResultCode = errno;
+  this->Error = cmStrCat("Failed to change working directory to \"", newdir,
+                         "\": ", status.GetString());
   return false;
 }
 

Source/cmWorkingDirectory.h

@@ -26,19 +26,11 @@ public:
 
   bool SetDirectory(std::string const& newdir);
   void Pop();
-  bool Failed() const { return this->ResultCode != 0; }
+  bool Failed() const { return !this->Error.empty(); }
-
+  std::string const& GetError() const { return this->Error; }
-  /** \return 0 if the last attempt to set the working directory was
-   *          successful. If it failed, the value returned will be the
-   *          \c errno value associated with the failure. A description
-   *          of the error code can be obtained by passing the result
-   *          to \c std::strerror().
-   */
-  int GetLastResult() const { return this->ResultCode; }
-
   std::string const& GetOldDirectory() const { return this->OldDir; }
 
 private:
   std::string OldDir;
-  int ResultCode;
+  std::string Error;
 };

Source/cmake.cxx

@@ -7,7 +7,6 @@
 #include <chrono>
 #include <cstdio>
 #include <cstdlib>
-#include <cstring>
 #include <initializer_list>
 #include <iomanip>
 #include <iostream>
@@ -3548,8 +3547,7 @@ int cmake::GetSystemInformation(std::vector<std::string>& args)
       // file to it, so we wouldn't expect to get here unless the default
       // permissions are questionable or some other process has deleted the
       // directory
-      std::cerr << "Failed to change to directory " << destPath << " : "
+      std::cerr << workdir.GetError() << '\n';
-                << std::strerror(workdir.GetLastResult()) << '\n';
       return 1;
     }
     std::vector<std::string> args2;

Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stderr.txt

@@ -1 +1 @@
-Failed to change working directory to ".*/non-existing-dir" : No such file or directory
+Failed to change working directory to ".*/non-existing-dir": No such file or directory

Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stdout.txt

@@ -1 +0,0 @@
-Internal ctest changing into directory: .*/non-existing-dir

Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt

@@ -1 +1 @@
-Failed to change working directory to .*[/\\]buildAndTestNoBuildDir[/\\]CMakeLists.txt :
+Failed to change working directory to ".*[/\\]buildAndTestNoBuildDir[/\\]CMakeLists.txt": (Not a directory|Invalid argument)

Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt

@@ -1 +1 @@
-Failed to change working directory to .*[/\\]dirNotExist-build[/\\]thisDirWillNotExist :
+Failed to change working directory to ".*[/\\]dirNotExist-build[/\\]thisDirWillNotExist": No such file or directory