From 9d4883cce569ac4b8b334a797c7f31073a0b0f7e Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2020 13:23:41 -0500 Subject: Tests: Fix RunCMake.Ninja test for Ninja 1.10 With Ninja 1.10 we run the cleandead and recompact tools after generation. These require that `build.ninja` be loadable. Update the `CustomCommandJobPool` case to define the referenced job pools to make `build.ninja` loadable. --- Tests/RunCMake/Ninja/CustomCommandJobPool.cmake | 1 + 1 file changed, 1 insertion(+) 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 -- cgit v0.12 From 0944caaebbd9bbb3e5f0de44ec975571e3833a1a Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2020 13:24:56 -0500 Subject: Tests: Fix RunCMake.CMP0037 test with Ninja 1.10 The CMP0037 OLD and WARN cases that actually use reserved target names like `all` produce `build.ninja` files with duplicate build statements producing the same output. With Ninja 1.10 and above we run ninja tools at the end of generation that require `build.ninja` to be loadable. It is not loadable for these test cases, so skip them. --- Tests/RunCMake/CMP0037/RunCMakeTest.cmake | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) 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) -- cgit v0.12 From dd0a4718fd72d2d7797911d7e3544584ad4268ad Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2020 13:16:20 -0500 Subject: Ninja: Fix CMAKE_NINJA_OUTPUT_PATH_PREFIX with Ninja 1.10 The ninja 1.10 tools we use since commit fb18215904 (Ninja: clean ninja metadata once generated, 2019-05-13) expect `build.ninja` to be available and loadable. In commit 6cc74b6140 (cmGlobalNinjaGenerator: avoid cleandead and recompact in Ninja-Multi, 2020-01-22) we added a condition to exclude the tools in a case where `build.ninja` is not available. Generalize that condition using a local variable and extend it for the case that `build.ninja` is not loadable in the current directory because it is meant to be a sub-ninja for a higher directory. --- Source/cmGlobalNinjaGenerator.cxx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 2dd89e3..22c0e13 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -549,22 +549,25 @@ void cmGlobalNinjaGenerator::Generate() } }; + // Can the tools below expect 'build.ninja' to be loadable? + bool const expectBuildManifest = + !this->IsMultiConfig() && this->OutputPathPrefix.empty(); + // 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) { 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) { 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 -- cgit v0.12 From 5d92e60d81157a6cece044313d52cbe76b55ceed Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2020 13:22:15 -0500 Subject: Ninja: Skip cleandead and recompact if build.ninja is missing In error cases the `build.ninja` file may not exist. Skip running ninja tools that require it so that we do not generate additional errors. --- Source/cmGlobalNinjaGenerator.cxx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 22c0e13..c1ca2f8 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -553,18 +553,26 @@ void cmGlobalNinjaGenerator::Generate() 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 && expectBuildManifest) { + 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 && expectBuildManifest) { + if (this->NinjaSupportsUnconditionalRecompactTool && expectBuildManifest && + !missingBuildManifest) { run_ninja_tool({ "recompact" }); } if (this->NinjaSupportsRestatTool && this->OutputPathPrefix.empty()) { -- cgit v0.12 From b12b01302878853595ddbc66af5cd21436e8c737 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2020 15:08:23 -0500 Subject: Ninja: Factor metadata cleanup into dedicated method --- Source/cmGlobalNinjaGenerator.cxx | 5 +++++ Source/cmGlobalNinjaGenerator.h | 1 + 2 files changed, 6 insertions(+) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index c1ca2f8..a9f3711 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -528,6 +528,11 @@ void cmGlobalNinjaGenerator::Generate() this->CloseRulesFileStream(); this->CloseBuildFileStreams(); + this->CleanMetaData(); +} + +void cmGlobalNinjaGenerator::CleanMetaData() +{ auto run_ninja_tool = [this](std::vector const& args) { std::vector command; command.push_back(this->NinjaCommand); 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); -- cgit v0.12 From 657820a00b83cb28e018a94af2a8bf7475827c8b Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2020 15:25:49 -0500 Subject: Ninja: Track when running to re-generate during a build Tell CMake explicitly when it is re-running inside a `ninja` invocation to re-generate the build system. --- Source/cmGlobalGenerator.cxx | 1 + Source/cmGlobalNinjaGenerator.cxx | 2 +- Source/cmake.cxx | 2 ++ Source/cmake.h | 3 +++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index 38ff3ae..5c6bc5c 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -2525,6 +2525,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 a9f3711..f92ffd3 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1530,7 +1530,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/cmake.cxx b/Source/cmake.cxx index ab76df9..d125d0d 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -663,6 +663,8 @@ void cmake::SetArgs(const std::vector& 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 22d3c39..4195e8e 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 FileTimeCache; std::string GraphVizFile; InstalledFilesMap InstalledFiles; -- cgit v0.12 From ccaa0bccc42a31f7515c6f85a0601ac0ad006e96 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2020 15:31:53 -0500 Subject: Ninja: Do not clean metadata when re-generating inside a running build When `ninja` re-runs CMake to re-generate the build system, do not try to use the ninja tools to update metadata on Windows. The outer ninja process is already holding the files open. Issue: #20274 --- Source/cmGlobalNinjaGenerator.cxx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index f92ffd3..d093c43 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -528,7 +528,15 @@ void cmGlobalNinjaGenerator::Generate() this->CloseRulesFileStream(); this->CloseBuildFileStreams(); - this->CleanMetaData(); +#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() -- cgit v0.12