From e52eada2c26bf8ff8046d778897fe78adf6073a9 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Tue, 28 Jan 2025 10:00:49 +0100 Subject: [PATCH] cmCTestConfigureCommand: Refactor command line construction --- Source/CTest/cmCTestConfigureCommand.cxx | 195 ++++++++++------------- 1 file changed, 86 insertions(+), 109 deletions(-) diff --git a/Source/CTest/cmCTestConfigureCommand.cxx b/Source/CTest/cmCTestConfigureCommand.cxx index 87b7e29b8f..92123c8b76 100644 --- a/Source/CTest/cmCTestConfigureCommand.cxx +++ b/Source/CTest/cmCTestConfigureCommand.cxx @@ -3,7 +3,6 @@ #include "cmCTestConfigureCommand.h" #include -#include #include #include #include @@ -44,123 +43,101 @@ cmCTestConfigureCommand::InitializeHandler(HandlerArguments& arguments, { cmMakefile& mf = status.GetMakefile(); auto const& args = static_cast(arguments); - cmList options; - if (!args.Options.empty()) { - options.assign(args.Options); - } - - if (this->CTest->GetCTestConfiguration("BuildDirectory").empty()) { - status.SetError( - "Build directory not specified. Either use BUILD " - "argument to CTEST_CONFIGURE command or set CTEST_BINARY_DIRECTORY " - "variable"); + std::string const buildDirectory = !args.Build.empty() + ? args.Build + : mf.GetDefinition("CTEST_BINARY_DIRECTORY"); + if (buildDirectory.empty()) { + status.SetError("called with no build directory specified. " + "Either use the BUILD argument or set the " + "CTEST_BINARY_DIRECTORY variable."); return nullptr; } - cmValue ctestConfigureCommand = mf.GetDefinition("CTEST_CONFIGURE_COMMAND"); - - if (cmNonempty(ctestConfigureCommand)) { - this->CTest->SetCTestConfiguration("ConfigureCommand", - *ctestConfigureCommand, args.Quiet); - } else { - cmValue cmakeGeneratorName = mf.GetDefinition("CTEST_CMAKE_GENERATOR"); - if (cmNonempty(cmakeGeneratorName)) { - std::string const& source_dir = - this->CTest->GetCTestConfiguration("SourceDirectory"); - if (source_dir.empty()) { - status.SetError( - "Source directory not specified. Either use SOURCE " - "argument to CTEST_CONFIGURE command or set CTEST_SOURCE_DIRECTORY " - "variable"); - return nullptr; - } - - std::string const cmlName = mf.GetSafeDefinition("CMAKE_LIST_FILE_NAME"); - std::string const cmakelists_file = cmStrCat( - source_dir, "/", cmlName.empty() ? "CMakeLists.txt" : cmlName); - if (!cmSystemTools::FileExists(cmakelists_file)) { - std::ostringstream e; - e << "CMakeLists.txt file does not exist [" << cmakelists_file << "]"; - status.SetError(e.str()); - return nullptr; - } - - bool multiConfig = false; - bool cmakeBuildTypeInOptions = false; - - auto gg = - mf.GetCMakeInstance()->CreateGlobalGenerator(*cmakeGeneratorName); - if (gg) { - multiConfig = gg->IsMultiConfig(); - gg.reset(); - } - - std::string cmakeConfigureCommand = - cmStrCat('"', cmSystemTools::GetCMakeCommand(), '"'); - - for (std::string const& option : options) { - cmakeConfigureCommand += " \""; - cmakeConfigureCommand += option; - cmakeConfigureCommand += "\""; - - if ((nullptr != strstr(option.c_str(), "CMAKE_BUILD_TYPE=")) || - (nullptr != strstr(option.c_str(), "CMAKE_BUILD_TYPE:STRING="))) { - cmakeBuildTypeInOptions = true; - } - } - - if (!multiConfig && !cmakeBuildTypeInOptions && - !this->CTest->GetConfigType().empty()) { - cmakeConfigureCommand += " \"-DCMAKE_BUILD_TYPE:STRING="; - cmakeConfigureCommand += this->CTest->GetConfigType(); - cmakeConfigureCommand += "\""; - } - - if (mf.IsOn("CTEST_USE_LAUNCHERS")) { - cmakeConfigureCommand += " \"-DCTEST_USE_LAUNCHERS:BOOL=TRUE\""; - } - - cmakeConfigureCommand += " \"-G"; - cmakeConfigureCommand += *cmakeGeneratorName; - cmakeConfigureCommand += "\""; - - cmValue cmakeGeneratorPlatform = - mf.GetDefinition("CTEST_CMAKE_GENERATOR_PLATFORM"); - if (cmNonempty(cmakeGeneratorPlatform)) { - cmakeConfigureCommand += " \"-A"; - cmakeConfigureCommand += *cmakeGeneratorPlatform; - cmakeConfigureCommand += "\""; - } - - cmValue cmakeGeneratorToolset = - mf.GetDefinition("CTEST_CMAKE_GENERATOR_TOOLSET"); - if (cmNonempty(cmakeGeneratorToolset)) { - cmakeConfigureCommand += " \"-T"; - cmakeConfigureCommand += *cmakeGeneratorToolset; - cmakeConfigureCommand += "\""; - } - - cmakeConfigureCommand += " \"-S"; - cmakeConfigureCommand += source_dir; - cmakeConfigureCommand += "\""; - - cmakeConfigureCommand += " \"-B"; - cmakeConfigureCommand += - this->CTest->GetCTestConfiguration("BuildDirectory"); - cmakeConfigureCommand += "\""; - - this->CTest->SetCTestConfiguration("ConfigureCommand", - cmakeConfigureCommand, args.Quiet); - } else { + std::string configureCommand = mf.GetDefinition("CTEST_CONFIGURE_COMMAND"); + if (configureCommand.empty()) { + cmValue cmakeGenerator = mf.GetDefinition("CTEST_CMAKE_GENERATOR"); + if (!cmNonempty(cmakeGenerator)) { status.SetError( - "Configure command is not specified. If this is a " - "\"built with CMake\" project, set CTEST_CMAKE_GENERATOR. If not, " - "set CTEST_CONFIGURE_COMMAND."); + "called with no configure command specified. " + "If this is a \"built with CMake\" project, set " + "CTEST_CMAKE_GENERATOR. If not, set CTEST_CONFIGURE_COMMAND."); return nullptr; } + + std::string const sourceDirectory = !args.Source.empty() + ? args.Source + : mf.GetDefinition("CTEST_SOURCE_DIRECTORY"); + if (sourceDirectory.empty() || + !cmSystemTools::FileExists(sourceDirectory + "/CMakeLists.txt")) { + status.SetError("called with invalid source directory. " + "CTEST_SOURCE_DIRECTORY must be set to a directory " + "that contains CMakeLists.txt."); + return nullptr; + } + + bool const multiConfig = [&]() -> bool { + cmake* cm = mf.GetCMakeInstance(); + auto gg = cm->CreateGlobalGenerator(cmakeGenerator); + return gg && gg->IsMultiConfig(); + }(); + + bool const buildTypeInOptions = + args.Options.find("CMAKE_BUILD_TYPE=") != std::string::npos || + args.Options.find("CMAKE_BUILD_TYPE:STRING=") != std::string::npos; + + configureCommand = cmStrCat('"', cmSystemTools::GetCMakeCommand(), '"'); + + auto const options = cmList(args.Options); + for (std::string const& option : options) { + configureCommand += " \""; + configureCommand += option; + configureCommand += "\""; + } + + cmValue cmakeBuildType = mf.GetDefinition("CTEST_CONFIGURATION_TYPE"); + if (!multiConfig && !buildTypeInOptions && cmNonempty(cmakeBuildType)) { + configureCommand += " \"-DCMAKE_BUILD_TYPE:STRING="; + configureCommand += cmakeBuildType; + configureCommand += "\""; + } + + if (mf.IsOn("CTEST_USE_LAUNCHERS")) { + configureCommand += " \"-DCTEST_USE_LAUNCHERS:BOOL=TRUE\""; + } + + configureCommand += " \"-G"; + configureCommand += cmakeGenerator; + configureCommand += "\""; + + cmValue cmakeGeneratorPlatform = + mf.GetDefinition("CTEST_CMAKE_GENERATOR_PLATFORM"); + if (cmNonempty(cmakeGeneratorPlatform)) { + configureCommand += " \"-A"; + configureCommand += *cmakeGeneratorPlatform; + configureCommand += "\""; + } + + cmValue cmakeGeneratorToolset = + mf.GetDefinition("CTEST_CMAKE_GENERATOR_TOOLSET"); + if (cmNonempty(cmakeGeneratorToolset)) { + configureCommand += " \"-T"; + configureCommand += *cmakeGeneratorToolset; + configureCommand += "\""; + } + + configureCommand += " \"-S"; + configureCommand += sourceDirectory; + configureCommand += "\""; + + configureCommand += " \"-B"; + configureCommand += buildDirectory; + configureCommand += "\""; } + this->CTest->SetCTestConfiguration("ConfigureCommand", configureCommand, + args.Quiet); + if (cmValue labelsForSubprojects = mf.GetDefinition("CTEST_LABELS_FOR_SUBPROJECTS")) { this->CTest->SetCTestConfiguration("LabelsForSubprojects",