cmCTestConfigureCommand: Refactor command line construction

Source/CTest/cmCTestConfigureCommand.cxx

@@ -3,7 +3,6 @@
 #include "cmCTestConfigureCommand.h"
 
 #include <chrono>
-#include <cstring>
 #include <sstream>
 #include <string>
 #include <utility>
@@ -44,123 +43,101 @@ cmCTestConfigureCommand::InitializeHandler(HandlerArguments& arguments,
 {
   cmMakefile& mf = status.GetMakefile();
   auto const& args = static_cast<ConfigureArguments&>(arguments);
-  cmList options;
 
-  if (!args.Options.empty()) {
+  std::string const buildDirectory = !args.Build.empty()
-    options.assign(args.Options);
+    ? args.Build
-  }
+    : mf.GetDefinition("CTEST_BINARY_DIRECTORY");
-
+  if (buildDirectory.empty()) {
-  if (this->CTest->GetCTestConfiguration("BuildDirectory").empty()) {
+    status.SetError("called with no build directory specified.  "
-    status.SetError(
+                    "Either use the BUILD argument or set the "
-      "Build directory not specified. Either use BUILD "
+                    "CTEST_BINARY_DIRECTORY variable.");
-      "argument to CTEST_CONFIGURE command or set CTEST_BINARY_DIRECTORY "
-      "variable");
     return nullptr;
   }
 
-  cmValue ctestConfigureCommand = mf.GetDefinition("CTEST_CONFIGURE_COMMAND");
+  std::string configureCommand = mf.GetDefinition("CTEST_CONFIGURE_COMMAND");
-
+  if (configureCommand.empty()) {
-  if (cmNonempty(ctestConfigureCommand)) {
+    cmValue cmakeGenerator = mf.GetDefinition("CTEST_CMAKE_GENERATOR");
-    this->CTest->SetCTestConfiguration("ConfigureCommand",
+    if (!cmNonempty(cmakeGenerator)) {
-                                       *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 {
       status.SetError(
-        "Configure command is not specified. If this is a "
+        "called with no configure command specified.  "
-        "\"built with CMake\" project, set CTEST_CMAKE_GENERATOR. If not, "
+        "If this is a  \"built with CMake\" project, set "
-        "set CTEST_CONFIGURE_COMMAND.");
+        "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",