Merge topic 'custom-command-dedup'

45fedf0e17 Makefile: Add policy CMP0113 to avoid duplication of custom commands
844779bdc1 cmMakefileTargetGenerator: Simplify custom command output collection

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

5 files changed, 69 insertions, 11 deletions

diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx
index c449450..c877cf8 100644
--- a/Source/cmLocalUnixMakefileGenerator3.cxx
+++ b/Source/cmLocalUnixMakefileGenerator3.cxx
@@ -38,6 +38,7 @@
 #include "cmStateTypes.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
+#include "cmTargetDepend.h"
 #include "cmVersion.h"
 #include "cmake.h"
 
@@ -105,6 +106,15 @@ void cmLocalUnixMakefileGenerator3::Generate()
     if (!gt->IsInBuildSystem()) {
       continue;
     }
+
+    auto& gtVisited = this->GetCommandsVisited(gt);
+    auto& deps = this->GlobalGenerator->GetTargetDirectDepends(gt);
+    for (auto& d : deps) {
+      // Take the union of visited source files of custom commands
+      auto depVisited = this->GetCommandsVisited(d);
+      gtVisited.insert(depVisited.begin(), depVisited.end());
+    }
+
     std::unique_ptr<cmMakefileTargetGenerator> tg(
       cmMakefileTargetGenerator::New(gt));
     if (tg) {
diff --git a/Source/cmLocalUnixMakefileGenerator3.h b/Source/cmLocalUnixMakefileGenerator3.h
index 095836e..8286d67 100644
--- a/Source/cmLocalUnixMakefileGenerator3.h
+++ b/Source/cmLocalUnixMakefileGenerator3.h
@@ -19,6 +19,7 @@ class cmCustomCommandGenerator;
 class cmGeneratorTarget;
 class cmGlobalGenerator;
 class cmMakefile;
+class cmSourceFile;
 
 /** \class cmLocalUnixMakefileGenerator3
  * \brief Write a LocalUnix makefiles.
@@ -294,4 +295,13 @@ private:
   bool ColorMakefile;
   bool SkipPreprocessedSourceRules;
   bool SkipAssemblySourceRules;
+
+  std::set<cmSourceFile const*>& GetCommandsVisited(
+    cmGeneratorTarget const* target)
+  {
+    return this->CommandsVisited[target];
+  };
+
+  std::map<cmGeneratorTarget const*, std::set<cmSourceFile const*>>
+    CommandsVisited;
 };
diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx
index ebfddb9..e1fe0e5 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/Source/cmMakefileTargetGenerator.cxx
@@ -26,10 +26,12 @@
 #include "cmMakefileLibraryTargetGenerator.h"
 #include "cmMakefileUtilityTargetGenerator.h"
 #include "cmOutputConverter.h"
+#include "cmPolicies.h"
 #include "cmProperty.h"
 #include "cmRange.h"
 #include "cmRulePlaceholderExpander.h"
 #include "cmSourceFile.h"
+#include "cmSourceFileLocationKind.h"
 #include "cmState.h"
 #include "cmStateDirectory.h"
 #include "cmStateSnapshot.h"
@@ -51,6 +53,17 @@ cmMakefileTargetGenerator::cmMakefileTargetGenerator(cmGeneratorTarget* target)
   if (cmProp ruleStatus = cm->GetState()->GetGlobalProperty("RULE_MESSAGES")) {
     this->NoRuleMessages = cmIsOff(*ruleStatus);
   }
+  switch (this->GeneratorTarget->GetPolicyStatusCMP0113()) {
+    case cmPolicies::WARN:
+    case cmPolicies::OLD:
+      this->CMP0113New = false;
+      break;
+    case cmPolicies::NEW:
+    case cmPolicies::REQUIRED_IF_USED:
+    case cmPolicies::REQUIRED_ALWAYS:
+      this->CMP0113New = true;
+      break;
+  }
   MacOSXContentGenerator = cm::make_unique<MacOSXContentGeneratorType>(this);
 }
 
@@ -217,6 +230,12 @@ void cmMakefileTargetGenerator::WriteTargetBuildRules()
   this->GeneratorTarget->GetCustomCommands(customCommands,
                                            this->GetConfigName());
   for (cmSourceFile const* sf : customCommands) {
+    if (this->CMP0113New &&
+        !this->LocalGenerator->GetCommandsVisited(this->GeneratorTarget)
+           .insert(sf)
+           .second) {
+      continue;
+    }
     cmCustomCommandGenerator ccg(*sf->GetCustomCommand(),
                                  this->GetConfigName(), this->LocalGenerator);
     this->GenerateCustomRuleFile(ccg);
@@ -1290,16 +1309,7 @@ void cmMakefileTargetGenerator::DriveCustomCommands(
   std::vector<std::string>& depends)
 {
   // Depend on all custom command outputs.
-  std::vector<cmSourceFile*> sources;
-  this->GeneratorTarget->GetSourceFiles(
-    sources, this->Makefile->GetSafeDefinition("CMAKE_BUILD_TYPE"));
-  for (cmSourceFile* source : sources) {
-    if (cmCustomCommand* cc = source->GetCustomCommand()) {
-      cmCustomCommandGenerator ccg(*cc, this->GetConfigName(),
-                                   this->LocalGenerator);
-      cm::append(depends, ccg.GetOutputs());
-    }
-  }
+  cm::append(depends, this->CustomCommandOutputs);
 }
 
 void cmMakefileTargetGenerator::WriteObjectDependRules(
@@ -1346,6 +1356,22 @@ void cmMakefileTargetGenerator::GenerateCustomRuleFile(
   bool symbolic = this->WriteMakeRule(*this->BuildFileStream, nullptr, outputs,
                                       depends, commands);
 
+  // Symbolic inputs are not expected to exist, so add dummy rules.
+  if (this->CMP0113New && !depends.empty()) {
+    std::vector<std::string> no_depends;
+    std::vector<std::string> no_commands;
+    for (std::string const& dep : depends) {
+      if (cmSourceFile* dsf =
+            this->Makefile->GetSource(dep, cmSourceFileLocationKind::Known)) {
+        if (dsf->GetPropertyAsBool("SYMBOLIC")) {
+          this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, nullptr,
+                                              dep, no_depends, no_commands,
+                                              true);
+        }
+      }
+    }
+  }
+
   // If the rule has changed make sure the output is rebuilt.
   if (!symbolic) {
     this->GlobalGenerator->AddRuleHash(ccg.GetOutputs(), content.str());
@@ -1360,6 +1386,8 @@ void cmMakefileTargetGenerator::GenerateCustomRuleFile(
     this->LocalGenerator->AddImplicitDepends(this->GeneratorTarget, idi.first,
                                              objFullPath, srcFullPath);
   }
+
+  this->CustomCommandOutputs.insert(outputs.begin(), outputs.end());
 }
 
 void cmMakefileTargetGenerator::MakeEchoProgress(
diff --git a/Source/cmMakefileTargetGenerator.h b/Source/cmMakefileTargetGenerator.h
index fd6dac8..1740d54 100644
--- a/Source/cmMakefileTargetGenerator.h
+++ b/Source/cmMakefileTargetGenerator.h
@@ -196,6 +196,8 @@ protected:
   unsigned long NumberOfProgressActions;
   bool NoRuleMessages;
 
+  bool CMP0113New = false;
+
   // the path to the directory the build file is in
   std::string TargetBuildDirectory;
   std::string TargetBuildDirectoryFull;
@@ -228,6 +230,9 @@ protected:
   // Set of extra output files to be driven by the build.
   std::set<std::string> ExtraFiles;
 
+  // Set of custom command output files to be driven by the build.
+  std::set<std::string> CustomCommandOutputs;
+
   using MultipleOutputPairsType = std::map<std::string, std::string>;
   MultipleOutputPairsType MultipleOutputPairs;
   bool WriteMakeRule(std::ostream& os, const char* comment,
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 5b286d4..f9ec0d6 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -333,6 +333,10 @@ class cmMakefile;
   SELECT(POLICY, CMP0112,                                                     \
          "Target file component generator expressions do not add target "     \
          "dependencies.",                                                     \
+         3, 19, 0, cmPolicies::WARN)                                          \
+  SELECT(POLICY, CMP0113,                                                     \
+         "Makefile generators do not repeat custom commands from target "     \
+         "dependencies.",                                                     \
          3, 19, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
@@ -367,7 +371,8 @@ class cmMakefile;
   F(CMP0104)                                                                  \
   F(CMP0105)                                                                  \
   F(CMP0108)                                                                  \
-  F(CMP0112)
+  F(CMP0112)                                                                  \
+  F(CMP0113)
 
 /** \class cmPolicies
  * \brief Handles changes in CMake behavior and policies