Merge topic 'instrumentation-presets-hooks' into release-4.0

45d3ffaa95 instrumentation: Use auto for variables holding closures
d75ccd9f8d instrumentation: Fix docs for ctest --collect-instrumentation option
a2232db802 instrumentation: Make hooks compatible with presets

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Merge-request: !10352

Help/manual/cmake-instrumentation.7.rst

@@ -53,7 +53,7 @@ triggered, an index file is generated containing a list of snippet files newer
 than the previous indexing.
 
 Indexing and can also be performed by manually invoking
-``ctest --collect-instrumentation``.
+``ctest --collect-instrumentation <build>``.
 
 Callbacks
 ---------
@@ -410,7 +410,7 @@ occurs and deleted after any user-specified `Callbacks`_ are executed.
   The name of the hook responsible for generating the index file. In addition
   to the hooks that can be specified by one of the `v1 Query Files`_, this value may
   be set to ``manual`` if indexing is performed by invoking
-  ``ctest --collect-instrumentation``.
+  ``ctest --collect-instrumentation <build>``.
 
 ``snippets``
   Contains a list of `v1 Snippet File`_. This includes all snippet files

Source/cmCTest.cxx

@@ -10,7 +10,6 @@
 #include <cstdlib>
 #include <cstring>
 #include <ctime>
-#include <functional>
 #include <initializer_list>
 #include <iostream>
 #include <map>
@@ -2638,32 +2637,23 @@ int cmCTest::Run(std::vector<std::string> const& args)
   }
 #endif
 
