Merge topic 'instrumentation-always-record-build'

e730008d7e instrumentation: Record build snippets regardless of hooks

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

Help/manual/cmake-instrumentation.7.rst

@@ -350,8 +350,8 @@ and contain the following data:
     * ``compile``: an individual compile step invoked during the build
     * ``link``: an individual link step invoked during the build
     * ``custom``: an individual custom command invoked during the build
-    * ``build``: a complete ``make`` or ``ninja`` invocation.
-      Only generated if ``preBuild`` or ``postBuild`` hooks are enabled.
+    * ``build``: a complete ``make`` or ``ninja`` invocation
+      (not through ``cmake --build``).
     * ``cmakeBuild``: a complete ``cmake --build`` invocation
     * ``cmakeInstall``: a complete ``cmake --install`` invocation
     * ``install``: an individual ``cmake -P cmake_install.cmake`` invocation

Source/cmGlobalNinjaGenerator.cxx

@@ -1779,9 +1779,7 @@ void cmGlobalNinjaGenerator::WriteBuiltinTargets(std::ostream& os)
   this->WriteTargetClean(os);
   this->WriteTargetHelp(os);
 #ifndef CMAKE_BOOTSTRAP
-  if (this->GetCMakeInstance()
-        ->GetInstrumentation()
-        ->HasPreOrPostBuildHook()) {
+  if (this->GetCMakeInstance()->GetInstrumentation()->HasQuery()) {
     this->WriteTargetInstrument(os);
   }
 #endif
@@ -1860,9 +1858,7 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os)
   reBuild.ImplicitDeps.push_back(this->CMakeCacheFile);
 
 #ifndef CMAKE_BOOTSTRAP
-  if (this->GetCMakeInstance()
-        ->GetInstrumentation()
-        ->HasPreOrPostBuildHook()) {
+  if (this->GetCMakeInstance()->GetInstrumentation()->HasQuery()) {
     reBuild.ExplicitDeps.push_back(this->NinjaOutputPath("start_instrument"));
   }
 #endif

Source/cmInstrumentation.cxx

@@ -306,12 +306,6 @@ bool cmInstrumentation::HasHook(cmInstrumentationQuery::Hook hook) const
   return (this->hooks.find(hook) != this->hooks.end());
 }
 
-bool cmInstrumentation::HasPreOrPostBuildHook() const
-{
-  return (this->HasHook(cmInstrumentationQuery::Hook::PreBuild) ||
-          this->HasHook(cmInstrumentationQuery::Hook::PostBuild));
-}
-
 int cmInstrumentation::CollectTimingData(cmInstrumentationQuery::Hook hook)
 {
   // Don't run collection if hook is disabled
@@ -745,18 +739,16 @@ int cmInstrumentation::SpawnBuildDaemon()
   }
 
   // postBuild Hook
-  if (this->HasHook(cmInstrumentationQuery::Hook::PostBuild)) {
-    auto ppid = uv_os_getppid();
-    if (ppid) {
-      std::vector<std::string> args;
-      args.push_back(cmSystemTools::GetCTestCommand());
-      args.push_back("--wait-and-collect-instrumentation");
-      args.push_back(this->binaryDir);
-      args.push_back(std::to_string(ppid));
-      auto builder = cmUVProcessChainBuilder().SetDetached().AddCommand(args);
-      auto chain = builder.Start();
-      uv_run(&chain.GetLoop(), UV_RUN_DEFAULT);
-    }
+  auto ppid = uv_os_getppid();
+  if (ppid) {
+    std::vector<std::string> args;
+    args.push_back(cmSystemTools::GetCTestCommand());
+    args.push_back("--wait-and-collect-instrumentation");
+    args.push_back(this->binaryDir);
+    args.push_back(std::to_string(ppid));
+    auto builder = cmUVProcessChainBuilder().SetDetached().AddCommand(args);
+    auto chain = builder.Start();
+    uv_run(&chain.GetLoop(), UV_RUN_DEFAULT);
   }
   return 0;
 }

Source/cmInstrumentation.h

@@ -55,7 +55,6 @@ public:
   bool HasQuery() const;
   bool HasOption(cmInstrumentationQuery::Option option) const;
   bool HasHook(cmInstrumentationQuery::Hook hook) const;
