From 81e904bd533ecbd0f50fe69969135d8e9ee2a46f Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Wed, 23 Oct 2024 16:48:44 +0200 Subject: [PATCH] cmCTestScriptHandler: Don't inherit from cmCTestGenericHandler Add `cmake*` and `cmCTest*` instances and arguments where needed, such that `GetScriptHandler` does not have to be called. --- Source/CTest/cmCTestBuildHandler.cxx | 2 +- Source/CTest/cmCTestConfigureHandler.cxx | 2 +- Source/CTest/cmCTestCoverageHandler.cxx | 6 ++-- Source/CTest/cmCTestGenericHandler.h | 4 +++ Source/CTest/cmCTestHandlerCommand.cxx | 1 + Source/CTest/cmCTestMemCheckHandler.cxx | 2 +- Source/CTest/cmCTestScriptHandler.cxx | 16 ++------- Source/CTest/cmCTestScriptHandler.h | 15 +++----- Source/CTest/cmCTestSubmitCommand.cxx | 3 +- Source/CTest/cmCTestSubmitHandler.cxx | 10 ++---- Source/CTest/cmCTestTestHandler.cxx | 2 +- Source/CTest/cmCTestUploadHandler.cxx | 2 +- Source/cmCTest.cxx | 44 ++++++++---------------- Source/cmCTest.h | 13 ++++--- 14 files changed, 46 insertions(+), 76 deletions(-) diff --git a/Source/CTest/cmCTestBuildHandler.cxx b/Source/CTest/cmCTestBuildHandler.cxx index 13cbb5b1f8..4a701145b2 100644 --- a/Source/CTest/cmCTestBuildHandler.cxx +++ b/Source/CTest/cmCTestBuildHandler.cxx @@ -502,7 +502,7 @@ int cmCTestBuildHandler::ProcessHandler() void cmCTestBuildHandler::GenerateXMLHeader(cmXMLWriter& xml) { - this->CTest->StartXML(xml, this->AppendXML); + this->CTest->StartXML(xml, this->CMake, this->AppendXML); this->CTest->GenerateSubprojectsOutput(xml); xml.StartElement("Build"); xml.Element("StartDateTime", this->StartBuild); diff --git a/Source/CTest/cmCTestConfigureHandler.cxx b/Source/CTest/cmCTestConfigureHandler.cxx index d16ab54e21..e91f98eaa4 100644 --- a/Source/CTest/cmCTestConfigureHandler.cxx +++ b/Source/CTest/cmCTestConfigureHandler.cxx @@ -71,7 +71,7 @@ int cmCTestConfigureHandler::ProcessHandler() if (os) { cmXMLWriter xml(os); - this->CTest->StartXML(xml, this->AppendXML); + this->CTest->StartXML(xml, this->CMake, this->AppendXML); this->CTest->GenerateSubprojectsOutput(xml); xml.StartElement("Configure"); xml.Element("StartDateTime", start_time); diff --git a/Source/CTest/cmCTestCoverageHandler.cxx b/Source/CTest/cmCTestCoverageHandler.cxx index 9fc4f70b05..c13fc5fe42 100644 --- a/Source/CTest/cmCTestCoverageHandler.cxx +++ b/Source/CTest/cmCTestCoverageHandler.cxx @@ -99,7 +99,7 @@ void cmCTestCoverageHandler::EndCoverageLogFile(cmGeneratedFileStream& ostr, void cmCTestCoverageHandler::StartCoverageLogXML(cmXMLWriter& xml) { - this->CTest->StartXML(xml, this->AppendXML); + this->CTest->StartXML(xml, this->CMake, this->AppendXML); xml.StartElement("CoverageLog"); xml.Element("StartDateTime", this->CTest->CurrentTime()); xml.Element("StartTime", std::chrono::system_clock::now()); @@ -330,7 +330,7 @@ int cmCTestCoverageHandler::ProcessHandler() covSumFile.setf(std::ios::fixed, std::ios::floatfield); covSumFile.precision(2); - this->CTest->StartXML(covSumXML, this->AppendXML); + this->CTest->StartXML(covSumXML, this->CMake, this->AppendXML); // Produce output xml files covSumXML.StartElement("Coverage"); @@ -1909,7 +1909,7 @@ int cmCTestCoverageHandler::RunBullseyeSourceSummary( "Cannot open coverage summary file." << std::endl); return 0; } - this->CTest->StartXML(xml, this->AppendXML); + this->CTest->StartXML(xml, this->CMake, this->AppendXML); auto elapsed_time_start = std::chrono::steady_clock::now(); std::string coverage_start_time = this->CTest->CurrentTime(); xml.StartElement("Coverage"); diff --git a/Source/CTest/cmCTestGenericHandler.h b/Source/CTest/cmCTestGenericHandler.h index d5d2b41c05..e23d8596b3 100644 --- a/Source/CTest/cmCTestGenericHandler.h +++ b/Source/CTest/cmCTestGenericHandler.h @@ -12,6 +12,7 @@ class cmGeneratedFileStream; class cmMakefile; +class cmake; /** \class cmCTestGenericHandler * \brief A superclass of all CTest Handlers @@ -67,6 +68,8 @@ public: void SetTestLoad(unsigned long load) { this->TestLoad = load; } unsigned long GetTestLoad() const { return this->TestLoad; } + void SetCMakeInstance(cmake* cm) { this->CMake = cm; } + protected: bool StartResultingXML(cmCTest::Part part, const char* name, cmGeneratedFileStream& xofs); @@ -78,6 +81,7 @@ protected: cmSystemTools::OutputOption HandlerVerbose; cmCTest* CTest; t_StringToString LogFileNames; + cmake* CMake = nullptr; int SubmitIndex; }; diff --git a/Source/CTest/cmCTestHandlerCommand.cxx b/Source/CTest/cmCTestHandlerCommand.cxx index 36dbbcb7eb..2af0613b31 100644 --- a/Source/CTest/cmCTestHandlerCommand.cxx +++ b/Source/CTest/cmCTestHandlerCommand.cxx @@ -202,6 +202,7 @@ bool cmCTestHandlerCommand::InitialPass(std::vector const& args, // reread time limit, as the variable may have been modified. this->CTest->SetTimeLimit(this->Makefile->GetDefinition("CTEST_TIME_LIMIT")); + handler->SetCMakeInstance(this->Makefile->GetCMakeInstance()); int res = handler->ProcessHandler(); if (!this->ReturnValue.empty()) { diff --git a/Source/CTest/cmCTestMemCheckHandler.cxx b/Source/CTest/cmCTestMemCheckHandler.cxx index 83fd1eea21..dd2846deaf 100644 --- a/Source/CTest/cmCTestMemCheckHandler.cxx +++ b/Source/CTest/cmCTestMemCheckHandler.cxx @@ -311,7 +311,7 @@ void cmCTestMemCheckHandler::GenerateCTestXML(cmXMLWriter& xml) if (!this->CTest->GetProduceXML()) { return; } - this->CTest->StartXML(xml, this->AppendXML); + this->CTest->StartXML(xml, this->CMake, this->AppendXML); this->CTest->GenerateSubprojectsOutput(xml); xml.StartElement("DynamicAnalysis"); switch (this->MemoryTesterStyle) { diff --git a/Source/CTest/cmCTestScriptHandler.cxx b/Source/CTest/cmCTestScriptHandler.cxx index f5185f7783..953f116c70 100644 --- a/Source/CTest/cmCTestScriptHandler.cxx +++ b/Source/CTest/cmCTestScriptHandler.cxx @@ -40,18 +40,9 @@ #include "cmUVProcessChain.h" #include "cmake.h" -cmCTestScriptHandler::cmCTestScriptHandler() = default; - -void cmCTestScriptHandler::Initialize(cmCTest* ctest) +cmCTestScriptHandler::cmCTestScriptHandler(cmCTest* ctest) + : CTest(ctest) { - this->Superclass::Initialize(ctest); - - this->Makefile.reset(); - this->ParentMakefile = nullptr; - - this->GlobalGenerator.reset(); - - this->CMake.reset(); } cmCTestScriptHandler::~cmCTestScriptHandler() = default; @@ -355,8 +346,7 @@ bool cmCTestScriptHandler::RunScript(cmCTest* ctest, cmMakefile* mf, const std::string& sname, bool InProcess, int* returnValue) { - auto sh = cm::make_unique(); - sh->Initialize(ctest); + auto sh = cm::make_unique(ctest); sh->ParentMakefile = mf; sh->AddConfigurationScript(sname, InProcess); int res = sh->ProcessHandler(); diff --git a/Source/CTest/cmCTestScriptHandler.h b/Source/CTest/cmCTestScriptHandler.h index 0b9fc798b3..109dac4823 100644 --- a/Source/CTest/cmCTestScriptHandler.h +++ b/Source/CTest/cmCTestScriptHandler.h @@ -8,8 +8,6 @@ #include #include -#include "cmCTestGenericHandler.h" - class cmCTest; class cmCTestCommand; class cmGlobalGenerator; @@ -19,11 +17,9 @@ class cmake; /** \class cmCTestScriptHandler * \brief A class that handles ctest -S invocations */ -class cmCTestScriptHandler : public cmCTestGenericHandler +class cmCTestScriptHandler { public: - using Superclass = cmCTestGenericHandler; - /** * Add a script to run, and if is should run in the current process */ @@ -32,7 +28,7 @@ public: /** * Run a dashboard using a specified configuration script */ - int ProcessHandler() override; + int ProcessHandler(); /* * Run a script @@ -46,12 +42,10 @@ public: */ void UpdateElapsedTime(); - cmCTestScriptHandler(); + cmCTestScriptHandler(cmCTest* ctest); cmCTestScriptHandler(const cmCTestScriptHandler&) = delete; const cmCTestScriptHandler& operator=(const cmCTestScriptHandler&) = delete; - ~cmCTestScriptHandler() override; - - void Initialize(cmCTest* ctest) override; + ~cmCTestScriptHandler(); void CreateCMake(); cmake* GetCMake() { return this->CMake.get(); } @@ -68,6 +62,7 @@ private: void AddCTestCommand(std::string const& name, std::unique_ptr command); + cmCTest* CTest = nullptr; std::vector ConfigurationScripts; std::vector ScriptProcessScope; diff --git a/Source/CTest/cmCTestSubmitCommand.cxx b/Source/CTest/cmCTestSubmitCommand.cxx index 2d6d99668c..4bd3beffb1 100644 --- a/Source/CTest/cmCTestSubmitCommand.cxx +++ b/Source/CTest/cmCTestSubmitCommand.cxx @@ -108,7 +108,8 @@ cmCTestGenericHandler* cmCTestSubmitCommand::InitializeHandler() this->Makefile->GetDefinition("CTEST_NOTES_FILES"); if (notesFilesVariable) { cmList notesFiles{ *notesFilesVariable }; - this->CTest->GenerateNotesFile(notesFiles); + this->CTest->GenerateNotesFile(this->Makefile->GetCMakeInstance(), + notesFiles); } cmValue extraFilesVariable = diff --git a/Source/CTest/cmCTestSubmitHandler.cxx b/Source/CTest/cmCTestSubmitHandler.cxx index 3b3baa98ae..1cb8135f13 100644 --- a/Source/CTest/cmCTestSubmitHandler.cxx +++ b/Source/CTest/cmCTestSubmitHandler.cxx @@ -19,7 +19,6 @@ #include "cmAlgorithms.h" #include "cmCTest.h" #include "cmCTestCurl.h" -#include "cmCTestScriptHandler.h" #include "cmCryptoHash.h" #include "cmCurl.h" #include "cmDuration.h" @@ -259,9 +258,7 @@ bool cmCTestSubmitHandler::SubmitUsingHTTP( upload_as += ctest_curl.Escape(this->CTest->GetCurrentTag()); upload_as += "-"; upload_as += ctest_curl.Escape(this->CTest->GetTestGroupString()); - cmCTestScriptHandler* ch = this->CTest->GetScriptHandler(); - cmake* cm = ch->GetCMake(); - if (cm) { + if (cmake* cm = this->CMake) { cmValue subproject = cm->GetState()->GetGlobalProperty("SubProject"); if (subproject) { upload_as += "&subproject="; @@ -556,9 +553,8 @@ int cmCTestSubmitHandler::HandleCDashUploadFile(std::string const& file, // has already been uploaded // TODO I added support for subproject. You would need to add // a "&subproject=subprojectname" to the first POST. - cmCTestScriptHandler* ch = this->CTest->GetScriptHandler(); - cmake* cm = ch->GetCMake(); - cmValue subproject = cm->GetState()->GetGlobalProperty("SubProject"); + cmValue subproject = + this->CMake->GetState()->GetGlobalProperty("SubProject"); // TODO: Encode values for a URL instead of trusting caller. std::ostringstream str; if (subproject) { diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 84fde4361e..83718f0df1 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -1417,7 +1417,7 @@ void cmCTestTestHandler::GenerateCTestXML(cmXMLWriter& xml) return; } - this->CTest->StartXML(xml, this->AppendXML); + this->CTest->StartXML(xml, this->CMake, this->AppendXML); this->CTest->GenerateSubprojectsOutput(xml); xml.StartElement("Testing"); xml.Element("StartDateTime", this->StartTest); diff --git a/Source/CTest/cmCTestUploadHandler.cxx b/Source/CTest/cmCTestUploadHandler.cxx index 552bd3e1c4..bc39ab35f1 100644 --- a/Source/CTest/cmCTestUploadHandler.cxx +++ b/Source/CTest/cmCTestUploadHandler.cxx @@ -50,7 +50,7 @@ int cmCTestUploadHandler::ProcessHandler() xml.Attribute("Name", this->CTest->GetCTestConfiguration("Site")); xml.Attribute("Generator", std::string("ctest-") + cmVersion::GetCMakeVersion()); - this->CTest->AddSiteProperties(xml); + this->CTest->AddSiteProperties(xml, this->CMake); xml.StartElement("Upload"); xml.Element("Time", std::chrono::system_clock::now()); diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx index 2a409d5172..7a08668f19 100644 --- a/Source/cmCTest.cxx +++ b/Source/cmCTest.cxx @@ -126,7 +126,6 @@ struct cmCTest::Private cmCTestBuildHandler BuildHandler; cmCTestBuildAndTest BuildAndTest; cmCTestCoverageHandler CoverageHandler; - cmCTestScriptHandler ScriptHandler; cmCTestTestHandler TestHandler; cmCTestUpdateHandler UpdateHandler; cmCTestConfigureHandler ConfigureHandler; @@ -724,11 +723,6 @@ cmCTestCoverageHandler* cmCTest::GetCoverageHandler() return &this->Impl->CoverageHandler; } -cmCTestScriptHandler* cmCTest::GetScriptHandler() -{ - return &this->Impl->ScriptHandler; -} - cmCTestTestHandler* cmCTest::GetTestHandler() { return &this->Impl->TestHandler; @@ -783,8 +777,7 @@ int cmCTest::ProcessSteps() this->BlockTestErrorDiagnostics(); int res = 0; - cmCTestScriptHandler script; - script.Initialize(this); + cmCTestScriptHandler script(this); script.CreateCMake(); cmMakefile& mf = *script.GetMakefile(); this->ReadCustomConfigurationFileTree(this->Impl->BinaryDir, &mf); @@ -898,7 +891,7 @@ int cmCTest::ProcessSteps() if (this->Impl->Parts[PartNotes]) { this->UpdateCTestConfiguration(); if (!this->Impl->NotesFiles.empty()) { - this->GenerateNotesFile(this->Impl->NotesFiles); + this->GenerateNotesFile(script.GetCMake(), this->Impl->NotesFiles); } } if (this->Impl->Parts[PartSubmit]) { @@ -1102,7 +1095,7 @@ std::string cmCTest::SafeBuildIdField(const std::string& value) return safevalue; } -void cmCTest::StartXML(cmXMLWriter& xml, bool append) +void cmCTest::StartXML(cmXMLWriter& xml, cmake* cm, bool append) { if (this->Impl->CurrentTag.empty()) { cmCTestLog(this, ERROR_MESSAGE, @@ -1164,25 +1157,17 @@ void cmCTest::StartXML(cmXMLWriter& xml, bool append) xml.Attribute("ChangeId", changeId); } - this->AddSiteProperties(xml); + this->AddSiteProperties(xml, cm); } -void cmCTest::AddSiteProperties(cmXMLWriter& xml) +void cmCTest::AddSiteProperties(cmXMLWriter& xml, cmake* cm) { - cmCTestScriptHandler* ch = this->GetScriptHandler(); - cmake* cm = ch->GetCMake(); - // if no CMake then this is the old style script and props like - // this will not work anyway. - if (!cm) { - return; - } // This code should go when cdash is changed to use labels only cmValue subproject = cm->GetState()->GetGlobalProperty("SubProject"); if (subproject) { xml.StartElement("Subproject"); xml.Attribute("name", *subproject); - cmValue labels = - ch->GetCMake()->GetState()->GetGlobalProperty("SubProjectLabels"); + cmValue labels = cm->GetState()->GetGlobalProperty("SubProjectLabels"); if (labels) { xml.StartElement("Labels"); cmList args{ *labels }; @@ -1234,7 +1219,7 @@ void cmCTest::EndXML(cmXMLWriter& xml) xml.EndDocument(); } -int cmCTest::GenerateCTestNotesOutput(cmXMLWriter& xml, +int cmCTest::GenerateCTestNotesOutput(cmXMLWriter& xml, cmake* cm, std::vector const& files) { std::string buildname = @@ -1251,7 +1236,7 @@ int cmCTest::GenerateCTestNotesOutput(cmXMLWriter& xml, xml.Attribute("Name", this->GetCTestConfiguration("Site")); xml.Attribute("Generator", std::string("ctest-") + cmVersion::GetCMakeVersion()); - this->AddSiteProperties(xml); + this->AddSiteProperties(xml, cm); xml.StartElement("Notes"); for (std::string const& file : files) { @@ -1285,7 +1270,8 @@ int cmCTest::GenerateCTestNotesOutput(cmXMLWriter& xml, return 1; } -int cmCTest::GenerateNotesFile(std::vector const& files) +int cmCTest::GenerateNotesFile(cmake* cm, + std::vector const& files) { cmGeneratedFileStream ofs; if (!this->OpenOutputFile(this->Impl->CurrentTag, "Notes.xml", ofs)) { @@ -1293,11 +1279,11 @@ int cmCTest::GenerateNotesFile(std::vector const& files) return 1; } cmXMLWriter xml(ofs); - this->GenerateCTestNotesOutput(xml, files); + this->GenerateCTestNotesOutput(xml, cm, files); return 0; } -int cmCTest::GenerateNotesFile(const std::string& cfiles) +int cmCTest::GenerateNotesFile(cmake* cm, const std::string& cfiles) { if (cfiles.empty()) { return 1; @@ -1311,7 +1297,7 @@ int cmCTest::GenerateNotesFile(const std::string& cfiles) return 1; } - return this->GenerateNotesFile(files); + return this->GenerateNotesFile(cm, files); } int cmCTest::GenerateDoneFile() @@ -2717,9 +2703,7 @@ int cmCTest::RunScripts( cmCTestLog(this, OUTPUT, "* Extra verbosity turned on" << std::endl); } - cmCTestScriptHandler* ch = this->GetScriptHandler(); - ch->Initialize(this); - ch->SetVerbose(this->Impl->Verbose); + auto ch = cm::make_unique(this); for (auto const& script : scripts) { ch->AddConfigurationScript(script.first, script.second); } diff --git a/Source/cmCTest.h b/Source/cmCTest.h index 2c0575b467..4ef3b18618 100644 --- a/Source/cmCTest.h +++ b/Source/cmCTest.h @@ -19,9 +19,9 @@ #include "cmDuration.h" #include "cmProcessOutput.h" +class cmake; class cmCTestBuildHandler; class cmCTestCoverageHandler; -class cmCTestScriptHandler; class cmCTestTestHandler; class cmCTestUpdateHandler; class cmCTestConfigureHandler; @@ -237,7 +237,7 @@ public: static std::string SafeBuildIdField(const std::string& value); /** Start CTest XML output file */ - void StartXML(cmXMLWriter& xml, bool append); + void StartXML(cmXMLWriter& xml, cmake* cm, bool append); /** End CTest XML output file */ void EndXML(cmXMLWriter& xml); @@ -294,7 +294,6 @@ public: */ cmCTestBuildHandler* GetBuildHandler(); cmCTestCoverageHandler* GetCoverageHandler(); - cmCTestScriptHandler* GetScriptHandler(); cmCTestTestHandler* GetTestHandler(); cmCTestUpdateHandler* GetUpdateHandler(); cmCTestConfigureHandler* GetConfigureHandler(); @@ -332,7 +331,7 @@ public: void AddCTestConfigurationOverwrite(const std::string& encstr); /** Create XML file that contains all the notes specified */ - int GenerateNotesFile(std::vector const& files); + int GenerateNotesFile(cmake* cm, std::vector const& files); /** Create XML file to indicate that build is complete */ int GenerateDoneFile(); @@ -408,7 +407,7 @@ public: bool GetExtraVerbose() const; int GetSubmitIndex() const; - void AddSiteProperties(cmXMLWriter& xml); + void AddSiteProperties(cmXMLWriter& xml, cmake* cm); bool GetLabelSummary() const; bool GetSubprojectSummary() const; @@ -448,7 +447,7 @@ public: cmCTestTestOptions const& GetTestOptions() const; private: - int GenerateNotesFile(const std::string& files); + int GenerateNotesFile(cmake* cm, const std::string& files); void BlockTestErrorDiagnostics(); @@ -476,7 +475,7 @@ private: static bool ColoredOutputSupportedByConsole(); /** Create note from files. */ - int GenerateCTestNotesOutput(cmXMLWriter& xml, + int GenerateCTestNotesOutput(cmXMLWriter& xml, cmake* cm, std::vector const& files); /** Check if the argument is the one specified */