Merge topic 'ninja-1.10'

ccaa0bccc4 Ninja: Do not clean metadata when re-generating inside a running build
657820a00b Ninja: Track when running to re-generate during a build
b12b013028 Ninja: Factor metadata cleanup into dedicated method
5d92e60d81 Ninja: Skip cleandead and recompact if build.ninja is missing
dd0a4718fd Ninja: Fix CMAKE_NINJA_OUTPUT_PATH_PREFIX with Ninja 1.10
0944caaebb Tests: Fix RunCMake.CMP0037 test with Ninja 1.10
9d4883cce5 Tests: Fix RunCMake.Ninja test for Ninja 1.10

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

7 files changed, 60 insertions, 7 deletions

diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 45e13bc..1f084f5 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -2527,6 +2527,7 @@ void cmGlobalGenerator::AddGlobalTarget_RebuildCache(
   gti.PerConfig = false;
   cmCustomCommandLine singleLine;
   singleLine.push_back(cmSystemTools::GetCMakeCommand());
+  singleLine.push_back("--regenerate-during-build");
   singleLine.push_back("-S$(CMAKE_SOURCE_DIR)");
   singleLine.push_back("-B$(CMAKE_BINARY_DIR)");
   gti.CommandLines.push_back(std::move(singleLine));
diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index 2dd89e3..d093c43 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -528,6 +528,19 @@ void cmGlobalNinjaGenerator::Generate()
   this->CloseRulesFileStream();
   this->CloseBuildFileStreams();
 
+#ifdef _WIN32
+  // The ninja tools will not be able to update metadata on Windows
+  // when we are re-generating inside an existing 'ninja' invocation
+  // because the outer tool has the files open for write.
+  if (!this->GetCMakeInstance()->GetRegenerateDuringBuild())
+#endif
+  {
+    this->CleanMetaData();
+  }
+}
+
+void cmGlobalNinjaGenerator::CleanMetaData()
+{
   auto run_ninja_tool = [this](std::vector<char const*> const& args) {
     std::vector<std::string> command;
     command.push_back(this->NinjaCommand);
@@ -549,22 +562,33 @@ void cmGlobalNinjaGenerator::Generate()
     }
   };
 
+  // Can the tools below expect 'build.ninja' to be loadable?
+  bool const expectBuildManifest =
+    !this->IsMultiConfig() && this->OutputPathPrefix.empty();
+
+  // Skip some ninja tools if they need 'build.ninja' but it is missing.
+  bool const missingBuildManifest = expectBuildManifest &&
+    (this->NinjaSupportsCleanDeadTool ||
+     this->NinjaSupportsUnconditionalRecompactTool) &&
+    !cmSystemTools::FileExists("build.ninja");
+
   // The `cleandead` tool needs to know about all outputs in the build we just
   // wrote out. Ninja-Multi doesn't have a single `build.ninja` we can use that
   // is the union of all generated configurations, so we can't run it reliably
   // in that case.
