From a30cf4a66ac5a56d16807402400eeb69dbad5ae4 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 4 Mar 2025 11:09:51 -0500 Subject: [PATCH 1/2] Tests/RunCMake/Configure: Split ninja-specific RerunCMake case Split the test case from commit f50fb77a4f (Ninja: Regenerate when test or install scripts are missing, 2024-10-29, v4.0.0-rc1~516^2) into a dedicated case. --- .../Configure/RerunCMake-build5-check.cmake | 4 ---- .../RerunCMakeNinja-build1-check.cmake | 4 ++++ .../RunCMake/Configure/RerunCMakeNinja.cmake | 4 ++++ Tests/RunCMake/Configure/RunCMakeTest.cmake | 23 +++++++++++++------ 4 files changed, 24 insertions(+), 11 deletions(-) delete mode 100644 Tests/RunCMake/Configure/RerunCMake-build5-check.cmake create mode 100644 Tests/RunCMake/Configure/RerunCMakeNinja-build1-check.cmake create mode 100644 Tests/RunCMake/Configure/RerunCMakeNinja.cmake diff --git a/Tests/RunCMake/Configure/RerunCMake-build5-check.cmake b/Tests/RunCMake/Configure/RerunCMake-build5-check.cmake deleted file mode 100644 index d740671b07..0000000000 --- a/Tests/RunCMake/Configure/RerunCMake-build5-check.cmake +++ /dev/null @@ -1,4 +0,0 @@ -file(READ ${stamp} content) -if(NOT content STREQUAL 5) - set(RunCMake_TEST_FAILED "Expected stamp '5' but got: '${content}'") -endif() diff --git a/Tests/RunCMake/Configure/RerunCMakeNinja-build1-check.cmake b/Tests/RunCMake/Configure/RerunCMakeNinja-build1-check.cmake new file mode 100644 index 0000000000..3fb557fba6 --- /dev/null +++ b/Tests/RunCMake/Configure/RerunCMakeNinja-build1-check.cmake @@ -0,0 +1,4 @@ +file(READ ${stamp} content) +if(NOT content STREQUAL 1) + set(RunCMake_TEST_FAILED "Expected stamp '1' but got: '${content}'") +endif() diff --git a/Tests/RunCMake/Configure/RerunCMakeNinja.cmake b/Tests/RunCMake/Configure/RerunCMakeNinja.cmake new file mode 100644 index 0000000000..da0131f76a --- /dev/null +++ b/Tests/RunCMake/Configure/RerunCMakeNinja.cmake @@ -0,0 +1,4 @@ +set(input ${CMAKE_CURRENT_BINARY_DIR}/input.txt) +set(stamp ${CMAKE_CURRENT_BINARY_DIR}/stamp.txt) +file(READ ${input} content) +file(WRITE ${stamp} "${content}") diff --git a/Tests/RunCMake/Configure/RunCMakeTest.cmake b/Tests/RunCMake/Configure/RunCMakeTest.cmake index 57cf947b6b..b29640a411 100644 --- a/Tests/RunCMake/Configure/RunCMakeTest.cmake +++ b/Tests/RunCMake/Configure/RunCMakeTest.cmake @@ -37,16 +37,25 @@ block() set(RunCMake_TEST_OUTPUT_MERGE 0) run_cmake_command(RerunCMake-build4 ${CMAKE_COMMAND} --build .) endif() - if(RunCMake_GENERATOR MATCHES "^Ninja") - file(REMOVE "${error}") - run_cmake(RerunCMake) +endblock() + +if(RunCMake_GENERATOR MATCHES "^Ninja") + block() + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/RerunCMakeNinja-build) + set(RunCMake_TEST_NO_CLEAN 1) + file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}") + file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}") + set(input "${RunCMake_TEST_BINARY_DIR}/input.txt") + set(stamp "${RunCMake_TEST_BINARY_DIR}/stamp.txt") + file(WRITE "${input}" "0") + run_cmake(RerunCMakeNinja) execute_process(COMMAND ${CMAKE_COMMAND} -E sleep 1) # handle 1s resolution # remove cmake_install.cmake to trigger rerun file(REMOVE "${RunCMake_TEST_BINARY_DIR}/cmake_install.cmake") - file(WRITE "${input}" "5") - run_cmake_command(RerunCMake-build5 ${CMAKE_COMMAND} --build .) - endif() -endblock() + file(WRITE "${input}" "1") + run_cmake_command(RerunCMakeNinja-build1 ${CMAKE_COMMAND} --build .) + endblock() +endif() block() set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/RemoveCache-build) From 5a36d0c9e7d7b7e3988b54a3f38b8e66f52da7e3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 4 Mar 2025 17:46:38 +0100 Subject: [PATCH 2/2] Ninja: Fix regression with a large number of subdirectories Since commit f50fb77a4f (Ninja: Regenerate when test or install scripts are missing, 2024-10-29, v4.0.0-rc1~516^2) the list of paths we pass to `ninja -t restat` scales with the number of project subdirectories. Run it in blocks to avoid "command line too long" errors, particularly on Windows. Fixes: #26738 --- Source/cmGlobalNinjaGenerator.cxx | 31 ++++++++++++------- .../RunCMake/Configure/RerunCMakeNinja.cmake | 12 +++++++ .../Configure/RerunCMakeNinja/CMakeLists.txt | 1 + 3 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 Tests/RunCMake/Configure/RerunCMakeNinja/CMakeLists.txt diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index ce96c2656f..fe07b925cd 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -7,11 +7,11 @@ #include #include #include +#include #include #include #include -#include #include #include #include @@ -663,6 +663,7 @@ void cmGlobalNinjaGenerator::Generate() void cmGlobalNinjaGenerator::CleanMetaData() { + constexpr size_t ninja_tool_arg_size = 8; // 2 `-_` flags and 4 separators auto run_ninja_tool = [this](std::vector const& args) { std::vector command; command.push_back(this->NinjaCommand); @@ -705,19 +706,27 @@ void cmGlobalNinjaGenerator::CleanMetaData() run_ninja_tool({ "recompact" }); } 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 - // outputs for the reconfigure build statement will need to be listed here. cmNinjaDeps outputs; this->AddRebuildManifestOutputs(outputs); - std::vector args; - args.reserve(outputs.size() + 1); - args.push_back("restat"); - for (auto const& output : outputs) { - args.push_back(output.c_str()); + auto output_it = outputs.begin(); + size_t static_arg_size = ninja_tool_arg_size + this->NinjaCommand.size() + + this->GetCMakeInstance()->GetHomeOutputDirectory().size(); + // The Windows command-line length limit is 32768. Leave plenty. + constexpr size_t maximum_arg_size = 30000; + while (output_it != outputs.end()) { + size_t total_arg_size = static_arg_size; + std::vector args; + args.reserve(std::distance(output_it, outputs.end()) + 1); + args.push_back("restat"); + total_arg_size += 7; // restat + 1 + while (output_it != outputs.end() && + total_arg_size + output_it->size() + 1 < maximum_arg_size) { + args.push_back(output_it->c_str()); + total_arg_size += output_it->size() + 1; + ++output_it; + } + run_ninja_tool(args); } - run_ninja_tool(args); } } diff --git a/Tests/RunCMake/Configure/RerunCMakeNinja.cmake b/Tests/RunCMake/Configure/RerunCMakeNinja.cmake index da0131f76a..35e11d4fa8 100644 --- a/Tests/RunCMake/Configure/RerunCMakeNinja.cmake +++ b/Tests/RunCMake/Configure/RerunCMakeNinja.cmake @@ -2,3 +2,15 @@ set(input ${CMAKE_CURRENT_BINARY_DIR}/input.txt) set(stamp ${CMAKE_CURRENT_BINARY_DIR}/stamp.txt) file(READ ${input} content) file(WRITE ${stamp} "${content}") + +# Add enough subdirectories to make the total list of paths to 'cmake_install.cmake' +# files exceed the Windows command-line length limit. +set(length 0) +foreach(i RANGE 1 1000) + if(length GREATER_EQUAL 32678) + break() + endif() + add_subdirectory(RerunCMakeNinja RerunCMakeNinja${i}) + string(LENGTH "${CMAKE_CURRENT_BINARY_DIR}/RerunCMakeNinja${i}/cmake_install.cmake" subdir_length) + math(EXPR length "${length} + ${subdir_length}") +endforeach() diff --git a/Tests/RunCMake/Configure/RerunCMakeNinja/CMakeLists.txt b/Tests/RunCMake/Configure/RerunCMakeNinja/CMakeLists.txt new file mode 100644 index 0000000000..d77c3633f2 --- /dev/null +++ b/Tests/RunCMake/Configure/RerunCMakeNinja/CMakeLists.txt @@ -0,0 +1 @@ +# Empty subdirectory, but it has a 'cmake_install.cmake'.