-  std::function<int()> doTest = [this, &cmakeAndTest, &runScripts,
-                                 &processSteps]() -> int {
-    // now what should cmake do? if --build-and-test was specified then
-    // we run the build and test handler and return
-    if (cmakeAndTest) {
-      return this->RunCMakeAndTest();
-    }
+  // now what should cmake do? if --build-and-test was specified then
+  // we run the build and test handler and return
+  if (cmakeAndTest) {
+    return this->RunCMakeAndTest();
+  }
 
-    // -S, -SP, and/or -SP was specified
-    if (!runScripts.empty()) {
-      return this->RunScripts(runScripts);
-    }
+  // -S, -SP, and/or -SP was specified
+  if (!runScripts.empty()) {
+    return this->RunScripts(runScripts);
+  }
 
-    // -D, -T, and/or -M was specified
-    if (processSteps) {
-      return this->ProcessSteps();
-    }
+  // -D, -T, and/or -M was specified
+  if (processSteps) {
+    return this->ProcessSteps();
+  }
 
-    return this->ExecuteTests();
-  };
-  cmInstrumentation instrumentation(
-    cmSystemTools::GetCurrentWorkingDirectory());
-  int ret = instrumentation.InstrumentCommand("ctest", args,
-                                              [doTest]() { return doTest(); });
-  instrumentation.CollectTimingData(cmInstrumentationQuery::Hook::PostTest);
-  return ret;
+  return this->ExecuteTests(args);
 }
 
 int cmCTest::RunScripts(
@@ -2687,7 +2677,7 @@ int cmCTest::RunScripts(
   return res;
 }
 
-int cmCTest::ExecuteTests()
+int cmCTest::ExecuteTests(std::vector<std::string> const& args)
 {
   this->Impl->ExtraVerbose = this->Impl->Verbose;
   this->Impl->Verbose = true;
@@ -2732,7 +2722,14 @@ int cmCTest::ExecuteTests()
   }
 
   handler.SetVerbose(this->Impl->Verbose);
-  if (handler.ProcessHandler() < 0) {
+
+  cmInstrumentation instrumentation(this->GetBinaryDir());
+  auto processHandler = [&handler]() -> int {
+    return handler.ProcessHandler();
+  };
+  int ret = instrumentation.InstrumentCommand("ctest", args, processHandler);
+  instrumentation.CollectTimingData(cmInstrumentationQuery::Hook::PostTest);
+  if (ret < 0) {
     cmCTestLog(this, ERROR_MESSAGE, "Errors while running CTest\n");
     if (!this->Impl->OutputTestOutputOnTestFailure) {
       std::string const lastTestLog =

Source/cmCTest.h

@@ -480,7 +480,7 @@ private:
 
   int RunCMakeAndTest();
   int RunScripts(std::vector<std::pair<std::string, bool>> const& scripts);
-  int ExecuteTests();
+  int ExecuteTests(std::vector<std::string> const& args);
 
   struct Private;
   std::unique_ptr<Private> Impl;

Source/cmake.cxx

@@ -2624,7 +2624,7 @@ int cmake::ActualConfigure()
     cmSystemTools::Error(this->Instrumentation->errorMsg);
     return 1;
   }
-  std::function<int()> doConfigure = [this]() -> int {
+  auto doConfigure = [this]() -> int {
     this->GlobalGenerator->Configure();
     return 0;
   };
@@ -3023,7 +3023,7 @@ int cmake::Generate()
   auto startTime = std::chrono::steady_clock::now();
 #if !defined(CMAKE_BOOTSTRAP)
   auto profilingRAII = this->CreateProfilingEntry("project", "generate");
-  std::function<int()> doGenerate = [this]() -> int {
+  auto doGenerate = [this]() -> int {
     if (!this->GlobalGenerator->Compute()) {
       return -1;
     }
@@ -3720,7 +3720,8 @@ std::vector<std::string> cmake::GetDebugConfigs()
 int cmake::Build(int jobs, std::string dir, std::vector<std::string> targets,
                  std::string config, std::vector<std::string> nativeOptions,
                  cmBuildOptions& buildOptions, bool verbose,
-                 std::string const& presetName, bool listPresets)
+                 std::string const& presetName, bool listPresets,
+                 std::vector<std::string> const& args)
 {
   this->SetHomeDirectory("");
   this->SetHomeOutputDirectory("");
@@ -3958,16 +3959,38 @@ int cmake::Build(int jobs, std::string dir, std::vector<std::string> targets,
     return 1;
   }
 
+#if !defined(CMAKE_BOOTSTRAP)
+  cmInstrumentation instrumentation(dir);
+  if (!instrumentation.errorMsg.empty()) {
+    cmSystemTools::Error(instrumentation.errorMsg);
+    return 1;
+  }
+  instrumentation.CollectTimingData(
+    cmInstrumentationQuery::Hook::PreCMakeBuild);
+#endif
+
   this->GlobalGenerator->PrintBuildCommandAdvice(std::cerr, jobs);
   std::stringstream ostr;
   // `cmGlobalGenerator::Build` logs metadata about what directory and commands
   // are being executed to the `output` parameter. If CMake is verbose, print
   // this out.
   std::ostream& verbose_ostr = verbose ? std::cout : ostr;
-  int buildresult = this->GlobalGenerator->Build(
-    jobs, "", dir, projName, targets, verbose_ostr, "", config, buildOptions,
-    verbose, cmDuration::zero(), cmSystemTools::OUTPUT_PASSTHROUGH,
-    nativeOptions);
+  auto doBuild = [this, jobs, dir, projName, targets, &verbose_ostr, config,
+                  buildOptions, verbose, nativeOptions]() -> int {
+    return this->GlobalGenerator->Build(
+      jobs, "", dir, projName, targets, verbose_ostr, "", config, buildOptions,
+      verbose, cmDuration::zero(), cmSystemTools::OUTPUT_PASSTHROUGH,
+      nativeOptions);
+  };
+
+#if !defined(CMAKE_BOOTSTRAP)
+  int buildresult =
+    instrumentation.InstrumentCommand("cmakeBuild", args, doBuild);
+  instrumentation.CollectTimingData(
+    cmInstrumentationQuery::Hook::PostCMakeBuild);
+#else
+  int buildresult = doBuild();
+#endif
 
   return buildresult;
 }

Source/cmake.h

@@ -640,7 +640,8 @@ public:
   int Build(int jobs, std::string dir, std::vector<std::string> targets,
             std::string config, std::vector<std::string> nativeOptions,
             cmBuildOptions& buildOptions, bool verbose,
-            std::string const& presetName, bool listPresets);
+            std::string const& presetName, bool listPresets,
+            std::vector<std::string> const& args);
 
   //! run the --open option
   bool Open(std::string const& dir, bool dryRun);

Source/cmakemain.cxx

@@ -705,27 +705,12 @@ int do_build(int ac, char const* const* av)
     cmakemainProgressCallback(msg, prog, &cm);
   });
 
-  cmInstrumentation instrumentation(dir);
-  if (!instrumentation.errorMsg.empty()) {
-    cmSystemTools::Error(instrumentation.errorMsg);
-    return 1;
-  }
   cmBuildOptions buildOptions(cleanFirst, false, resolveMode);
-  std::function<int()> doBuild = [&cm, &jobs, &dir, &targets, &config,
-                                  &nativeOptions, &buildOptions, &verbose,
-                                  &presetName, &listPresets]() {
-    return cm.Build(jobs, dir, std::move(targets), std::move(config),
-                    std::move(nativeOptions), buildOptions, verbose,
-                    presetName, listPresets);
-  };
-  instrumentation.CollectTimingData(
-    cmInstrumentationQuery::Hook::PreCMakeBuild);
   std::vector<std::string> cmd;
   cm::append(cmd, av, av + ac);
-  int ret = instrumentation.InstrumentCommand("cmakeBuild", cmd, doBuild);
-  instrumentation.CollectTimingData(
-    cmInstrumentationQuery::Hook::PostCMakeBuild);
-  return ret;
+  return cm.Build(jobs, dir, std::move(targets), std::move(config),
+                  std::move(nativeOptions), buildOptions, verbose, presetName,
+                  listPresets, cmd);
 #endif
 }
 
@@ -986,8 +971,7 @@ int do_install(int ac, char const* const* av)
     }
   }
 
-  std::function<int()> doInstall = [&handler, &verbose, &jobs,
-                                    &instrumentation]() -> int {
+  auto doInstall = [&handler, &verbose, &jobs, &instrumentation]() -> int {
     int ret_ = 0;
     if (handler.IsParallel()) {
       ret_ = handler.Install(jobs, instrumentation);

Tests/RunCMake/Instrumentation/check-data-dir.cmake

@@ -107,6 +107,15 @@ foreach(snippet IN LISTS snippets)
       snippet_error(${snippet} "Unexpected config: ${config}")
     endif()
   endif()
+
+  # Verify command args were passed
+  if (filename MATCHES "^cmakeBuild|^ctest")
+    string(JSON command GET "${contents}" command)
+    if (NOT command MATCHES "-.* Debug")
+      snippet_error(${snippet} "Command value missing passed arguments")
+    endif()
+  endif()
+
 endforeach()
 
 # Verify that listed snippets match expected roles