From 74092d92bffa26b0e17da638dabcbc462c3f407a Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Thu, 25 Jan 2018 14:04:12 -0500 Subject: cmCTestScriptHandler: Add new field ShouldRunCurrentScript This is to avoid scope issues with CTEST_RUN_CURRENT_SCRIPT. If ctest_start() is called within a function scope, the value of CTEST_RUN_CURRENT_SCRIPT that it sets doesn't make it to the global scope. With this change, ctest_start() no longer sets CTEST_RUN_CURRENT_SCRIPT, and instead sets a field directly in cmCTestScriptHandler. The old behavior of CTEST_RUN_CURRENT_SCRIPT has also been kept for projects and tests that rely on setting it. --- Source/CTest/cmCTestScriptHandler.cxx | 11 +++++++++-- Source/CTest/cmCTestScriptHandler.h | 4 ++++ Source/CTest/cmCTestStartCommand.cxx | 2 +- Source/cmCTest.cxx | 8 ++++++++ Source/cmCTest.h | 2 ++ Tests/RunCMake/ctest_start/FunctionScope-stdout.txt | 1 + Tests/RunCMake/ctest_start/RunCMakeTest.cmake | 2 ++ Tests/RunCMake/ctest_start/test.cmake.in | 10 +++++++++- 8 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 Tests/RunCMake/ctest_start/FunctionScope-stdout.txt diff --git a/Source/CTest/cmCTestScriptHandler.cxx b/Source/CTest/cmCTestScriptHandler.cxx index 716ea10..edc7669 100644 --- a/Source/CTest/cmCTestScriptHandler.cxx +++ b/Source/CTest/cmCTestScriptHandler.cxx @@ -343,6 +343,7 @@ int cmCTestScriptHandler::ReadInScript(const std::string& total_script_arg) this->Makefile->AddDefinition("CMAKE_EXECUTABLE_NAME", cmSystemTools::GetCMakeCommand().c_str()); this->Makefile->AddDefinition("CTEST_RUN_CURRENT_SCRIPT", true); + this->SetRunCurrentScript(true); this->UpdateElapsedTime(); // add the script arg if defined @@ -524,7 +525,8 @@ int cmCTestScriptHandler::RunConfigurationScript( } // only run the curent script if we should - if (this->Makefile && this->Makefile->IsOn("CTEST_RUN_CURRENT_SCRIPT")) { + if (this->Makefile && this->Makefile->IsOn("CTEST_RUN_CURRENT_SCRIPT") && + this->ShouldRunCurrentScript) { return this->RunCurrentScript(); } return result; @@ -535,7 +537,7 @@ int cmCTestScriptHandler::RunCurrentScript() int result; // do not run twice - this->Makefile->AddDefinition("CTEST_RUN_CURRENT_SCRIPT", false); + this->SetRunCurrentScript(false); // no popup widows cmSystemTools::SetRunCommandHideConsole(true); @@ -978,3 +980,8 @@ std::chrono::duration cmCTestScriptHandler::GetRemainingTimeAllowed() std::chrono::steady_clock::now() - this->ScriptStartTime); return (timelimit - duration); } + +void cmCTestScriptHandler::SetRunCurrentScript(bool value) +{ + this->ShouldRunCurrentScript = value; +} diff --git a/Source/CTest/cmCTestScriptHandler.h b/Source/CTest/cmCTestScriptHandler.h index 9b7fa75..844230f 100644 --- a/Source/CTest/cmCTestScriptHandler.h +++ b/Source/CTest/cmCTestScriptHandler.h @@ -106,6 +106,8 @@ public: void CreateCMake(); cmake* GetCMake() { return this->CMake; } + void SetRunCurrentScript(bool value); + private: // reads in a script int ReadInScript(const std::string& total_script_arg); @@ -136,6 +138,8 @@ private: std::vector ConfigurationScripts; std::vector ScriptProcessScope; + bool ShouldRunCurrentScript; + bool Backup; bool EmptyBinDir; bool EmptyBinDirOnce; diff --git a/Source/CTest/cmCTestStartCommand.cxx b/Source/CTest/cmCTestStartCommand.cxx index 4f0d87b..38ee623 100644 --- a/Source/CTest/cmCTestStartCommand.cxx +++ b/Source/CTest/cmCTestStartCommand.cxx @@ -126,7 +126,7 @@ bool cmCTestStartCommand::InitialPass(std::vector const& args, return false; } - this->Makefile->AddDefinition("CTEST_RUN_CURRENT_SCRIPT", "OFF"); + this->CTest->SetRunCurrentScript(false); this->CTest->SetSuppressUpdatingCTestConfiguration(true); int model = this->CTest->GetTestModelFromString(smodel); this->CTest->SetTestModel(model); diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx index fd7c5e8..25e0bd0 100644 --- a/Source/cmCTest.cxx +++ b/Source/cmCTest.cxx @@ -2805,6 +2805,14 @@ std::chrono::duration cmCTest::MaxDuration() return std::chrono::duration(1.0e7); } +void cmCTest::SetRunCurrentScript(bool value) +{ + cmCTestScriptHandler* ch = + static_cast(this->GetHandler("script")); + + ch->SetRunCurrentScript(value); +} + void cmCTest::OutputTestErrors(std::vector const& process_output) { std::string test_outputs("\n*** Test Failed:\n"); diff --git a/Source/cmCTest.h b/Source/cmCTest.h index 61487f1..e9642c9 100644 --- a/Source/cmCTest.h +++ b/Source/cmCTest.h @@ -462,6 +462,8 @@ public: void GenerateSubprojectsOutput(cmXMLWriter& xml); std::vector GetLabelsForSubprojects(); + void SetRunCurrentScript(bool value); + private: int RepeatTests; bool RepeatUntilFail; diff --git a/Tests/RunCMake/ctest_start/FunctionScope-stdout.txt b/Tests/RunCMake/ctest_start/FunctionScope-stdout.txt new file mode 100644 index 0000000..10f3293 --- /dev/null +++ b/Tests/RunCMake/ctest_start/FunctionScope-stdout.txt @@ -0,0 +1 @@ +^$ diff --git a/Tests/RunCMake/ctest_start/RunCMakeTest.cmake b/Tests/RunCMake/ctest_start/RunCMakeTest.cmake index d630a79..bf47256 100644 --- a/Tests/RunCMake/ctest_start/RunCMakeTest.cmake +++ b/Tests/RunCMake/ctest_start/RunCMakeTest.cmake @@ -11,6 +11,8 @@ run_ctest_start(StartQuiet Experimental QUIET) run_ctest_start(ConfigInSource Experimental) +run_ctest_start(FunctionScope Experimental QUIET) + function(run_ConfigInBuild) set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/ConfigInBuild-build) set(RunCMake_TEST_NO_CLEAN 1) diff --git a/Tests/RunCMake/ctest_start/test.cmake.in b/Tests/RunCMake/ctest_start/test.cmake.in index 21e3fad..0a27942 100644 --- a/Tests/RunCMake/ctest_start/test.cmake.in +++ b/Tests/RunCMake/ctest_start/test.cmake.in @@ -9,5 +9,13 @@ set(CTEST_CMAKE_GENERATOR_PLATFORM "@RunCMake_GENERATOR_PLATFORM@") set(CTEST_CMAKE_GENERATOR_TOOLSET "@RunCMake_GENERATOR_TOOLSET@") set(CTEST_BUILD_CONFIGURATION "$ENV{CMAKE_CONFIG_TYPE}") +function(setup_tests) + ctest_start(${ctest_start_args}) +endfunction() + set(ctest_start_args "@CASE_CTEST_START_ARGS@") -ctest_start(${ctest_start_args}) +if("@CASE_NAME@" STREQUAL "FunctionScope") + setup_tests() +else() + ctest_start(${ctest_start_args}) +endif() -- cgit v0.12 From 13347740e2fe00ad51493c89087f1bbbc35b224c Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Thu, 25 Jan 2018 14:24:09 -0500 Subject: Help: add release notes, documentation for CTEST_RUN_CURRENT_SCRIPT behavior --- Help/manual/cmake-variables.7.rst | 1 + Help/release/dev/ctest_start_function_scope.rst | 8 ++++++++ Help/variable/CTEST_RUN_CURRENT_SCRIPT.rst | 5 +++++ 3 files changed, 14 insertions(+) create mode 100644 Help/release/dev/ctest_start_function_scope.rst create mode 100644 Help/variable/CTEST_RUN_CURRENT_SCRIPT.rst diff --git a/Help/manual/cmake-variables.7.rst b/Help/manual/cmake-variables.7.rst index c18d8da..3ac5123 100644 --- a/Help/manual/cmake-variables.7.rst +++ b/Help/manual/cmake-variables.7.rst @@ -513,6 +513,7 @@ Variables for CTest /variable/CTEST_P4_COMMAND /variable/CTEST_P4_OPTIONS /variable/CTEST_P4_UPDATE_OPTIONS + /variable/CTEST_RUN_CURRENT_SCRIPT /variable/CTEST_SCP_COMMAND /variable/CTEST_SITE /variable/CTEST_SOURCE_DIRECTORY diff --git a/Help/release/dev/ctest_start_function_scope.rst b/Help/release/dev/ctest_start_function_scope.rst new file mode 100644 index 0000000..f949c2b --- /dev/null +++ b/Help/release/dev/ctest_start_function_scope.rst @@ -0,0 +1,8 @@ +ctest_start_function_scope +-------------------------- + +* The :command:`ctest_start` command no longer sets + :variable:`CTEST_RUN_CURRENT_SCRIPT` due to issues with scoping if it is + called from inside a function. Instead, it sets an internal variable in + CTest. However, setting :variable:`CTEST_RUN_CURRENT_SCRIPT` to 0 at the + global scope still prevents the script from being re-run at the end. diff --git a/Help/variable/CTEST_RUN_CURRENT_SCRIPT.rst b/Help/variable/CTEST_RUN_CURRENT_SCRIPT.rst new file mode 100644 index 0000000..abc123c --- /dev/null +++ b/Help/variable/CTEST_RUN_CURRENT_SCRIPT.rst @@ -0,0 +1,5 @@ +CTEST_RUN_CURRENT_SCRIPT +------------------------ + +Setting this to 0 prevents :manual:`ctest(1)` from being run again when it +reaches the end of a script run by calling ``ctest -S``. -- cgit v0.12