Merge topic 'instrumentation-more-snippet-data'

2fdd1e787f instrumentation: Additional data in custom, install and link snippets
3db07b0189 cmInstallScriptHandler: Refactor to store config and path for each command

Acked-by: Kitware Robot <[email protected]>
Tested-by: buildbot <[email protected]>
Merge-request: !11241

Help/manual/cmake-instrumentation.7.rst

@@ -359,8 +359,9 @@ and contain the following data:
     * ``test``: a single test executed by CTest
 
   ``target``
-    The CMake target associated with the command. Only included when ``role`` is
-    ``compile`` or ``link``.
+    The CMake target associated with the command. Included when ``role`` is
+    ``compile``, or ``link``, and when ``role`` is ``custom`` and the custom
+    command is attached to a target with :ref:`add_custom_command(TARGET)`.
 
   ``targetType``
     The :prop_tgt:`TYPE` of the target. Only included when ``role`` is
@@ -391,14 +392,15 @@ and contain the following data:
 
   ``language``
     The language of the source file being compiled. Only included when ``role`` is
-    ``compile``.
+    ``compile`` or ``link``.
 
   ``testName``
     The name of the test being executed. Only included when ``role`` is ``test``.
 
   ``config``
     The type of build, such as ``Release`` or ``Debug``. Only included when
-    ``role`` is ``compile``, ``link`` or ``test``.
+    ``role`` is one of: ``compile``, ``link``, ``custom``, ``install``,
+    ``test``.
 
   ``dynamicSystemInformation``
     Specifies the dynamic information collected about the host machine

Source/cmFastbuildTargetGenerator.cxx

@@ -715,6 +715,8 @@ std::string cmFastbuildTargetGenerator::MakeCustomLauncher(
   }
   vars.Output = output.c_str();
   vars.Role = ccg.GetCC().GetRole().c_str();
+  vars.CMTargetName = ccg.GetCC().GetTarget().c_str();
+  vars.Config = ccg.GetOutputConfig().c_str();
 
   auto rulePlaceholderExpander =
     this->LocalGenerator->CreateRulePlaceholderExpander();

Source/cmInstallScriptHandler.cxx

@@ -31,6 +31,7 @@
 #include "cmUVStream.h"
 
 using InstallScript = cmInstallScriptHandler::InstallScript;
