ctest: prioritize presets over existing DartConfiguration.tcl commands

CMake writes ConfigureCommand and MakeCommand entries into
DartConfiguration.tcl when configuring a project that uses the CTest module.

These values prevented users from being able to use configure/build presets
in dashboard client mode from a pre-configured build directory.

This commit updates ctest_configure() and ctest_build() to ignore
CTEST_CONFIGURE_COMMAND / CTEST_BUILD_COMMAND when a preset is specified.
This allows the requested preset to be honored.

Help/command/ctest_build.rst

@@ -23,8 +23,8 @@ Build the project and store results in ``Build.xml``
 for submission with the :command:`ctest_submit` command.
 
 The :variable:`CTEST_BUILD_COMMAND` variable may be set to explicitly
-specify the build command line.  Otherwise the build command line is
-computed automatically based on the options given.
+specify the build command line when no preset is selected.  Otherwise
+the build command line is computed automatically based on the options given.
 
 The options are:
 
@@ -86,6 +86,9 @@ The options are:
   argument will override the :preset:`buildPresets.targets` setting from
   the chosen preset.
 
+  When a preset is specified, the :variable:`CTEST_BUILD_COMMAND`
+  variable is ignored so that the preset is always honored.
+
   See also the :variable:`CTEST_BUILD_PRESET` and
   :variable:`CTEST_PRESET` variables.
 

Help/command/ctest_configure.rst

@@ -46,8 +46,8 @@ The options are:
   :variable:`CTEST_BINARY_DIRECTORY` variable will override the
   :preset:`configurePresets.binaryDir` setting from the chosen preset.
 
-  This option is ignored when :variable:`CTEST_CONFIGURE_COMMAND`
-  is used.
+  When a preset is specified, the :variable:`CTEST_CONFIGURE_COMMAND`
+  variable is ignored so that the preset is always honored.
 
   See also the :variable:`CTEST_CONFIGURE_PRESET` and
   :variable:`CTEST_PRESET` variables.

Help/release/dev/ctest-script-preset-arg.rst

@@ -12,3 +12,9 @@ ctest-script-preset-arg
   arguments to the above commands in a :ref:`Dashboard Client` script or
   on the :program:`ctest` command line via the
   :ref:`-D <ctest-option-D-var>` option.
+
+* When a preset is selected for :command:`ctest_configure` or
+  :command:`ctest_build`, the :variable:`CTEST_CONFIGURE_COMMAND` and
+  :variable:`CTEST_BUILD_COMMAND` variables (which may be populated from a
+  pre-existing ``DartConfiguration.tcl`` file) are ignored so that the
+  requested preset is always honored.

Source/CTest/cmCTestBuildCommand.cxx

@@ -116,11 +116,20 @@ std::unique_ptr<cmCTestGenericHandler> cmCTestBuildCommand::InitializeHandler(
     }
   }
 
+  // Skip checking CTEST_BUILD_COMMAND when a preset is specified.
+  // We do this to avoid using a stale MakeCommand from DartConfiguration.tcl
+  // that would cause us to silently ignore the requested preset.
   cmValue ctestBuildCommand = mf.GetDefinition("CTEST_BUILD_COMMAND");
