Merge topic 'nmc-cross-config-target-deps'

95f44e00cd Ninja Multi-Config: Fix custom command target dependencies in cross-configs
a883363935 Ninja Multi-Config: Fix internal cross-config target dependency ordering
16e24748c5 Ninja Multi-Config: Fix cross-config custom command dependency tracing

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

11 files changed, 47 insertions, 13 deletions

diff --git a/Source/cmCustomCommandGenerator.cxx b/Source/cmCustomCommandGenerator.cxx
index 105ccf1..41d4442 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<std::pair<std::string, bool>>(
@@ -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<std::pair<std::string, bool>>(
           { gt->GetName(), cross }, cc.GetBacktrace()));
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 38034d4..94eec2e 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -1456,6 +1456,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 d5b5eb0..19c4ee3 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<std::string>& all, const std::set<std::string>& defaults,
     const std::vector<std::string>& items);
 
-  virtual bool InspectConfigTypeVariables() { return true; }
-
   std::set<std::string> CrossConfigs;
   std::set<std::string> DefaultConfigs;
   std::string DefaultFileConfig;
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;
 }
 
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 "$<$<CONFIG:Debug>: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 $<TARGET_FILE:echo> $<CONFIG>
+  # A real project should do:
+  #   DEPENDS $<TARGET_FILE:echo>
+  # 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_$<CONFIG>.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)