+using InstallScriptRunner = cmInstallScriptHandler::InstallScriptRunner;
 
 cmInstallScriptHandler::cmInstallScriptHandler(std::string _binaryDir,
                                                std::string _component,
@@ -45,13 +46,13 @@ cmInstallScriptHandler::cmInstallScriptHandler(std::string _binaryDir,
 
   auto addScript = [this, &args](std::string script,
                                  std::string config) -> void {
-    this->commands.push_back(args);
+    this->scripts.push_back({ script, config, args });
     if (!config.empty()) {
-      this->commands.back().insert(
-        this->commands.back().end() - 1,
+      this->scripts.back().command.insert(
+        this->scripts.back().command.end() - 1,
         cmStrCat("-DCMAKE_INSTALL_CONFIG_NAME=", config));
     }
-    this->commands.back().emplace_back(script);
+    this->scripts.back().command.emplace_back(script);
     this->directories.push_back(cmSystemTools::GetFilenamePath(script));
   };
 
@@ -97,10 +98,9 @@ bool cmInstallScriptHandler::IsParallel()
   return this->parallel;
 }
 
-std::vector<std::vector<std::string>> cmInstallScriptHandler::GetCommands()
-  const
+std::vector<InstallScript> cmInstallScriptHandler::GetScripts() const
 {
-  return this->commands;
+  return this->scripts;
 }
 
 int cmInstallScriptHandler::Install(unsigned int j,
@@ -108,8 +108,8 @@ int cmInstallScriptHandler::Install(unsigned int j,
 {
   cm::uv_loop_ptr loop;
   loop.init();
-  std::vector<InstallScript> scripts;
-  scripts.reserve(this->commands.size());
+  std::vector<InstallScriptRunner> runners;
+  runners.reserve(this->scripts.size());
 
   std::vector<std::string> instrument_arg;
   if (instrumentation.HasQuery()) {
@@ -119,26 +119,32 @@ int cmInstallScriptHandler::Install(unsigned int j,
                        "install",
                        "--build-dir",
                        this->binaryDir,
+                       "--config",
+                       "",
                        "--" };
   }
 
-  for (auto& cmd : this->commands) {
-    cmd.insert(cmd.begin(), instrument_arg.begin(), instrument_arg.end());
-    scripts.emplace_back(cmd);
+  for (auto& script : this->scripts) {
+    if (!instrument_arg.empty()) {
+      instrument_arg[7] = script.config; // --config <script.config>
+    }
+    script.command.insert(script.command.begin(), instrument_arg.begin(),
+                          instrument_arg.end());
+    runners.emplace_back(script);
   }
   std::size_t working = 0;
   std::size_t installed = 0;
   std::size_t i = 0;
 
   std::function<void()> queueScripts;
-  queueScripts = [&scripts, &working, &installed, &i, &loop, j,
+  queueScripts = [&runners, &working, &installed, &i, &loop, j,
                   &queueScripts]() {
-    for (auto queue = std::min(j - working, scripts.size() - i); queue > 0;
+    for (auto queue = std::min(j - working, runners.size() - i); queue > 0;
          --queue) {
       ++working;
-      scripts[i].start(loop,
-                       [&scripts, &working, &installed, i, &queueScripts]() {
-                         scripts[i].printResult(++installed, scripts.size());
+      runners[i].start(loop,
+                       [&runners, &working, &installed, i, &queueScripts]() {
+                         runners[i].printResult(++installed, runners.size());
                          --working;
                          queueScripts();
                        });
@@ -180,15 +186,15 @@ int cmInstallScriptHandler::Install(unsigned int j,
   return 0;
 }
 
-InstallScript::InstallScript(std::vector<std::string> const& cmd)
+InstallScriptRunner::InstallScriptRunner(InstallScript const& script)
 {
   this->name = cmSystemTools::RelativePath(
-    cmSystemTools::GetLogicalWorkingDirectory(), cmd.back());
-  this->command = cmd;
+    cmSystemTools::GetLogicalWorkingDirectory(), script.path);
+  this->command = script.command;
 }
 
-void InstallScript::start(cm::uv_loop_ptr& loop,
-                          std::function<void()> callback)
+void InstallScriptRunner::start(cm::uv_loop_ptr& loop,
+                                std::function<void()> callback)
 {
   cmUVProcessChainBuilder builder;
   builder.AddCommand(this->command)
@@ -208,7 +214,7 @@ void InstallScript::start(cm::uv_loop_ptr& loop,
     std::move(callback));
 }
 
-void InstallScript::printResult(std::size_t n, std::size_t total)
+void InstallScriptRunner::printResult(std::size_t n, std::size_t total)
 {
   cmSystemTools::Stdout(cmStrCat('[', n, '/', total, "] ", this->name, '\n'));
   for (auto const& line : this->output) {

Source/cmInstallScriptHandler.h

@@ -22,11 +22,17 @@ public:
                          std::vector<std::string>&);
   bool IsParallel();
   int Install(unsigned int j, cmInstrumentation& instrumentation);
-  std::vector<std::vector<std::string>> GetCommands() const;
-  class InstallScript
+  struct InstallScript
+  {
+    std::string path;
+    std::string config;
+    std::vector<std::string> command;
+  };
+  std::vector<InstallScript> GetScripts() const;
+  class InstallScriptRunner
   {
   public:
-    InstallScript(std::vector<std::string> const&);
+    InstallScriptRunner(InstallScript const&);
     void start(cm::uv_loop_ptr&, std::function<void()>);
     void printResult(std::size_t n, std::size_t total);
 
@@ -40,9 +46,9 @@ public:
   };
 
 private:
-  std::vector<std::vector<std::string>> commands;
-  std::vector<std::string> directories;
+  std::vector<InstallScript> scripts;
   std::vector<std::string> configs;
+  std::vector<std::string> directories;
   std::string binaryDir;
   std::string component;
   bool parallel;

Source/cmInstrumentation.cxx

@@ -636,7 +636,8 @@ int cmInstrumentation::InstrumentCommand(
 
   // Create empty config entry if config not found
   if (!root.isMember("config") &&
-      (command_type == "compile" || command_type == "link")) {
+      (command_type == "compile" || command_type == "link" ||
+       command_type == "custom" || command_type == "install")) {
     root["config"] = "";
   }
 

Source/cmLocalNinjaGenerator.cxx

@@ -912,6 +912,8 @@ std::string cmLocalNinjaGenerator::MakeCustomLauncher(
   }
   vars.Output = output.c_str();
   vars.Role = ccg.GetCC().GetRole().c_str();
+  vars.CMTargetName = ccg.GetCC().GetTarget().c_str();
+  vars.Config = ccg.GetOutputConfig().c_str();
 
   auto rulePlaceholderExpander = this->CreateRulePlaceholderExpander();
 

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -1073,6 +1073,8 @@ void cmLocalUnixMakefileGenerator3::AppendCustomCommand(
         }
         vars.Output = output.c_str();
         vars.Role = ccg.GetCC().GetRole().c_str();
+        vars.CMTargetName = ccg.GetCC().GetTarget().c_str();
+        vars.Config = ccg.GetOutputConfig().c_str();
 
         launcher = val;
         rulePlaceholderExpander->ExpandRuleVariables(this, launcher, vars);

Source/cmake.cxx

@@ -2750,19 +2750,18 @@ int cmake::ActualConfigure()
         cmStrCat('"', cmSystemTools::GetCTestCommand(), "\" --instrument ");
     }
     std::string common_args =
-      cmStrCat(" --target-name <TARGET_NAME> --build-dir \"",
+      cmStrCat(" --target-name <TARGET_NAME> --config <CONFIG> --build-dir \"",
                this->State->GetBinaryDirectory(), "\" ");
     this->State->SetGlobalProperty(
       "RULE_LAUNCH_COMPILE",
       cmStrCat(
         launcher, "--command-type compile", common_args,
-        "--config <CONFIG> "
         "--output <OBJECT> --source <SOURCE> --language <LANGUAGE> -- "));
     this->State->SetGlobalProperty(
       "RULE_LAUNCH_LINK",
       cmStrCat(
         launcher, "--command-type link", common_args,
-        "--output <TARGET> --target-type <TARGET_TYPE> --config <CONFIG> "
+        "--output <TARGET> --target-type <TARGET_TYPE> "
         "--language <LANGUAGE> --target-labels \"<TARGET_LABELS>\" -- "));
     this->State->SetGlobalProperty(
       "RULE_LAUNCH_CUSTOM",

Source/cmakemain.cxx

@@ -965,7 +965,8 @@ int do_install(int ac, char const* const* av)
     if (handler.IsParallel()) {
       ret_ = handler.Install(jobs, instrumentation);
     } else {
-      for (auto const& cmd : handler.GetCommands()) {
+      for (auto const& script : handler.GetScripts()) {
+        std::vector<std::string> cmd = script.command;
         cmake cm(cmake::RoleScript, cmState::Script);
         cmSystemTools::SetMessageCallback(
           [&cm](std::string const& msg, cmMessageMetadata const& md) {

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

@@ -87,8 +87,21 @@ foreach(snippet IN LISTS snippets)
   # Verify contents of custom-* Snippets
   if (filename MATCHES "^custom-")
     string(JSON outputs GET "${contents}" outputs)
-    if (NOT output1 MATCHES "output1" OR NOT output2 MATCHES "output2")
-      json_error("${snippet}" "Custom command missing outputs")
+    # if "outputs" is CMakeFiles/customTarget, should not have a "target"
+    if (outputs MATCHES "customTarget")
+      json_missing_key("${snippet}" "${contents}" target)
+    # if "outputs" is empty list, should have "target" main
+    elseif (outputs MATCHES "\\[\\]")
+      json_assert_key("${snippet}" "${contents}" target main)
+    # if "outputs" is includes output1, should also include output2, and no target
+    elseif (outputs MATCHES "output1")
+      if (NOT outputs MATCHES "output2")
+        json_error("${snippet}" "Custom command missing outputs")
+      endif()
+      json_missing_key("${snippet}" "${contents}" target)
+    # unrecognized outputs
+    else()
+      json_error("${snippet}" "Custom command has unexpected outputs\n${outputs}")
     endif()
   endif()
 
@@ -101,7 +114,7 @@ foreach(snippet IN LISTS snippets)
   endif()
 
   # Verify that Config is Debug
-  if (filename MATCHES "^test|^compile|^link")
+  if (filename MATCHES "^test|^compile|^link|^custom|^install")
     string(JSON config GET "${contents}" config)
     if (NOT config STREQUAL "Debug")
       json_error(${snippet} "Unexpected config: ${config}")

Tests/RunCMake/Instrumentation/project/CMakeLists.txt

@@ -9,12 +9,16 @@ endif()
 add_executable(main main.cxx)
 add_library(lib lib.cxx)
 target_link_libraries(main lib)
+add_custom_command(TARGET main POST_BUILD
+  COMMAND ${CMAKE_COMMAND} -E true
+)
 add_custom_command(
   COMMAND ${CMAKE_COMMAND} -E true
   OUTPUT output1 output2
 )
 set_property(SOURCE output1 output2 PROPERTY SYMBOLIC 1)
 add_custom_target(customTarget ALL
+  COMMAND ${CMAKE_COMMAND} -E true
   DEPENDS output1
 )
 add_test(NAME test COMMAND $<TARGET_FILE:main>)

Tests/RunCMake/Instrumentation/verify-snippet.cmake

@@ -17,6 +17,7 @@ function(snippet_has_fields snippet contents)
     json_has_key("${snippet}" "${contents}" targetType)
     json_has_key("${snippet}" "${contents}" targetLabels)
     json_has_key("${snippet}" "${contents}" config)
+    json_has_key("${snippet}" "${contents}" language)
   elseif (filename MATCHES "^compile-*")
     json_has_key("${snippet}" "${contents}" target)
     json_has_key("${snippet}" "${contents}" outputs)
@@ -27,9 +28,12 @@ function(snippet_has_fields snippet contents)
   elseif (filename MATCHES "^custom-*")
     json_has_key("${snippet}" "${contents}" outputs)
     json_has_key("${snippet}" "${contents}" outputSizes)
+    json_has_key("${snippet}" "${contents}" config)
   elseif (filename MATCHES "^test-*")
     json_has_key("${snippet}" "${contents}" testName)
     json_has_key("${snippet}" "${contents}" config)
+  elseif (filename MATCHES "^install-*")
+    json_has_key("${snippet}" "${contents}" config)
   endif()
   if(ARGS_DYNAMIC_QUERY)
     json_has_key("${snippet}" "${contents}" dynamicSystemInformation)