-  if (cmNonempty(ctestBuildCommand)) {
+  if (cmNonempty(ctestBuildCommand) && effectivePreset.empty()) {
     this->CTest->SetCTestConfiguration("MakeCommand", *ctestBuildCommand,
                                        args.Quiet);
   } else if (!effectivePreset.empty()) {
+    if (cmNonempty(ctestBuildCommand)) {
+      cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
+                         "Ignoring CTEST_BUILD_COMMAND because preset \""
+                           << effectivePreset << "\" is in use.\n",
+                         args.Quiet);
+    }
     cmCMakePresetsGraph presetsGraph;
     if (!presetsGraph.ReadProjectPresets(sourceDirectory, presetsFile)) {
       status.SetError(cmStrCat("\n Could not read presets from \"",

Source/CTest/cmCTestConfigureCommand.cxx

@@ -211,7 +211,18 @@ bool cmCTestConfigureCommand::ExecuteConfigure(ConfigureArguments const& args,
     ? ""
     : cmSystemTools::CollapseFullPath(rawPresetsFile, sourceDirectory);
 
-  std::string configureCommand = mf.GetDefinition("CTEST_CONFIGURE_COMMAND");
+  // Skip checking CTEST_CONFIGURE_COMMAND when a preset is specified.
+  // We do this to avoid using a stale ConfigureCommand from a previous cmake
+  // run that would cause us to silently ignore the requested preset.
+  std::string configureCommand;
+  if (presetName.empty()) {
+    configureCommand = mf.GetDefinition("CTEST_CONFIGURE_COMMAND");
+  } else if (cmNonempty(mf.GetDefinition("CTEST_CONFIGURE_COMMAND"))) {
+    cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
+                       "Ignoring CTEST_CONFIGURE_COMMAND because preset \""
+                         << presetName << "\" is in use.\n",
+                       args.Quiet);
+  }
   if (configureCommand.empty() &&
       !ConstructConfigureCommand(status, mf, sourceDirectory, buildDirectory,
                                  args.Options, presetName, presetsFile,

Tests/RunCMake/ctest_build/BuildConfigurationCLIVar-check.cmake

@@ -0,0 +1,13 @@
+file(GLOB build_xml_file "${RunCMake_TEST_BINARY_DIR}/Testing/*/Build.xml")
+if(build_xml_file)
+  file(READ "${build_xml_file}" build_xml)
+  if(NOT build_xml MATCHES "--preset.*my-build-preset")
+    set(RunCMake_TEST_FAILED
+        "Build.xml does not contain the expected --preset argument")
+  elseif(NOT build_xml MATCHES "--config.*my-config")
+    set(RunCMake_TEST_FAILED
+        "Build.xml does not contain the expected --config argument")
+  endif()
+else()
+  set(RunCMake_TEST_FAILED "Build.xml not found")
+endif()

Tests/RunCMake/ctest_build/BuildConfigurationCLIVar-stderr.txt

@@ -0,0 +1 @@
+Error\(s\) when building project

Tests/RunCMake/ctest_build/BuildPresetCLIVar-check.cmake

@@ -0,0 +1,10 @@
+file(GLOB build_xml_file "${RunCMake_TEST_BINARY_DIR}/Testing/*/Build.xml")
+if(build_xml_file)
+  file(READ "${build_xml_file}" build_xml)
+  if(NOT build_xml MATCHES "--preset.*my-build-preset")
+    set(RunCMake_TEST_FAILED
+        "Build.xml does not contain the expected --preset argument")
+  endif()
+else()
+  set(RunCMake_TEST_FAILED "Build.xml not found")
+endif()

Tests/RunCMake/ctest_build/BuildPresetCLIVar-stderr.txt

@@ -0,0 +1 @@
+Error\(s\) when building project

Tests/RunCMake/ctest_build/ConfigurationTypeCLIVar-check.cmake

@@ -0,0 +1,13 @@
+file(GLOB build_xml_file "${RunCMake_TEST_BINARY_DIR}/Testing/*/Build.xml")
+if(build_xml_file)
+  file(READ "${build_xml_file}" build_xml)
+  if(NOT build_xml MATCHES "--preset.*my-build-preset")
+    set(RunCMake_TEST_FAILED
+        "Build.xml does not contain the expected --preset argument")
+  elseif(NOT build_xml MATCHES "--config.*my-type")
+    set(RunCMake_TEST_FAILED
+        "Build.xml does not contain the expected --config argument")
+  endif()
+else()
+  set(RunCMake_TEST_FAILED "Build.xml not found")
+endif()

Tests/RunCMake/ctest_build/ConfigurationTypeCLIVar-stderr.txt

@@ -0,0 +1 @@
+Error\(s\) when building project

Tests/RunCMake/ctest_build/RunCMakeTest.cmake

@@ -132,6 +132,52 @@ run_ctest_build(BuildPresetBadFile
   PRESET my-build-preset
   PRESETS_FILE /nonexistent/path/presets.json)
 
+# Helper for tests that verify -D variables reach ctest_build() in -M/-T mode.
+# Accepts a case name followed by the -D arguments to pass to ctest.
+# Sets up a source directory with CMakePresets.json and a binary directory with
+# DartConfiguration.tcl, then runs ctest with -M Experimental -T Build -V.
+function(run_build_cli_var_test CASE_NAME)
+  set(case_source_dir "${RunCMake_BINARY_DIR}/${CASE_NAME}")
+  set(case_binary_dir "${RunCMake_BINARY_DIR}/${CASE_NAME}-build")
+  file(MAKE_DIRECTORY "${case_source_dir}")
+  configure_file(
+    "${RunCMake_SOURCE_DIR}/CMakePresets.json.in"
+    "${case_source_dir}/CMakePresets.json"
+    @ONLY)
+  file(REMOVE_RECURSE "${case_binary_dir}")
+  file(MAKE_DIRECTORY "${case_binary_dir}")
+  file(WRITE "${case_binary_dir}/DartConfiguration.tcl"
+    "BuildDirectory: ${case_binary_dir}\n"
+    "SourceDirectory: ${case_source_dir}\n"
+    "MakeCommand: \"${CMAKE_COMMAND}\" --build \"${case_binary_dir}\"\n")
+  set(RunCMake_TEST_SOURCE_DIR "${case_source_dir}")
+  set(RunCMake_TEST_BINARY_DIR "${case_binary_dir}")
+  set(RunCMake_TEST_NO_CLEAN 1)
+  run_cmake_command(${CASE_NAME}
+    ${CMAKE_CTEST_COMMAND}
+    -M Experimental
+    ${ARGN}
+    -T Build
+    -V)
+endfunction()
+
+# Verify that CTEST_BUILD_PRESET passed via -D on the command line reaches
+# ctest_build() when ctest is run with -M/-T.
+run_build_cli_var_test(BuildPresetCLIVar
+  -D "CTEST_BUILD_PRESET=my-build-preset")
+
+# Verify that CTEST_BUILD_CONFIGURATION passed via -D on the command line
+# reaches ctest_build() when ctest is run with -M/-T.
+run_build_cli_var_test(BuildConfigurationCLIVar
+  -D "CTEST_BUILD_PRESET=my-build-preset"
+  -D "CTEST_BUILD_CONFIGURATION=my-config")
+
+# Verify that CTEST_CONFIGURATION_TYPE passed via -D on the command line
+# reaches ctest_build() when ctest is run with -M/-T.
+run_build_cli_var_test(ConfigurationTypeCLIVar
+  -D "CTEST_BUILD_PRESET=my-build-preset"
+  -D "CTEST_CONFIGURATION_TYPE=my-type")
+
 set(RunCMake_USE_CUSTOM_BUILD_COMMAND FALSE)
 if(RunCMake_GENERATOR MATCHES "Ninja")
   function(run_NinjaLauncherSingleBuildFailure)

Tests/RunCMake/ctest_configure/ConfigurePresetCLIVar-check.cmake

@@ -0,0 +1,25 @@
+if(IS_DIRECTORY "${RunCMake_TEST_SOURCE_DIR}/build")
+  set(RunCMake_TEST_FAILED
+      "CTEST_BINARY_DIRECTORY did not override buildDir from preset")
+endif()
+
+file(GLOB configure_xml_file "${RunCMake_TEST_BINARY_DIR}/Testing/*/Configure.xml")
+if(configure_xml_file)
+  file(READ "${configure_xml_file}" configure_xml)
+  if(NOT configure_xml MATCHES "\"--preset\" \"my-preset\"")
+    set(RunCMake_TEST_FAILED
+        "Configure.xml does not contain the expected --preset argument")
+  endif()
+else()
+  set(RunCMake_TEST_FAILED "Configure.xml not found")
+endif()
+
+set(cmakecache_file "${RunCMake_TEST_BINARY_DIR}/CMakeCache.txt")
+if(EXISTS "${cmakecache_file}")
+  file(READ "${cmakecache_file}" cmakecache_txt)
+  if(NOT cmakecache_txt MATCHES "MY_CUSTOM_VAR:STRING=this-gets-set")
+    set(RunCMake_TEST_FAILED "CMakeCache.txt does not contain MY_CUSTOM_VAR")
+  endif()
+else()
+  set(RunCMake_TEST_FAILED "CMakeCache.txt not found")
+endif()

Tests/RunCMake/ctest_configure/RunCMakeTest.cmake

@@ -56,3 +56,36 @@ run_ctest(ConfigurePresetFromFileVar)
 unset(CASE_TEST_PREFIX_CODE)
 unset(custom_presets_file)
 unset(RunCMake_TEST_SOURCE_DIR)
+
+# Verify that CTEST_PRESET passed via -D on the command line reaches
+# ctest_configure() when ctest is run with -M/-T.
+set(case_source_dir "${RunCMake_BINARY_DIR}/ConfigurePresetCLIVar")
+set(case_binary_dir "${RunCMake_BINARY_DIR}/ConfigurePresetCLIVar-build")
+set(CASE_NAME "ConfigurePresetCLIVar")
+file(MAKE_DIRECTORY "${case_source_dir}")
+configure_file(
+  "${RunCMake_SOURCE_DIR}/CMakeLists.txt.in"
+  "${case_source_dir}/CMakeLists.txt"
+  @ONLY)
+configure_file(
+  "${RunCMake_SOURCE_DIR}/CMakePresets.json.in"
+  "${case_source_dir}/CMakePresets.json"
+  @ONLY)
+file(REMOVE_RECURSE "${case_binary_dir}")
+file(MAKE_DIRECTORY "${case_binary_dir}")
+file(WRITE "${case_binary_dir}/DartConfiguration.tcl"
+  "BuildDirectory: ${case_binary_dir}\n"
+  "SourceDirectory: ${case_source_dir}\n"
+  "ConfigureCommand: \"${CMAKE_COMMAND}\" -S\"${case_source_dir}\" -B\"${case_binary_dir}\"\n")
+set(RunCMake_TEST_SOURCE_DIR "${case_source_dir}")
+set(RunCMake_TEST_BINARY_DIR "${case_binary_dir}")
+set(RunCMake_TEST_NO_CLEAN 1)
+run_cmake_command(ConfigurePresetCLIVar
+  ${CMAKE_CTEST_COMMAND}
+  -M Experimental
+  -D "CTEST_PRESET=my-preset"
+  -T Configure
+  -V)
+unset(RunCMake_TEST_SOURCE_DIR)
+unset(RunCMake_TEST_BINARY_DIR)
+unset(RunCMake_TEST_NO_CLEAN)