Merge topic 'ninja-cmd'

22e8b3af Ninja: Generate scripts for long custom command sequences

Acked-by: Kitware Robot <[email protected]>
Merge-request: !1604

Source/cmLocalNinjaGenerator.cxx

@@ -10,6 +10,7 @@
 #include <stdio.h>
 #include <utility>
 
+#include "cmCryptoHash.h"
 #include "cmCustomCommand.h"
 #include "cmCustomCommandGenerator.h"
 #include "cmGeneratedFileStream.h"
@@ -24,6 +25,7 @@
 #include "cmStateTypes.h"
 #include "cmSystemTools.h"
 #include "cmake.h"
+#include "cmsys/FStream.hxx"
 
 cmLocalNinjaGenerator::cmLocalNinjaGenerator(cmGlobalGenerator* gg,
                                              cmMakefile* mf)
@@ -286,8 +288,51 @@ void cmLocalNinjaGenerator::AppendCustomCommandDeps(
   }
 }
 
+std::string cmLocalNinjaGenerator::WriteCommandScript(
+  std::vector<std::string> const& cmdLines, std::string const& customStep,
+  cmGeneratorTarget const* target) const
+{
+  std::string scriptPath;
+  if (target) {
+    scriptPath = target->GetSupportDirectory();
+  } else {
+    scriptPath = this->GetCurrentBinaryDirectory();
+    scriptPath += cmake::GetCMakeFilesDirectory();
+  }
+  cmSystemTools::MakeDirectory(scriptPath);
+  scriptPath += '/';
+  scriptPath += customStep;
+#ifdef _WIN32
+  scriptPath += ".bat";
+#else
+  scriptPath += ".sh";
+#endif
+
+  cmsys::ofstream script(scriptPath.c_str());
+
+#ifndef _WIN32
+  script << "set -e\n\n";
+#endif
+
+  for (auto const& i : cmdLines) {
+    std::string cmd = i;
+    // The command line was built assuming it would be written to
+    // the build.ninja file, so it uses '$$' for '$'.  Remove this
+    // for the raw shell script.
+    cmSystemTools::ReplaceString(cmd, "$$", "$");
+#ifdef _WIN32
+    script << cmd << " || exit /b" << '\n';
+#else
+    script << cmd << '\n';
+#endif
+  }
+
+  return scriptPath;
+}
+
 std::string cmLocalNinjaGenerator::BuildCommandLine(
-  const std::vector<std::string>& cmdLines)
+  std::vector<std::string> const& cmdLines, std::string const& customStep,
+  cmGeneratorTarget const* target) const
 {
   // If we have no commands but we need to build a command anyway, use noop.
   // This happens when building a POST_BUILD value for link targets that
@@ -296,6 +341,35 @@ std::string cmLocalNinjaGenerator::BuildCommandLine(
     return cmGlobalNinjaGenerator::SHELL_NOOP;
   }
 
+  // If this is a custom step then we will have no '$VAR' ninja placeholders.
+  // This means we can deal with long command sequences by writing to a script.
+  // Do this if the command lines are on the scale of the OS limit.
+  if (!customStep.empty()) {
+    size_t cmdLinesTotal = 0;
+    for (std::string const& cmd : cmdLines) {
+      cmdLinesTotal += cmd.length() + 6;
+    }
+    if (cmdLinesTotal > cmSystemTools::CalculateCommandLineLengthLimit() / 2) {
+      std::string const scriptPath =
+        this->WriteCommandScript(cmdLines, customStep, target);
+      std::string cmd
+#ifndef _WIN32
+        = "/bin/sh "
+#endif
+        ;
+      cmd += this->ConvertToOutputFormat(
+        this->GetGlobalNinjaGenerator()->ConvertToNinjaPath(scriptPath),
+        cmOutputConverter::SHELL);
+
+      // Add an unused argument based on script content so that Ninja
+      // knows when the command lines change.
+      cmd += " ";
+      cmCryptoHash hash(cmCryptoHash::AlgoSHA256);
+      cmd += hash.HashFile(scriptPath).substr(0, 16);
+      return cmd;
+    }
+  }
+
   std::ostringstream cmd;
   for (std::vector<std::string>::const_iterator li = cmdLines.begin();
        li != cmdLines.end(); ++li)
@@ -406,10 +480,16 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement(
       "Phony custom command for " + ninjaOutputs[0], ninjaOutputs, ninjaDeps,
       cmNinjaDeps(), orderOnlyDeps, cmNinjaVars());
   } else {
+    std::string customStep = cmSystemTools::GetFilenameName(ninjaOutputs[0]);
+    // Hash full path to make unique.
+    customStep += '-';
+    cmCryptoHash hash(cmCryptoHash::AlgoSHA256);
+    customStep += hash.HashString(ninjaOutputs[0]).substr(0, 7);
+
     this->GetGlobalNinjaGenerator()->WriteCustomCommandBuild(
-      this->BuildCommandLine(cmdLines), this->ConstructComment(ccg),
-      "Custom command for " + ninjaOutputs[0], cc->GetDepfile(),
-      cc->GetUsesTerminal(),
+      this->BuildCommandLine(cmdLines, customStep),
+      this->ConstructComment(ccg), "Custom command for " + ninjaOutputs[0],
+      cc->GetDepfile(), cc->GetUsesTerminal(),
       /*restat*/ !symbolic || !byproducts.empty(), ninjaOutputs, ninjaDeps,
       orderOnlyDeps);
   }

