From a865f0beb2e9f8d92e021855c773ab90eaf24581 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 4 Mar 2018 18:59:04 +1100 Subject: Tests: Confirm test working dir set successfully Specifically, this tests that setting WORKING_DIRECTORY to an invalid directory results in the test failing. --- Tests/RunCMake/CMakeLists.txt | 1 + Tests/RunCMake/WorkingDirectory/CMakeLists.txt.in | 3 +++ Tests/RunCMake/WorkingDirectory/CTestConfig.cmake.in | 1 + Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake | 3 +++ Tests/RunCMake/WorkingDirectory/dirNotExist-result.txt | 1 + Tests/RunCMake/WorkingDirectory/dirNotExist-stdout.txt | 10 ++++++++++ Tests/RunCMake/WorkingDirectory/dirNotExist.cmake | 6 ++++++ Tests/RunCMake/WorkingDirectory/test.cmake.in | 15 +++++++++++++++ 8 files changed, 40 insertions(+) create mode 100644 Tests/RunCMake/WorkingDirectory/CMakeLists.txt.in create mode 100644 Tests/RunCMake/WorkingDirectory/CTestConfig.cmake.in create mode 100644 Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake create mode 100644 Tests/RunCMake/WorkingDirectory/dirNotExist-result.txt create mode 100644 Tests/RunCMake/WorkingDirectory/dirNotExist-stdout.txt create mode 100644 Tests/RunCMake/WorkingDirectory/dirNotExist.cmake create mode 100644 Tests/RunCMake/WorkingDirectory/test.cmake.in diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index 31069fa..09a0093 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -188,6 +188,7 @@ if (QT4_FOUND) endif() add_RunCMake_test(CompatibleInterface) add_RunCMake_test(Syntax) +add_RunCMake_test(WorkingDirectory) add_RunCMake_test(add_custom_command) add_RunCMake_test(add_custom_target) diff --git a/Tests/RunCMake/WorkingDirectory/CMakeLists.txt.in b/Tests/RunCMake/WorkingDirectory/CMakeLists.txt.in new file mode 100644 index 0000000..46047b8 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/CMakeLists.txt.in @@ -0,0 +1,3 @@ +cmake_minimum_required(VERSION 3.11) +project(@CASE_NAME@ NONE) +include("@RunCMake_SOURCE_DIR@/@CASE_NAME@.cmake") diff --git a/Tests/RunCMake/WorkingDirectory/CTestConfig.cmake.in b/Tests/RunCMake/WorkingDirectory/CTestConfig.cmake.in new file mode 100644 index 0000000..0226230 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/CTestConfig.cmake.in @@ -0,0 +1 @@ +set(CTEST_PROJECT_NAME "CTestTestWorkingDir.@CASE_NAME@") diff --git a/Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake b/Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake new file mode 100644 index 0000000..a57cacb --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake @@ -0,0 +1,3 @@ +include(RunCTest) + +run_ctest(dirNotExist) diff --git a/Tests/RunCMake/WorkingDirectory/dirNotExist-result.txt b/Tests/RunCMake/WorkingDirectory/dirNotExist-result.txt new file mode 100644 index 0000000..b57e2de --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/dirNotExist-result.txt @@ -0,0 +1 @@ +(-1|255) diff --git a/Tests/RunCMake/WorkingDirectory/dirNotExist-stdout.txt b/Tests/RunCMake/WorkingDirectory/dirNotExist-stdout.txt new file mode 100644 index 0000000..58aa6e4 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/dirNotExist-stdout.txt @@ -0,0 +1,10 @@ +Test project .*/Tests/RunCMake/WorkingDirectory/dirNotExist-build +.* +Start 1: dirNotExist +1/1 Test #1: dirNotExist +\.+\*\*\*Not Run +[0-9.]+ sec ++ +0% tests passed, 1 tests failed out of 1 ++ +Total Test time \(real\) = +[0-9.]+ sec ++ +The following tests FAILED: +.* +1 - dirNotExist \(Not Run\)$ diff --git a/Tests/RunCMake/WorkingDirectory/dirNotExist.cmake b/Tests/RunCMake/WorkingDirectory/dirNotExist.cmake new file mode 100644 index 0000000..642386e --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/dirNotExist.cmake @@ -0,0 +1,6 @@ +include(CTest) + +add_test(NAME dirNotExist + COMMAND ${CMAKE_COMMAND} -E touch someFile.txt + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/thisDirWillNotExist +) diff --git a/Tests/RunCMake/WorkingDirectory/test.cmake.in b/Tests/RunCMake/WorkingDirectory/test.cmake.in new file mode 100644 index 0000000..8eccd79 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/test.cmake.in @@ -0,0 +1,15 @@ +cmake_minimum_required(VERSION 3.11) + +set(CTEST_SITE "test-site") +set(CTEST_BUILD_NAME "test-build-name") +set(CTEST_SOURCE_DIRECTORY "@RunCMake_BINARY_DIR@/@CASE_NAME@") +set(CTEST_BINARY_DIRECTORY "@RunCMake_BINARY_DIR@/@CASE_NAME@-build") +set(CTEST_CMAKE_GENERATOR "@RunCMake_GENERATOR@") +set(CTEST_CMAKE_GENERATOR_PLATFORM "@RunCMake_GENERATOR_PLATFORM@") +set(CTEST_CMAKE_GENERATOR_TOOLSET "@RunCMake_GENERATOR_TOOLSET@") +set(CTEST_BUILD_CONFIGURATION "$ENV{CMAKE_CONFIG_TYPE}") + +ctest_start(Experimental) +ctest_configure() +ctest_build() +ctest_test() -- cgit v0.12 From e654622aee22655c418a9c663fad79243ca0c819 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 4 Mar 2018 21:30:35 +1100 Subject: Tests: Add --build-and-test test case Checks that giving an invalid build directory to ctest --build-and-test will fail. --- Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake | 6 ++++++ Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-check.cmake | 3 +++ Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-result.txt | 1 + Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir.cmake | 7 +++++++ 4 files changed, 17 insertions(+) create mode 100644 Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-check.cmake create mode 100644 Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-result.txt create mode 100644 Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir.cmake diff --git a/Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake b/Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake index a57cacb..a7685ae 100644 --- a/Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake +++ b/Tests/RunCMake/WorkingDirectory/RunCMakeTest.cmake @@ -1,3 +1,9 @@ include(RunCTest) run_ctest(dirNotExist) +run_ctest(buildAndTestNoBuildDir + --build-and-test + ${RunCMake_BINARY_DIR}/buildAndTestNoBuildDir + ${RunCMake_BINARY_DIR}/buildAndTestNoBuildDir/CMakeLists.txt # Deliberately a file + --build-generator "${RunCMake_GENERATOR}" +) diff --git a/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-check.cmake b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-check.cmake new file mode 100644 index 0000000..fcfe461 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-check.cmake @@ -0,0 +1,3 @@ +if(EXISTS ${RunCMake_TEST_BINARY_DIR}/CMakeCache.txt) + set(RunCMake_TEST_FAILED "Default build dir ${RunCMake_TEST_BINARY_DIR} was used, should not have been") +endif() diff --git a/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-result.txt b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-result.txt new file mode 100644 index 0000000..0617a38 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-result.txt @@ -0,0 +1 @@ +^[^0][0-9]*$ diff --git a/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir.cmake b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir.cmake new file mode 100644 index 0000000..ad795c4 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir.cmake @@ -0,0 +1,7 @@ +# We want a single test that always passes. We should never actually get to +# configure with this file, so we use a successful configure-build-test +# sequence to denote failure of the test case. +include(CTest) +add_test(NAME willPass + COMMAND ${CMAKE_COMMAND} -E touch someFile.txt +) -- cgit v0.12 From e60e4dfc88252aaec53f0d832508d41dff6165fd Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 4 Mar 2018 17:27:09 +1100 Subject: cmWorkingDirectory: Check success of current dir changes --- Source/CPack/cmCPackArchiveGenerator.cxx | 16 ++++++++ Source/CPack/cmCPackGenerator.cxx | 8 ++++ Source/CTest/cmCTestBuildAndTestHandler.cxx | 22 ++++++++++- Source/CTest/cmCTestCoverageHandler.cxx | 22 ++++++++++- Source/CTest/cmCTestHandlerCommand.cxx | 16 ++++++++ Source/CTest/cmCTestMultiProcessHandler.cxx | 19 ++++++--- Source/CTest/cmCTestRunTest.cxx | 45 +++++++++++++++++++--- Source/CTest/cmCTestRunTest.h | 2 + Source/CTest/cmCTestSubmitHandler.cxx | 19 +++++++++ Source/CTest/cmCTestTestHandler.cxx | 7 +++- Source/cmGlobalGenerator.cxx | 14 ++++++- Source/cmMakefile.cxx | 10 ++++- Source/cmWorkingDirectory.cxx | 16 +++++++- Source/cmWorkingDirectory.h | 17 ++++++++ Source/cmake.cxx | 11 +++++- .../buildAndTestNoBuildDir-stdout.txt | 1 + .../WorkingDirectory/dirNotExist-stderr.txt | 1 + 17 files changed, 227 insertions(+), 19 deletions(-) create mode 100644 Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt create mode 100644 Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt diff --git a/Source/CPack/cmCPackArchiveGenerator.cxx b/Source/CPack/cmCPackArchiveGenerator.cxx index 9ff547a..00fbdab 100644 --- a/Source/CPack/cmCPackArchiveGenerator.cxx +++ b/Source/CPack/cmCPackArchiveGenerator.cxx @@ -9,6 +9,7 @@ #include "cmSystemTools.h" #include "cmWorkingDirectory.h" +#include #include #include #include @@ -51,6 +52,7 @@ int cmCPackArchiveGenerator::InitializeInternal() this->SetOptionIfNotSet("CPACK_INCLUDE_TOPLEVEL_DIRECTORY", "1"); return this->Superclass::InitializeInternal(); } + int cmCPackArchiveGenerator::addOneComponentToArchive( cmArchiveWrite& archive, cmCPackComponent* component) { @@ -61,6 +63,13 @@ int cmCPackArchiveGenerator::addOneComponentToArchive( localToplevel += "/" + component->Name; // 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); + return 0; + } std::string filePrefix; if (this->IsOn("CPACK_COMPONENT_INCLUDE_TOPLEVEL_DIRECTORY")) { filePrefix = this->GetOption("CPACK_PACKAGE_FILE_NAME"); @@ -237,6 +246,13 @@ int cmCPackArchiveGenerator::PackageFiles() // CASE 3 : NON COMPONENT package. DECLARE_AND_OPEN_ARCHIVE(packageFileNames[0], archive); cmWorkingDirectory workdir(toplevel); + if (workdir.Failed()) { + cmCPackLogger(cmCPackLog::LOG_ERROR, + "Failed to change working directory to " + << toplevel << " : " + << std::strerror(workdir.GetLastResult()) << std::endl); + return 0; + } for (std::string const& file : files) { // Get the relative path to the file std::string rp = cmSystemTools::RelativePath(toplevel, file); diff --git a/Source/CPack/cmCPackGenerator.cxx b/Source/CPack/cmCPackGenerator.cxx index d838b30..d41a9e5 100644 --- a/Source/CPack/cmCPackGenerator.cxx +++ b/Source/CPack/cmCPackGenerator.cxx @@ -6,6 +6,7 @@ #include "cmsys/Glob.hxx" #include "cmsys/RegularExpression.hxx" #include +#include #include // IWYU pragma: keep #include @@ -404,6 +405,13 @@ int cmCPackGenerator::InstallProjectViaInstalledDirectories( cmCPackLogger(cmCPackLog::LOG_DEBUG, "Change dir to: " << goToDir << std::endl); cmWorkingDirectory workdir(goToDir); + if (workdir.Failed()) { + cmCPackLogger( + cmCPackLog::LOG_ERROR, "Failed to change working directory to " + << goToDir << " : " << std::strerror(workdir.GetLastResult()) + << std::endl); + return 0; + } for (auto const& symlinked : symlinkedFiles) { cmCPackLogger(cmCPackLog::LOG_DEBUG, "Will create a symlink: " << symlinked.second << "--> " << symlinked.first diff --git a/Source/CTest/cmCTestBuildAndTestHandler.cxx b/Source/CTest/cmCTestBuildAndTestHandler.cxx index 2e1ea4c..b2c68e7 100644 --- a/Source/CTest/cmCTestBuildAndTestHandler.cxx +++ b/Source/CTest/cmCTestBuildAndTestHandler.cxx @@ -11,6 +11,7 @@ #include "cmsys/Process.h" #include +#include #include #include @@ -196,6 +197,16 @@ int cmCTestBuildAndTestHandler::RunCMakeAndTest(std::string* outstring) cmSystemTools::MakeDirectory(this->BinaryDir); } cmWorkingDirectory workdir(this->BinaryDir); + if (workdir.Failed()) { + auto msg = "Failed to change working directory to " + this->BinaryDir + + " : " + std::strerror(workdir.GetLastResult()) + "\n"; + if (outstring) { + *outstring = msg; + } else { + cmCTestLog(this->CTest, ERROR_MESSAGE, msg); + } + return 1; + } if (this->BuildNoCMake) { // Make the generator available for the Build call below. @@ -307,7 +318,16 @@ int cmCTestBuildAndTestHandler::RunCMakeAndTest(std::string* outstring) // run the test from the this->BuildRunDir if set if (!this->BuildRunDir.empty()) { out << "Run test in directory: " << this->BuildRunDir << "\n"; - cmSystemTools::ChangeDirectory(this->BuildRunDir); + if (!workdir.SetDirectory(this->BuildRunDir)) { + out << "Failed to change working directory : " + << std::strerror(workdir.GetLastResult()) << "\n"; + if (outstring) { + *outstring = out.str(); + } else { + cmCTestLog(this->CTest, ERROR_MESSAGE, out.str()); + } + return 1; + } } out << "Running test command: \"" << fullPath << "\""; for (std::string const& testCommandArg : this->TestCommandArgs) { diff --git a/Source/CTest/cmCTestCoverageHandler.cxx b/Source/CTest/cmCTestCoverageHandler.cxx index 9c66e73..6cf0ac2 100644 --- a/Source/CTest/cmCTestCoverageHandler.cxx +++ b/Source/CTest/cmCTestCoverageHandler.cxx @@ -23,6 +23,7 @@ #include "cmsys/RegularExpression.hxx" #include #include +#include #include #include #include @@ -975,7 +976,12 @@ int cmCTestCoverageHandler::HandleGCovCoverage( std::string testingDir = this->CTest->GetBinaryDir() + "/Testing"; std::string tempDir = testingDir + "/CoverageInfo"; - cmSystemTools::MakeDirectory(tempDir); + if (!cmSystemTools::MakeDirectory(tempDir)) { + cmCTestLog(this->CTest, ERROR_MESSAGE, + "Unable to make directory: " << tempDir << std::endl); + cont->Error++; + return 0; + } cmWorkingDirectory workdir(tempDir); int gcovStyle = 0; @@ -1376,6 +1382,14 @@ int cmCTestCoverageHandler::HandleLCovCoverage( this->Quiet); 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); + cont->Error++; + continue; + } cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "Current coverage dir: " << fileDir << std::endl, @@ -1600,6 +1614,12 @@ bool cmCTestCoverageHandler::FindLCovFiles(std::vector& files) gl.RecurseThroughSymlinksOff(); 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); + return false; + } // Run profmerge to merge all *.dyn files into dpi files if (!cmSystemTools::RunSingleCommand("profmerge")) { diff --git a/Source/CTest/cmCTestHandlerCommand.cxx b/Source/CTest/cmCTestHandlerCommand.cxx index 5a7baf5..1fff2fa 100644 --- a/Source/CTest/cmCTestHandlerCommand.cxx +++ b/Source/CTest/cmCTestHandlerCommand.cxx @@ -9,6 +9,7 @@ #include "cmWorkingDirectory.h" #include "cmake.h" +#include #include #include @@ -218,6 +219,21 @@ 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())); + if (capureCMakeError) { + this->Makefile->AddDefinition(this->Values[ct_CAPTURE_CMAKE_ERROR], + "-1"); + cmCTestLog(this->CTest, ERROR_MESSAGE, this->GetName() + << " " << this->GetError() << "\n"); + // return success because failure is recorded in CAPTURE_CMAKE_ERROR + return true; + } + return false; + } + int res = handler->ProcessHandler(); if (this->Values[ct_RETURN_VALUE] && *this->Values[ct_RETURN_VALUE]) { std::ostringstream str; diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 50c2d86..a8bf1e5 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -151,13 +152,19 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) } } - cmWorkingDirectory workdir(this->Properties[test]->Directory); - - // Lock the resources we'll be using + // Always lock the resources we'll be using, even if we fail to set the + // working directory because FinishTestProcess() will try to unlock them this->LockResources(test); - if (testRun->StartTest(this->Total)) { - return true; + cmWorkingDirectory workdir(this->Properties[test]->Directory); + if (workdir.Failed()) { + testRun->StartFailure("Failed to change working directory to " + + this->Properties[test]->Directory + " : " + + std::strerror(workdir.GetLastResult())); + } else { + if (testRun->StartTest(this->Total)) { + return true; + } } this->FinishTestProcess(testRun, false); @@ -666,6 +673,8 @@ void cmCTestMultiProcessHandler::PrintTestList() count++; cmCTestTestHandler::cmCTestTestProperties& p = *it.second; + // Don't worry if this fails, we are only showing the test list, not + // running the tests cmWorkingDirectory workdir(p.Directory); cmCTestRunTest testRun(*this); diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 30ad38c..32ecdc0 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -14,6 +14,7 @@ #include "cmsys/RegularExpression.hxx" #include #include +#include #include #include #include @@ -248,11 +249,7 @@ bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started) *this->TestHandler->LogFile << "Test time = " << buf << std::endl; } - // Set the working directory to the tests directory to process Dart files. - { - cmWorkingDirectory workdir(this->TestProperties->Directory); - this->DartProcessing(); - } + this->DartProcessing(); // if this is doing MemCheck then all the output needs to be put into // Output since that is what is parsed by cmCTestMemCheckHandler @@ -338,6 +335,13 @@ bool cmCTestRunTest::StartAgain() this->RunAgain = false; // reset // change to tests directory cmWorkingDirectory workdir(this->TestProperties->Directory); + if (workdir.Failed()) { + this->StartFailure("Failed to change working directory to " + + this->TestProperties->Directory + " : " + + std::strerror(workdir.GetLastResult())); + return true; + } + this->StartTest(this->TotalNumberOfTests); return true; } @@ -386,6 +390,37 @@ void cmCTestRunTest::MemCheckPostProcess() handler->PostProcessTest(this->TestResult, this->Index); } +void cmCTestRunTest::StartFailure(std::string const& output) +{ + // Still need to log the Start message so the test summary records our + // attempt to start this test + cmCTestLog(this->CTest, HANDLER_OUTPUT, + std::setw(2 * getNumWidth(this->TotalNumberOfTests) + 8) + << "Start " + << std::setw(getNumWidth(this->TestHandler->GetMaxIndex())) + << this->TestProperties->Index << ": " + << this->TestProperties->Name << std::endl); + + this->ProcessOutput.clear(); + if (!output.empty()) { + *this->TestHandler->LogFile << output << std::endl; + cmCTestLog(this->CTest, ERROR_MESSAGE, output << std::endl); + } + + this->TestResult.Properties = this->TestProperties; + this->TestResult.ExecutionTime = cmDuration::zero(); + this->TestResult.CompressOutput = false; + this->TestResult.ReturnValue = -1; + this->TestResult.CompletionStatus = "Failed to start"; + this->TestResult.Status = cmCTestTestHandler::NOT_RUN; + this->TestResult.TestCount = this->TestProperties->Index; + this->TestResult.Name = this->TestProperties->Name; + this->TestResult.Path = this->TestProperties->Directory; + this->TestResult.Output = output; + this->TestResult.FullCommandLine.clear(); + this->TestProcess = cm::make_unique(*this); +} + // Starts the execution of a test. Returns once it has started bool cmCTestRunTest::StartTest(size_t total) { diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h index 4d57357..256fe92 100644 --- a/Source/CTest/cmCTestRunTest.h +++ b/Source/CTest/cmCTestRunTest.h @@ -74,6 +74,8 @@ public: bool StartAgain(); + void StartFailure(std::string const& output); + cmCTest* GetCTest() const { return this->CTest; } void FinalizeTest(); diff --git a/Source/CTest/cmCTestSubmitHandler.cxx b/Source/CTest/cmCTestSubmitHandler.cxx index 08d05c8..3bab81e 100644 --- a/Source/CTest/cmCTestSubmitHandler.cxx +++ b/Source/CTest/cmCTestSubmitHandler.cxx @@ -7,6 +7,7 @@ #include "cm_jsoncpp_value.h" #include "cmsys/Process.h" #include +#include #include #include #include @@ -1532,6 +1533,15 @@ int cmCTestSubmitHandler::ProcessHandler() // change to the build directory so that we can uses a relative path // on windows since scp doesn't support "c:" a drive in the path cmWorkingDirectory workdir(buildDirectory); + if (workdir.Failed()) { + cmCTestLog(this->CTest, ERROR_MESSAGE, + " Failed to change directory to " + << buildDirectory << " : " + << std::strerror(workdir.GetLastResult()) << std::endl); + ofs << " Failed to change directory to " << buildDirectory << " : " + << std::strerror(workdir.GetLastResult()) << std::endl; + return -1; + } if (!this->SubmitUsingSCP(this->CTest->GetCTestConfiguration("ScpCommand"), "Testing/" + this->CTest->GetCurrentTag(), files, @@ -1551,6 +1561,15 @@ int cmCTestSubmitHandler::ProcessHandler() // change to the build directory so that we can uses a relative path // on windows since scp doesn't support "c:" a drive in the path cmWorkingDirectory workdir(buildDirectory); + if (workdir.Failed()) { + cmCTestLog(this->CTest, ERROR_MESSAGE, + " Failed to change directory to " + << buildDirectory << " : " + << std::strerror(workdir.GetLastResult()) << std::endl); + ofs << " Failed to change directory to " << buildDirectory << " : " + << std::strerror(workdir.GetLastResult()) << std::endl; + return -1; + } cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, " Change directory: " << buildDirectory << std::endl, this->Quiet); diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 84d8926..7ea8fc3 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -14,7 +15,6 @@ #include #include #include -#include #include #include "cmAlgorithms.h" @@ -85,6 +85,11 @@ bool cmCTestSubdirCommand::InitialPass(std::vector const& args, bool readit = false; { cmWorkingDirectory workdir(fname); + if (workdir.Failed()) { + this->SetError("Failed to change directory to " + fname + " : " + + std::strerror(workdir.GetLastResult())); + return false; + } const char* testFilename; if (cmSystemTools::FileExists("CTestTestfile.cmake")) { // does the CTestTestfile.cmake exist ? diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index c805b98..f9eb90f 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -6,11 +6,11 @@ #include "cmsys/FStream.hxx" #include #include +#include #include #include #include #include -#include #if defined(_WIN32) && !defined(__CYGWIN__) #include @@ -1806,6 +1806,8 @@ int cmGlobalGenerator::Build(const std::string& /*unused*/, cmSystemTools::OutputOption outputflag, std::vector const& nativeOptions) { + bool hideconsole = cmSystemTools::GetRunCommandHideConsole(); + /** * Run an executable command and put the stdout in output. */ @@ -1813,9 +1815,17 @@ int cmGlobalGenerator::Build(const std::string& /*unused*/, output += "Change Dir: "; output += bindir; output += "\n"; + if (workdir.Failed()) { + cmSystemTools::SetRunCommandHideConsole(hideconsole); + cmSystemTools::Error("Failed to change directory: ", + std::strerror(workdir.GetLastResult())); + output += "Failed to change directory: "; + output += std::strerror(workdir.GetLastResult()); + output += "\n"; + return 1; + } int retVal; - bool hideconsole = cmSystemTools::GetRunCommandHideConsole(); cmSystemTools::SetRunCommandHideConsole(true); std::string outputBuffer; std::string* outputPtr = &outputBuffer; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 47d75af..a21600d 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -6,12 +6,12 @@ #include "cmsys/RegularExpression.hxx" #include #include +#include #include #include #include // IWYU pragma: keep #include #include -#include #include #include "cmAlgorithms.h" @@ -3250,6 +3250,14 @@ int cmMakefile::TryCompile(const std::string& srcdir, // change to the tests directory and run cmake // use the cmake object instead of calling cmake cmWorkingDirectory workdir(bindir); + if (workdir.Failed()) { + this->IssueMessage(cmake::FATAL_ERROR, + "Failed to set working directory to " + bindir + " : " + + std::strerror(workdir.GetLastResult())); + cmSystemTools::SetFatalErrorOccured(); + this->IsSourceFileTryCompile = false; + return 1; + } // make sure the same generator is used // use this program as the cmake to be run, it should not diff --git a/Source/cmWorkingDirectory.cxx b/Source/cmWorkingDirectory.cxx index 99c9ba8..816f104 100644 --- a/Source/cmWorkingDirectory.cxx +++ b/Source/cmWorkingDirectory.cxx @@ -4,10 +4,12 @@ #include "cmSystemTools.h" +#include + cmWorkingDirectory::cmWorkingDirectory(std::string const& newdir) { this->OldDir = cmSystemTools::GetCurrentWorkingDirectory(); - cmSystemTools::ChangeDirectory(newdir); + this->SetDirectory(newdir); } cmWorkingDirectory::~cmWorkingDirectory() @@ -15,10 +17,20 @@ cmWorkingDirectory::~cmWorkingDirectory() this->Pop(); } +bool cmWorkingDirectory::SetDirectory(std::string const& newdir) +{ + if (cmSystemTools::ChangeDirectory(newdir) == 0) { + this->ResultCode = 0; + return true; + } + this->ResultCode = errno; + return false; +} + void cmWorkingDirectory::Pop() { if (!this->OldDir.empty()) { - cmSystemTools::ChangeDirectory(this->OldDir); + this->SetDirectory(this->OldDir); this->OldDir.clear(); } } diff --git a/Source/cmWorkingDirectory.h b/Source/cmWorkingDirectory.h index aff9267..1f18ce7 100644 --- a/Source/cmWorkingDirectory.h +++ b/Source/cmWorkingDirectory.h @@ -9,6 +9,12 @@ /** \class cmWorkingDirectory * \brief An RAII class to manipulate the working directory. + * + * The current working directory is set to the location given to the + * constructor. The working directory can be changed again as needed + * by calling SetDirectory(). When the object is destroyed, the destructor + * will restore the working directory to what it was when the object was + * created, regardless of any calls to SetDirectory() in the meantime. */ class cmWorkingDirectory { @@ -16,10 +22,21 @@ public: cmWorkingDirectory(std::string const& newdir); ~cmWorkingDirectory(); + bool SetDirectory(std::string const& newdir); void Pop(); + bool Failed() const { return 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 ResultCode; } private: std::string OldDir; + int ResultCode; }; #endif diff --git a/Source/cmake.cxx b/Source/cmake.cxx index f4f4a15..323bcf6 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -98,13 +98,13 @@ #include "cmsys/Glob.hxx" #include "cmsys/RegularExpression.hxx" #include +#include #include #include #include // IWYU pragma: keep #include #include #include -#include #include namespace { @@ -2209,6 +2209,15 @@ int cmake::GetSystemInformation(std::vector& args) { // now run cmake on the CMakeLists file cmWorkingDirectory workdir(destPath); + if (workdir.Failed()) { + // We created the directory and we were able to copy the CMakeLists.txt + // 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()) << std::endl; + return 1; + } std::vector args2; args2.push_back(args[0]); args2.push_back(destPath); diff --git a/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt new file mode 100644 index 0000000..da89317 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/buildAndTestNoBuildDir-stdout.txt @@ -0,0 +1 @@ +Failed to change working directory to .*[/\\]buildAndTestNoBuildDir[/\\]CMakeLists.txt : diff --git a/Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt b/Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt new file mode 100644 index 0000000..3cea890 --- /dev/null +++ b/Tests/RunCMake/WorkingDirectory/dirNotExist-stderr.txt @@ -0,0 +1 @@ +Failed to change working directory to .*[/\\]dirNotExist-build[/\\]thisDirWillNotExist : -- cgit v0.12 From 5901699672cce82d9622a02c4c7d22889029ee0c Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Thu, 8 Mar 2018 20:09:45 +1100 Subject: cmDepends: Remove attempt to change directory that always fails Nothing ever set `CompileDirectory` except `SetDirectory()`, but nothing ever called that function. Therefore, `CompileDirectory` was always empty for the attempt to change directory in `Check()`, which therefore would always fail. Nothing was checking the result and the code was always going to have no effect. --- Source/cmDepends.cxx | 7 +------ Source/cmDepends.h | 6 ------ 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/Source/cmDepends.cxx b/Source/cmDepends.cxx index cdab671..4716e14 100644 --- a/Source/cmDepends.cxx +++ b/Source/cmDepends.cxx @@ -7,7 +7,6 @@ #include "cmLocalGenerator.h" #include "cmMakefile.h" #include "cmSystemTools.h" -#include "cmWorkingDirectory.h" #include "cmsys/FStream.hxx" #include @@ -15,8 +14,7 @@ #include cmDepends::cmDepends(cmLocalGenerator* lg, const char* targetDir) - : CompileDirectory() - , LocalGenerator(lg) + : LocalGenerator(lg) , Verbose(false) , FileComparison(nullptr) , TargetDirectory(targetDir) @@ -73,9 +71,6 @@ bool cmDepends::Finalize(std::ostream& /*unused*/, std::ostream& /*unused*/) bool cmDepends::Check(const char* makeFile, const char* internalFile, std::map& validDeps) { - // Dependency checks must be done in proper working directory. - cmWorkingDirectory workdir(this->CompileDirectory); - // Check whether dependencies must be regenerated. bool okay = true; cmsys::ifstream fin(internalFile); diff --git a/Source/cmDepends.h b/Source/cmDepends.h index a4fee3c..4b9e05a 100644 --- a/Source/cmDepends.h +++ b/Source/cmDepends.h @@ -31,9 +31,6 @@ public: path from the build directory to the target file. */ cmDepends(cmLocalGenerator* lg = nullptr, const char* targetDir = ""); - /** at what level will the compile be done from */ - void SetCompileDirectory(const char* dir) { this->CompileDirectory = dir; } - /** Set the local generator for the directory in which we are scanning dependencies. This is not a full local generator; it has been setup to do relative path conversions for the current @@ -95,9 +92,6 @@ protected: virtual bool Finalize(std::ostream& makeDepends, std::ostream& internalDepends); - // The directory in which the build rule for the target file is executed. - std::string CompileDirectory; - // The local generator. cmLocalGenerator* LocalGenerator; -- cgit v0.12