From 281e9039cb9a69b9afb936f74cfee03e16137047 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 17 Oct 2024 00:31:34 +0200 Subject: [PATCH] cmWorkingDirectory: Unify error messages --- Source/CPack/cmCPackArchiveGenerator.cxx | 11 +---- Source/CPack/cmCPackGenerator.cxx | 6 +-- Source/CTest/cmCTestBuildAndTest.cxx | 7 +-- Source/CTest/cmCTestCoverageHandler.cxx | 10 +---- Source/CTest/cmCTestHandlerCommand.cxx | 5 +-- Source/CTest/cmCTestMultiProcessHandler.cxx | 5 +-- Source/CTest/cmCTestRunTest.cxx | 6 +-- Source/CTest/cmCTestTestHandler.cxx | 4 +- Source/cmCTest.cxx | 44 ++++--------------- Source/cmCTest.h | 3 -- Source/cmFileCommand.cxx | 3 +- Source/cmGlobalGenerator.cxx | 3 +- Source/cmMakefile.cxx | 5 +-- Source/cmWorkingDirectory.cxx | 11 ++--- Source/cmWorkingDirectory.h | 14 ++---- Source/cmake.cxx | 4 +- .../test-dir-non-existing-dir-stderr.txt | 2 +- .../test-dir-non-existing-dir-stdout.txt | 1 - .../buildAndTestNoBuildDir-stdout.txt | 2 +- .../WorkingDirectory/dirNotExist-stderr.txt | 2 +- 20 files changed, 36 insertions(+), 112 deletions(-) delete mode 100644 Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stdout.txt diff --git a/Source/CPack/cmCPackArchiveGenerator.cxx b/Source/CPack/cmCPackArchiveGenerator.cxx index c70a2f1971..e5b9297e55 100644 --- a/Source/CPack/cmCPackArchiveGenerator.cxx +++ b/Source/CPack/cmCPackArchiveGenerator.cxx @@ -2,7 +2,6 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmCPackArchiveGenerator.h" -#include #include #include #include @@ -238,10 +237,7 @@ int cmCPackArchiveGenerator::addOneComponentToArchive( // Change to local toplevel cmWorkingDirectory workdir(localToplevel); if (workdir.Failed()) { - cmCPackLogger(cmCPackLog::LOG_ERROR, - "Failed to change working directory to " - << localToplevel << " : " - << std::strerror(workdir.GetLastResult()) << std::endl); + cmCPackLogger(cmCPackLog::LOG_ERROR, workdir.GetError() << 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, - "Failed to change working directory to " - << this->toplevel << " : " - << std::strerror(workdir.GetLastResult()) << std::endl); + cmCPackLogger(cmCPackLog::LOG_ERROR, workdir.GetError() << std::endl); return 0; } for (std::string const& file : this->files) { diff --git a/Source/CPack/cmCPackGenerator.cxx b/Source/CPack/cmCPackGenerator.cxx index 44d0ece5d6..81539dcfd6 100644 --- a/Source/CPack/cmCPackGenerator.cxx +++ b/Source/CPack/cmCPackGenerator.cxx @@ -3,7 +3,6 @@ #include "cmCPackGenerator.h" #include -#include #include #include @@ -434,10 +433,7 @@ int cmCPackGenerator::InstallProjectViaInstalledDirectories( cmWorkingDirectory workdir(goToDir); if (workdir.Failed()) { cmCPackLogger(cmCPackLog::LOG_ERROR, - "Failed to change working directory to " - << goToDir << " : " - << std::strerror(workdir.GetLastResult()) - << std::endl); + workdir.GetError() << std::endl); return 0; } for (auto const& symlinked : symlinkedFiles) { diff --git a/Source/CTest/cmCTestBuildAndTest.cxx b/Source/CTest/cmCTestBuildAndTest.cxx index 667de3632b..e99cb07e01 100644 --- a/Source/CTest/cmCTestBuildAndTest.cxx +++ b/Source/CTest/cmCTestBuildAndTest.cxx @@ -4,7 +4,6 @@ #include #include -#include #include #include #include @@ -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::strerror(workdir.GetLastResult()) << '\n'; + std::cout << workdir.GetError() << '\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::strerror(workdir.GetLastResult()) << "\n"; + std::cout << workdir.GetError() << '\n'; return 1; } } diff --git a/Source/CTest/cmCTestCoverageHandler.cxx b/Source/CTest/cmCTestCoverageHandler.cxx index cec4a7e59f..edc9280780 100644 --- a/Source/CTest/cmCTestCoverageHandler.cxx +++ b/Source/CTest/cmCTestCoverageHandler.cxx @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -1325,10 +1324,7 @@ int cmCTestCoverageHandler::HandleLCovCoverage( std::string fileDir = cmSystemTools::GetFilenamePath(f); cmWorkingDirectory workdir(fileDir); if (workdir.Failed()) { - cmCTestLog(this->CTest, ERROR_MESSAGE, - "Unable to change working directory to " - << fileDir << " : " - << std::strerror(workdir.GetLastResult()) << std::endl); + cmCTestLog(this->CTest, ERROR_MESSAGE, workdir.GetError() << std::endl); cont->Error++; continue; } @@ -1550,9 +1546,7 @@ bool cmCTestCoverageHandler::FindLCovFiles(std::vector& files) std::string buildDir = this->CTest->GetCTestConfiguration("BuildDirectory"); cmWorkingDirectory workdir(buildDir); if (workdir.Failed()) { - cmCTestLog(this->CTest, ERROR_MESSAGE, - "Unable to change working directory to " << buildDir - << std::endl); + cmCTestLog(this->CTest, ERROR_MESSAGE, workdir.GetError() << std::endl); return false; } diff --git a/Source/CTest/cmCTestHandlerCommand.cxx b/Source/CTest/cmCTestHandlerCommand.cxx index c377d685cd..753ec0f3b8 100644 --- a/Source/CTest/cmCTestHandlerCommand.cxx +++ b/Source/CTest/cmCTestHandlerCommand.cxx @@ -4,7 +4,6 @@ #include #include -#include #include #include @@ -190,9 +189,7 @@ bool cmCTestHandlerCommand::InitialPass(std::vector const& args, cmWorkingDirectory workdir( this->CTest->GetCTestConfiguration("BuildDirectory")); if (workdir.Failed()) { - this->SetError("failed to change directory to " + - this->CTest->GetCTestConfiguration("BuildDirectory") + - " : " + std::strerror(workdir.GetLastResult())); + this->SetError(workdir.GetError()); if (captureCMakeError) { this->Makefile->AddDefinition(this->CaptureCMakeError, "-1"); cmCTestLog(this->CTest, ERROR_MESSAGE, diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 978c16b307..ac7469756e 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -8,7 +8,6 @@ #include #include // IWYU pragma: keep #include -#include #include #include #include @@ -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 " + - this->Properties[test]->Directory + " : " + - std::strerror(workdir.GetLastResult()), + workdir.GetError(), "Failed to change working directory"); return; } diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 483b3b4d7b..326846ebea 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -7,7 +7,6 @@ #include // IWYU pragma: keep #include #include -#include #include #include #include @@ -398,10 +397,7 @@ bool cmCTestRunTest::StartAgain(std::unique_ptr runner, // change to tests directory cmWorkingDirectory workdir(testRun->TestProperties->Directory); if (workdir.Failed()) { - testRun->StartFailure(testRun->TotalNumberOfTests, - "Failed to change working directory to " + - testRun->TestProperties->Directory + " : " + - std::strerror(workdir.GetLastResult()), + testRun->StartFailure(testRun->TotalNumberOfTests, workdir.GetError(), "Failed to change working directory"); return true; } diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index d108271391..eaa06e52ef 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -8,7 +8,6 @@ #include // IWYU pragma: keep #include #include -#include #include #include #include @@ -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 + " : " + - std::strerror(workdir.GetLastResult())); + status.SetError(workdir.GetError()); return false; } const char* testFilename; diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx index f0a2fa550d..bf2f817141 100644 --- a/Source/cmCTest.cxx +++ b/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) { - if (!this->TryToChangeDirectory(parentDir)) { - return ""; - } + cmWorkingDirectory workdir(cmSystemTools::GetParentDirectory(file)); + if (workdir.Failed()) { + 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) { - if (!this->TryToChangeDirectory(workDir)) { - return 1; - } + cmWorkingDirectory changeDir(workDir); + if (changeDir.Failed()) { + 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); diff --git a/Source/cmCTest.h b/Source/cmCTest.h index 70e364a2bb..ef1a71e336 100644 --- a/Source/cmCTest.h +++ b/Source/cmCTest.h @@ -493,9 +493,6 @@ private: int RunScripts(std::vector> const& scripts); int ExecuteTests(); - /** return true iff change directory was successful */ - bool TryToChangeDirectory(std::string const& dir); - struct Private; std::unique_ptr Impl; }; diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 92e6b3e813..964debf0f6 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -3852,8 +3852,7 @@ bool HandleArchiveExtractCommand(std::vector const& args, cmWorkingDirectory workdir(destDir); if (workdir.Failed()) { - status.SetError( - cmStrCat("failed to change working directory to: ", destDir)); + status.SetError(workdir.GetError()); cmSystemTools::SetFatalErrorOccurred(); return false; } diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index 0dc8ad79f1..19fd3ad000 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/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::strerror(workdir.GetLastResult())); + std::string const& err = workdir.GetError(); cmSystemTools::Error(err); ostr << err << std::endl; return 1; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 1785dfb263..9998ca6fc0 100644 --- a/Source/cmMakefile.cxx +++ b/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, - cmStrCat("Failed to set working directory to ", bindir, - " : ", - std::strerror(workdir.GetLastResult()))); + this->IssueMessage(MessageType::FATAL_ERROR, workdir.GetError()); cmSystemTools::SetFatalErrorOccurred(); this->IsSourceFileTryCompile = false; return 1; diff --git a/Source/cmWorkingDirectory.cxx b/Source/cmWorkingDirectory.cxx index 12fae1278c..574699cbde 100644 --- a/Source/cmWorkingDirectory.cxx +++ b/Source/cmWorkingDirectory.cxx @@ -2,8 +2,7 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmWorkingDirectory.h" -#include - +#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)) { - this->ResultCode = 0; + cmsys::Status status = cmSystemTools::ChangeDirectory(newdir); + if (status) { + this->Error.clear(); return true; } - this->ResultCode = errno; + this->Error = cmStrCat("Failed to change working directory to \"", newdir, + "\": ", status.GetString()); return false; } diff --git a/Source/cmWorkingDirectory.h b/Source/cmWorkingDirectory.h index e593621471..82f79bccb6 100644 --- a/Source/cmWorkingDirectory.h +++ b/Source/cmWorkingDirectory.h @@ -26,19 +26,11 @@ public: bool SetDirectory(std::string const& newdir); void Pop(); - bool Failed() const { return this->ResultCode != 0; } - - /** \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; } - + bool Failed() const { return !this->Error.empty(); } + std::string const& GetError() const { return this->Error; } std::string const& GetOldDirectory() const { return this->OldDir; } private: std::string OldDir; - int ResultCode; + std::string Error; }; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index fcee5e56d7..2b7f268ce0 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -3548,8 +3547,7 @@ int cmake::GetSystemInformation(std::vector& 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::strerror(workdir.GetLastResult()) << '\n'; + std::cerr << workdir.GetError() << '\n'; return 1; } std::vector args2; diff --git a/Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stderr.txt b/Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stderr.txt index 017ccb006a..6b16868b21 100644 --- a/Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stderr.txt +++ b/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 diff --git a/Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stdout.txt b/Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stdout.txt deleted file mode 100644 index ddcd238082..0000000000 --- a/Tests/RunCMake/CTestCommandLine/test-dir-non-existing-dir-stdout.txt +++ /dev/null @@ -1 +0,0 @@ -Internal ctest changing into directory: .*/non-existing-dir diff --git a/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt index da893176cc..a0098e7fa5 100644 --- a/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt +++ b/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) diff --git a/Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt b/Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt index 3cea8906da..21d288ebb3 100644 --- a/Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt +++ b/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