Source/cmLocalNinjaGenerator.h

@@ -59,7 +59,10 @@ public:
     return this->HomeRelativeOutputPath;
   }
 
-  std::string BuildCommandLine(const std::vector<std::string>& cmdLines);
+  std::string BuildCommandLine(
+    std::vector<std::string> const& cmdLines,
+    std::string const& customStep = std::string(),
+    cmGeneratorTarget const* target = nullptr) const;
 
   void AppendTargetOutputs(cmGeneratorTarget* target, cmNinjaDeps& outputs);
   void AppendTargetDepends(
@@ -98,6 +101,10 @@ private:
 
   std::string MakeCustomLauncher(cmCustomCommandGenerator const& ccg);
 
+  std::string WriteCommandScript(std::vector<std::string> const& cmdLines,
+                                 std::string const& customStep,
+                                 cmGeneratorTarget const* target) const;
+
   std::string HomeRelativeOutputPath;
 
   typedef std::map<cmCustomCommand const*, std::set<cmGeneratorTarget*>>

Source/cmNinjaNormalTargetGenerator.cxx

@@ -976,8 +976,10 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement()
     preLinkCmdLines.push_back("cd " + homeOutDir);
   }
 
-  vars["PRE_LINK"] = localGen.BuildCommandLine(preLinkCmdLines);
-  std::string postBuildCmdLine = localGen.BuildCommandLine(postBuildCmdLines);
+  vars["PRE_LINK"] = localGen.BuildCommandLine(preLinkCmdLines, "pre-link",
+                                               this->GeneratorTarget);
+  std::string postBuildCmdLine = localGen.BuildCommandLine(
+    postBuildCmdLines, "post-build", this->GeneratorTarget);
 
   cmNinjaVars symlinkVars;
   bool const symlinkNeeded =

Source/cmNinjaUtilityTargetGenerator.cxx

@@ -96,8 +96,8 @@ void cmNinjaUtilityTargetGenerator::Generate()
       this->GetBuildFileStream(),
       "Utility command for " + this->GetTargetName(), outputs, deps);
   } else {
-    std::string command =
-      this->GetLocalGenerator()->BuildCommandLine(commands);
+    std::string command = this->GetLocalGenerator()->BuildCommandLine(
+      commands, "utility", this->GeneratorTarget);
     const char* echoStr =
       this->GetGeneratorTarget()->GetProperty("EchoString");
     std::string desc;

Tests/CMakeLists.txt

@@ -816,6 +816,8 @@ ${CMake_BINARY_DIR}/bin/cmake -DDIR=dev -P ${CMake_SOURCE_DIR}/Utilities/Release
 
   ADD_TEST_MACRO(CustomCommandByproducts CustomCommandByproducts)
 
+  ADD_TEST_MACRO(CommandLength CommandLength)
+
   ADD_TEST_MACRO(EmptyDepends ${CMAKE_CTEST_COMMAND})
 
   add_test(CustomCommandWorkingDirectory  ${CMAKE_CTEST_COMMAND}

Tests/CommandLength/CMakeLists.txt

@@ -0,0 +1,17 @@
+cmake_minimum_required(VERSION 3.10)
+project(CommandLength C)
+
+add_executable(CommandLength test.c)
+add_custom_command(TARGET CommandLength POST_BUILD VERBATIM
+  COMMAND ${CMAKE_COMMAND} -E make_directory log)
+
+set(msg "xxxx $$$$ yyyy")
+set(msg "${msg} ${msg}")
+set(msg "${msg} ${msg}")
+set(msg "${msg} ${msg}")
+set(msg "${msg} ${msg}")
+foreach(i RANGE 1 1000)
+  add_custom_command(TARGET CommandLength POST_BUILD VERBATIM
+    COMMAND ${CMAKE_COMMAND} -E echo "${i} ${msg}" > log/${i}
+    )
+endforeach()

Tests/CommandLength/test.c

@@ -0,0 +1,4 @@
+int main()
+{
+  return 0;
+}