-  if (this->NinjaSupportsCleanDeadTool && !this->IsMultiConfig()) {
+  if (this->NinjaSupportsCleanDeadTool && expectBuildManifest &&
+      !missingBuildManifest) {
     run_ninja_tool({ "cleandead" });
   }
   // The `recompact` tool loads the manifest. As above, we don't have a single
   // `build.ninja` to load for this in Ninja-Multi. This may be relaxed in the
   // future pending further investigation into how Ninja works upstream
   // (ninja#1721).
-  if (this->NinjaSupportsUnconditionalRecompactTool &&
-      !this->IsMultiConfig()) {
+  if (this->NinjaSupportsUnconditionalRecompactTool && expectBuildManifest &&
+      !missingBuildManifest) {
     run_ninja_tool({ "recompact" });
   }
-  if (this->NinjaSupportsRestatTool) {
+  if (this->NinjaSupportsRestatTool && this->OutputPathPrefix.empty()) {
     // XXX(ninja): We only list `build.ninja` entry files here because CMake
     // *always* rewrites these files on a reconfigure. If CMake ever gets
     // smarter about this, all CMake-time created/edited files listed as
@@ -1514,7 +1538,7 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os)
   {
     cmNinjaRule rule("RERUN_CMAKE");
     rule.Command =
-      cmStrCat(CMakeCmd(), " -S",
+      cmStrCat(CMakeCmd(), " --regenerate-during-build -S",
                lg->ConvertToOutputFormat(lg->GetSourceDirectory(),
                                          cmOutputConverter::SHELL),
                " -B",
diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h
index d00a061..9d5521a 100644
--- a/Source/cmGlobalNinjaGenerator.h
+++ b/Source/cmGlobalNinjaGenerator.h
@@ -438,6 +438,7 @@ private:
 
   bool OpenRulesFileStream();
   void CloseRulesFileStream();
+  void CleanMetaData();
 
   /// Write the common disclaimer text at the top of each build file.
   void WriteDisclaimer(std::ostream& os);
diff --git a/Source/cmake.cxx b/Source/cmake.cxx
index 721d535..8214c52 100644
--- a/Source/cmake.cxx
+++ b/Source/cmake.cxx
@@ -663,6 +663,8 @@ void cmake::SetArgs(const std::vector<std::string>& args)
     } else if ((i < args.size() - 1) &&
                (arg.find("--check-stamp-list", 0) == 0)) {
       this->CheckStampList = args[++i];
+    } else if (arg == "--regenerate-during-build") {
+      this->RegenerateDuringBuild = true;
     }
 #if defined(CMAKE_HAVE_VS_GENERATORS)
     else if ((i < args.size() - 1) &&
diff --git a/Source/cmake.h b/Source/cmake.h
index c65b821..35425ec 100644
--- a/Source/cmake.h
+++ b/Source/cmake.h
@@ -547,6 +547,8 @@ public:
   }
   cmStateSnapshot GetCurrentSnapshot() const { return this->CurrentSnapshot; }
 
+  bool GetRegenerateDuringBuild() const { return this->RegenerateDuringBuild; }
+
 protected:
   void RunCheckForUnusedVariables();
   int HandleDeleteCacheVariables(const std::string& var);
@@ -621,6 +623,7 @@ private:
   FileExtensions FortranFileExtensions;
   bool ClearBuildSystem = false;
   bool DebugTryCompile = false;
+  bool RegenerateDuringBuild = false;
   std::unique_ptr<cmFileTimeCache> FileTimeCache;
   std::string GraphVizFile;
   InstalledFilesMap InstalledFiles;
diff --git a/Tests/RunCMake/CMP0037/RunCMakeTest.cmake b/Tests/RunCMake/CMP0037/RunCMakeTest.cmake
index 98274f0..5952279 100644
--- a/Tests/RunCMake/CMP0037/RunCMakeTest.cmake
+++ b/Tests/RunCMake/CMP0037/RunCMakeTest.cmake
@@ -1,5 +1,24 @@
 include(RunCMake)
 
+if(RunCMake_GENERATOR MATCHES "^Ninja")
+  # Detect ninja version so we know what tests can be supported.
+  execute_process(
+    COMMAND "${RunCMake_MAKE_PROGRAM}" --version
+    OUTPUT_VARIABLE ninja_out
+    ERROR_VARIABLE ninja_out
+    RESULT_VARIABLE ninja_res
+    OUTPUT_STRIP_TRAILING_WHITESPACE
+    )
+  if(ninja_res EQUAL 0 AND "x${ninja_out}" MATCHES "^x[0-9]+\\.[0-9]+")
+    set(ninja_version "${ninja_out}")
+    message(STATUS "ninja version: ${ninja_version}")
+  else()
+    message(FATAL_ERROR "'ninja --version' reported:\n${ninja_out}")
+  endif()
+else()
+  set(ninja_version "")
+endif()
+
 run_cmake(CMP0037-OLD-space)
 run_cmake(CMP0037-NEW-space)
 run_cmake(CMP0037-WARN-space)
@@ -9,8 +28,10 @@ if(NOT (WIN32 AND "${RunCMake_GENERATOR}" MATCHES "Make"))
   run_cmake(CMP0037-WARN-colon)
 endif()
 
-run_cmake(CMP0037-WARN-reserved)
-run_cmake(CMP0037-OLD-reserved)
+if(NOT ninja_version VERSION_GREATER_EQUAL 1.10)
+  run_cmake(CMP0037-WARN-reserved)
+  run_cmake(CMP0037-OLD-reserved)
+endif()
 run_cmake(CMP0037-NEW-reserved)
 
 run_cmake(NEW-cond)
diff --git a/Tests/RunCMake/Ninja/CustomCommandJobPool.cmake b/Tests/RunCMake/Ninja/CustomCommandJobPool.cmake
index 1e36e65..a96802a 100644
--- a/Tests/RunCMake/Ninja/CustomCommandJobPool.cmake
+++ b/Tests/RunCMake/Ninja/CustomCommandJobPool.cmake
@@ -1,3 +1,4 @@
+set_property(GLOBAL PROPERTY JOB_POOLS custom_command_pool=2 custom_target_pool=2)
 add_custom_command(
   OUTPUT hello.copy.c
   COMMAND "${CMAKE_COMMAND}" -E copy