From 9d7da095fdee68bc12c2fd960670438fc897cb8e Mon Sep 17 00:00:00 2001 From: Tyler Yankee Date: Mon, 13 Jul 2026 09:45:30 -0400 Subject: [PATCH] Ninja: Restore cmcldeps command-line ordering Restore the ordering before the refactoring in commit 0f20ba5b68 (Ninja: Swift: Extract helpers and reorder module flags, 2026-04-10, v4.4.0-rc1~273^2~1) to ensure the correct precedence with launchers. Add regression tests. Fixes: #27931, #27934 --- Source/cmNinjaTargetGenerator.cxx | 14 ++++++------- .../C-launch-Build-check.cmake | 21 +++++++++++++++++++ .../Instrumentation/RunCMakeTest.cmake | 4 ++++ .../Instrumentation/check-data-dir.cmake | 8 +++++++ 4 files changed, 40 insertions(+), 7 deletions(-) create mode 100644 Tests/RunCMake/CompilerLauncher/C-launch-Build-check.cmake diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index f2505de290..5d02ef4e73 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -725,6 +725,7 @@ cmList ExpandRuleCommands(std::string const& command, cmRulePlaceholderExpander::RuleVariables const& vars, cmMakefile const* mf, std::string const& lang, std::string const& launcher, + std::string const& cldeps, cmLocalGenerator* localGenerator, cmRulePlaceholderExpander* rulePlaceholderExpander) { @@ -735,6 +736,9 @@ cmList ExpandRuleCommands(std::string const& command, commands.front().insert(0, "${CODE_CHECK}"); commands.front().insert(0, "${LAUNCHER}"); } + if (!commands.empty()) { + commands.front().insert(0, cldeps); + } if (!extraCommands.empty()) { commands.append(extraCommands); } @@ -1038,13 +1042,9 @@ void cmNinjaTargetGenerator::WriteCompileRule(std::string const& lang, std::string const cmdVar = this->GetCompileTemplateVar(lang); std::string const& compileCmd = mf->GetRequiredDefinition(cmdVar); cmList compileCmds = ExpandRuleCommands(compileCmd, vars, mf, lang, launcher, - this->GetLocalGenerator(), + cldeps, this->GetLocalGenerator(), rulePlaceholderExpander.get()); - if (!compileCmds.empty()) { - compileCmds.front().insert(0, cldeps); - } - rule.Command = this->GetLocalGenerator()->BuildCommandLine(compileCmds, config, config); @@ -1076,8 +1076,8 @@ void cmNinjaTargetGenerator::WriteCompileRule(std::string const& lang, emitModRule.Restat = "1"; cmList emitModCmds = ExpandRuleCommands( - *emitModCmdVal, emVars, mf, lang, launcher, this->GetLocalGenerator(), - rulePlaceholderExpander.get()); + *emitModCmdVal, emVars, mf, lang, launcher, cldeps, + this->GetLocalGenerator(), rulePlaceholderExpander.get()); emitModRule.Command = this->GetLocalGenerator()->BuildCommandLine( emitModCmds, config, config); emitModRule.Comment = "Rule for emitting Swift .swiftmodule files."; diff --git a/Tests/RunCMake/CompilerLauncher/C-launch-Build-check.cmake b/Tests/RunCMake/CompilerLauncher/C-launch-Build-check.cmake new file mode 100644 index 0000000000..ac76a86570 --- /dev/null +++ b/Tests/RunCMake/CompilerLauncher/C-launch-Build-check.cmake @@ -0,0 +1,21 @@ +if(RunCMake_GENERATOR STREQUAL "Ninja") + set(rules_ninja "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/rules.ninja") + set(build_ninja "${RunCMake_TEST_BINARY_DIR}/build.ninja") + + file(READ "${rules_ninja}" ninja_content) + file(READ "${build_ninja}" build_ninja_content) + + if(NOT ninja_content MATCHES "command = \"[^\"]*ctest(\.exe)?\" --launch") + set(RunCMake_TEST_FAILED + "Expected compile command to use ctest --launch: ${ninja_content}" + ) + return() + endif() + + if(NOT build_ninja_content MATCHES "LAUNCHER = [^\n]*USED_LAUNCHER=1") + set(RunCMake_TEST_FAILED + "Expected compile command to include the compiler launcher" + ) + return() + endif() +endif() diff --git a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake index ee0a82fd93..92abc5d439 100644 --- a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake +++ b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake @@ -255,6 +255,9 @@ instrument(no-query BUILD INSTALL TEST CHECK_SCRIPT check-data-dir.cmake ) +if(RunCMake_GENERATOR STREQUAL "Ninja") + set(CHECK_NINJA_INSTRUMENT_ORDER 1) +endif() instrument(disabled-test BUILD TEST DISABLE_TEST CHECK_SCRIPT check-data-dir.cmake @@ -267,6 +270,7 @@ instrument(both-query BUILD INSTALL TEST STATIC_QUERY DYNAMIC_QUERY CAPTURE_OUTPUT_QUERY CHECK_SCRIPT check-data-dir.cmake ) +unset(CHECK_NINJA_INSTRUMENT_ORDER) # Test cmake_instrumentation command instrument(cmake-command diff --git a/Tests/RunCMake/Instrumentation/check-data-dir.cmake b/Tests/RunCMake/Instrumentation/check-data-dir.cmake index 5e38030cc4..3a73300628 100644 --- a/Tests/RunCMake/Instrumentation/check-data-dir.cmake +++ b/Tests/RunCMake/Instrumentation/check-data-dir.cmake @@ -240,6 +240,14 @@ if (ARGS_BUILD AND NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/shell_redirect.out) add_error("custom command with shell redirection did not run") endif() +if (CHECK_NINJA_INSTRUMENT_ORDER) + set(rules_ninja "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/rules.ninja") + file(READ "${rules_ninja}" ninja_content) + if (NOT ninja_content MATCHES "command = \"[^\"]*ctest(\.exe)?\" --instrument") + add_error("Expected compile command to start with ctest --instrument") + endif() +endif() + # Look for build snippet, which may not appear immediately if (ARGS_BUILD_MAKE_PROGRAM) set(NUM_TRIES 30)