From 16e24748c5092f19d53c80548083826f009aafc7 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 Nov 2021 11:58:06 -0400 Subject: Ninja Multi-Config: Fix cross-config custom command dependency tracing Process `CMAKE_CROSS_CONFIGS` and friends to properly configure the generator for cross-config behavior before custom command dependency tracing. --- Source/cmGlobalGenerator.cxx | 3 +++ Source/cmGlobalGenerator.h | 2 ++ Source/cmGlobalNinjaGenerator.cxx | 3 --- Source/cmGlobalNinjaGenerator.h | 2 -- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index ac283ab..9914902 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -1402,6 +1402,9 @@ bool cmGlobalGenerator::Compute() this->SupportsDefaultConfigs())) { return false; } + if (!this->InspectConfigTypeVariables()) { + return false; + } // Some generators track files replaced during the Generate. // Start with an empty vector: diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h index cc0ad29..96696aa 100644 --- a/Source/cmGlobalGenerator.h +++ b/Source/cmGlobalGenerator.h @@ -153,6 +153,8 @@ public: */ virtual void Configure(); + virtual bool InspectConfigTypeVariables() { return true; } + bool Compute(); virtual void AddExtraIDETargets() {} diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 3f6f55e..7122b9f 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -568,9 +568,6 @@ void cmGlobalNinjaGenerator::Generate() msg.str()); return; } - if (!this->InspectConfigTypeVariables()) { - return; - } if (!this->OpenBuildFileStreams()) { return; } diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index ec73475..84fc06c 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -481,8 +481,6 @@ protected: const std::set& all, const std::set& defaults, const std::vector& items); - virtual bool InspectConfigTypeVariables() { return true; } - std::set CrossConfigs; std::set DefaultConfigs; std::string DefaultFileConfig; -- cgit v0.12 From a8833639356204d4e3f0ff180c0ff102644a7bad Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 Nov 2021 13:26:48 -0400 Subject: Ninja Multi-Config: Fix internal cross-config target dependency ordering In commit 7abc3d61ac (Ninja Multi-Config: Fix issue with framework dependencies and Autogen, 2020-02-13, v3.17.0-rc2~18^2) the `cmLinkItem` comparison operator was updated to order identical strings by the cross-config boolean. We need to order identical targets that way too in order to represent both a cross and non-cross dependency on the same target. Issue: #22855 --- Source/cmLinkItem.cxx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Source/cmLinkItem.cxx b/Source/cmLinkItem.cxx index 4e50d70..62e7ef4 100644 --- a/Source/cmLinkItem.cxx +++ b/Source/cmLinkItem.cxx @@ -32,7 +32,11 @@ bool operator<(cmLinkItem const& l, cmLinkItem const& r) { // Order among targets. if (l.Target && r.Target) { - return l.Target < r.Target; + if (l.Target != r.Target) { + return l.Target < r.Target; + } + // Order identical targets via cross-config. + return l.Cross < r.Cross; } // Order targets before strings. if (l.Target) { @@ -42,10 +46,10 @@ bool operator<(cmLinkItem const& l, cmLinkItem const& r) return false; } // Order among strings. - if (l.String < r.String) { - return true; + if (l.String != r.String) { + return l.String < r.String; } - // Order among cross-config. + // Order identical strings via cross-config. return l.Cross < r.Cross; } -- cgit v0.12 From 95f44e00cd1fd90a68f466b432198ca98456cce7 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 Nov 2021 12:02:10 -0400 Subject: Ninja Multi-Config: Fix custom command target dependencies in cross-configs Generator expressions in a non-cross custom command's `COMMAND` arguments are evaluated in the command config. Target-level dependencies implied by `TARGET_FILE` must therefore be cross dependencies. This is important to generate proper target-level dependencies on the cross-config build statements for the target to which the custom command is attached. Fixes: #22855 --- Source/cmCustomCommandGenerator.cxx | 12 ++++++++---- ...cho_depend_target-debug-in-release-graph-ninja-stdout.txt | 2 ++ ...mandOutputGenex-echo_depend_target-debug-ninja-stdout.txt | 2 ++ ...ndOutputGenex-echo_depend_target-release-ninja-stdout.txt | 2 ++ .../RunCMake/NinjaMultiConfig/CustomCommandOutputGenex.cmake | 10 ++++++++++ Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake | 10 ++++++++++ 6 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-in-release-graph-ninja-stdout.txt create mode 100644 Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-ninja-stdout.txt create mode 100644 Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-release-ninja-stdout.txt diff --git a/Source/cmCustomCommandGenerator.cxx b/Source/cmCustomCommandGenerator.cxx index 04d09d4..fd0a63c 100644 --- a/Source/cmCustomCommandGenerator.cxx +++ b/Source/cmCustomCommandGenerator.cxx @@ -91,7 +91,7 @@ std::string EvaluateSplitConfigGenex( // Record targets referenced by the genex. if (utils) { - // FIXME: What is the proper condition for a cross-dependency? + // Use a cross-dependency if we referenced the command config. bool const cross = !useOutputConfig; for (cmGeneratorTarget* gt : cge->GetTargets()) { utils->emplace(BT>( @@ -176,6 +176,8 @@ cmCustomCommandGenerator::cmCustomCommandGenerator( cmGeneratorTarget const* target{ lg->FindGeneratorTargetToUse( this->Target) }; + bool const distinctConfigs = this->OutputConfig != this->CommandConfig; + const cmCustomCommandLines& cmdlines = this->CC->GetCommandLines(); for (cmCustomCommandLine const& cmdline : cmdlines) { cmCustomCommandLine argv; @@ -191,8 +193,10 @@ cmCustomCommandGenerator::cmCustomCommandGenerator( argv.push_back(std::move(parsed_arg)); } - // For remaining arguments, we default to the OUTPUT_CONFIG. - useOutputConfig = true; + if (distinctConfigs) { + // For remaining arguments, we default to the OUTPUT_CONFIG. + useOutputConfig = true; + } } if (!argv.empty()) { @@ -200,7 +204,7 @@ cmCustomCommandGenerator::cmCustomCommandGenerator( // collect the target to add a target-level dependency on it. cmGeneratorTarget* gt = this->LG->FindGeneratorTargetToUse(argv.front()); if (gt && gt->GetType() == cmStateEnums::EXECUTABLE) { - // FIXME: What is the proper condition for a cross-dependency? + // GetArgv0Location uses the command config, so use a cross-dependency. bool const cross = true; this->Utilities.emplace(BT>( { gt->GetName(), cross }, cc.GetBacktrace())); diff --git a/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-in-release-graph-ninja-stdout.txt b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-in-release-graph-ninja-stdout.txt new file mode 100644 index 0000000..80e9c2f --- /dev/null +++ b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-in-release-graph-ninja-stdout.txt @@ -0,0 +1,2 @@ +^\[1/1\] Generating echo_depend_target\.txt +'[^']*[\/]Tests[\/]RunCMake[\/]NinjaMultiConfig[\/]CustomCommandOutputGenex-build'\$ '[^']*[\/]Tests[\/]RunCMake[\/]NinjaMultiConfig[\/]CustomCommandOutputGenex-build[\/]Release[\/]echo(\.exe)?' 'Release'$ diff --git a/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-ninja-stdout.txt b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-ninja-stdout.txt new file mode 100644 index 0000000..1a79877 --- /dev/null +++ b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-debug-ninja-stdout.txt @@ -0,0 +1,2 @@ +^\[1/1\] Generating echo_depend_target\.txt +'[^']*[\/]Tests[\/]RunCMake[\/]NinjaMultiConfig[\/]CustomCommandOutputGenex-build'\$ '[^']*[\/]Tests[\/]RunCMake[\/]NinjaMultiConfig[\/]CustomCommandOutputGenex-build[\/]Debug[\/]echo(\.exe)?' 'Debug'$ diff --git a/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-release-ninja-stdout.txt b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-release-ninja-stdout.txt new file mode 100644 index 0000000..80e9c2f --- /dev/null +++ b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex-echo_depend_target-release-ninja-stdout.txt @@ -0,0 +1,2 @@ +^\[1/1\] Generating echo_depend_target\.txt +'[^']*[\/]Tests[\/]RunCMake[\/]NinjaMultiConfig[\/]CustomCommandOutputGenex-build'\$ '[^']*[\/]Tests[\/]RunCMake[\/]NinjaMultiConfig[\/]CustomCommandOutputGenex-build[\/]Release[\/]echo(\.exe)?' 'Release'$ diff --git a/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex.cmake b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex.cmake index bb68a50..2de5a3a 100644 --- a/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex.cmake +++ b/Tests/RunCMake/NinjaMultiConfig/CustomCommandOutputGenex.cmake @@ -143,6 +143,16 @@ add_custom_command( PROPERTY SYMBOLIC 1) add_custom_target(echo_dbgx DEPENDS "$<$:echo_dbgx_Debug.txt>") +# A non-cross-config custom command expresses target dependencies in command config. +add_custom_command( + OUTPUT echo_depend_target.txt + COMMAND ${CMAKE_COMMAND} -E env $ $ + # A real project should do: + # DEPENDS $ + # but here we are testing the target-level dependency implied by TARGET_FILE. + ) +add_custom_target(echo_depend_target DEPENDS echo_depend_target.txt) + add_custom_target(echo_target_raw BYPRODUCTS echo_target_raw_$.txt COMMENT echo_target_raw diff --git a/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake b/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake index c7b876c..4a0c130 100644 --- a/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake +++ b/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake @@ -344,6 +344,16 @@ run_ninja(CustomCommandOutputGenex echo_dbgx-release build-Release.ninja echo_db run_ninja(CustomCommandOutputGenex clean-release-graph build-Release.ninja -t clean) run_ninja(CustomCommandOutputGenex echo_dbgx-debug-in-release-graph build-Release.ninja echo_dbgx:Debug) run_ninja(CustomCommandOutputGenex clean-release-graph build-Release.ninja -t clean) +# echo_depend_target +run_ninja(CustomCommandOutputGenex echo_depend_target-debug-prep build-Debug.ninja echo:Debug) +run_ninja(CustomCommandOutputGenex echo_depend_target-debug build-Debug.ninja echo_depend_target) +run_ninja(CustomCommandOutputGenex clean-debug-graph build-Debug.ninja -t clean) +run_ninja(CustomCommandOutputGenex echo_depend_target-release-prep build-Release.ninja echo:Release) +run_ninja(CustomCommandOutputGenex echo_depend_target-release build-Release.ninja echo_depend_target) +run_ninja(CustomCommandOutputGenex clean-release-graph build-Release.ninja -t clean) +run_ninja(CustomCommandOutputGenex echo_depend_target-debug-in-release-graph-prep build-Release.ninja echo:Release) +run_ninja(CustomCommandOutputGenex echo_depend_target-debug-in-release-graph build-Release.ninja echo_depend_target:Debug) +run_ninja(CustomCommandOutputGenex clean-release-graph build-Release.ninja -t clean) # echo_target_raw run_ninja(CustomCommandOutputGenex echo_target_raw-debug build-Debug.ninja echo_target_raw:Debug) run_ninja(CustomCommandOutputGenex clean-debug-graph build-Debug.ninja -t clean) -- cgit v0.12