-  bool HasPreOrPostBuildHook() const;
   bool ReadJSONQueries(std::string const& directory);
   void ReadJSONQuery(std::string const& file);
   void WriteJSONQuery(std::set<cmInstrumentationQuery::Option> const& options,

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -79,7 +79,7 @@ std::string cmSplitExtension(std::string const& in, std::string& base)
 void addInstrumentationCommand(cmInstrumentation* instrumentation,
                                std::vector<std::string>& commands)
 {
-  if (instrumentation->HasPreOrPostBuildHook()) {
+  if (instrumentation->HasQuery()) {
     std::string instrumentationCommand =
       "$(CTEST_COMMAND) --start-instrumentation $(CMAKE_BINARY_DIR)";
 #  ifndef _WIN32
@@ -699,9 +699,7 @@ void cmLocalUnixMakefileGenerator3::WriteMakeVariables(
                  << cmakeShellCommand << "\n";
 
 #ifndef CMAKE_BOOTSTRAP
-  if (this->GetCMakeInstance()
-        ->GetInstrumentation()
-        ->HasPreOrPostBuildHook() &&
+  if (this->GetCMakeInstance()->GetInstrumentation()->HasQuery() &&
       // FIXME(#27079): This does not work for MSYS Makefiles.
       this->GlobalGenerator->GetName() != "MSYS Makefiles") {
     std::string ctestShellCommand =

Tests/RunCMake/Instrumentation/RunCMakeTest.cmake

@@ -239,4 +239,7 @@ if(NOT Skip_BUILD_MAKE_PROGRAM_Case)
   instrument(cmake-command-make-program
     NO_WARN BUILD_MAKE_PROGRAM
     CHECK_SCRIPT check-make-program-hooks.cmake)
+  instrument(cmake-command-build-snippet
+    NO_WARN BUILD_MAKE_PROGRAM
+    CHECK_SCRIPT check-data-dir.cmake)
 endif()

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

@@ -16,7 +16,7 @@ foreach(snippet IN LISTS snippets)
   verify_snippet_file("${snippet}" "${contents}")
 
   # Append to list of collected snippet roles
-  if (NOT role IN_LIST FOUND_SNIPPETS)
+  if (NOT role IN_LIST FOUND_SNIPPETS AND NOT role STREQUAL build)
     list(APPEND FOUND_SNIPPETS ${role})
   endif()
 
@@ -133,8 +133,11 @@ endforeach()
 
 # Verify that listed snippets match expected roles
 set(EXPECTED_SNIPPETS configure generate)
-if (ARGS_BUILD)
-  list(APPEND EXPECTED_SNIPPETS compile link custom cmakeBuild)
+if (ARGS_BUILD OR ARGS_BUILD_MAKE_PROGRAM)
+  list(APPEND EXPECTED_SNIPPETS compile link custom)
+  if (ARGS_BUILD)
+    list(APPEND EXPECTED_SNIPPETS cmakeBuild)
+  endif()
 endif()
 if (ARGS_TEST)
   list(APPEND EXPECTED_SNIPPETS ctest test)
@@ -165,3 +168,21 @@ endif()
 if (ARGS_TEST AND NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/Testing)
   add_error("ctest --instrument launcher failed to test the project")
 endif()
+
+# Look for build snippet, which may not appear immediately
+if (ARGS_BUILD_MAKE_PROGRAM)
+  set(NUM_TRIES 30)
+  set(DELAY 1)
+  set(foundBuildSnippet 0)
+  foreach(_ RANGE ${NUM_TRIES})
+    file(GLOB snippets LIST_DIRECTORIES false ${v1}/data/build-*)
+    if (snippets MATCHES build)
+      set(foundBuildSnippet 1)
+      break()
+    endif()
+    execute_process(COMMAND ${CMAKE_COMMAND} -E sleep ${DELAY})
+  endforeach()
+  if (NOT foundBuildSnippet)
+    add_error("No snippet files of role \"build\" were found in ${v1}")
+  endif()
+endif()

Tests/RunCMake/Instrumentation/query/cmake-command-build-snippet.cmake

@@ -0,0 +1,4 @@
+cmake_instrumentation(
+  API_VERSION 1
+  DATA_VERSION 1
+)