Makefile: Add policy CMP0113 to avoid duplication of custom commands

Do not attach a custom command to a target if it is already attached to one of
the target's dependencies.  The command's output will be available by the time
the target needs it because the dependency containing the command will have
already been built.

This may break existing projects that do not properly mark non-created
outputs with the `SYMBOLIC` property.  Previously a chain of two custom
commands whose intermediate dependency is not created would put both
commands in a dependent project's Makefile even if the first command is
also in its dependency's Makefile.  The first command would run twice
but the build would work.  Now the second command needs an explicit
`SYMBOLIC` mark on its input to tell CMake that it is not expected to
exist.  To maintain compatibility with projects that left out the mark,
add a policy activating the behavior.

20 files changed, 168 insertions, 1 deletions

diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst
index cd1d4d343c..3821dc3e59 100644
--- a/Help/manual/cmake-policies.7.rst
+++ b/Help/manual/cmake-policies.7.rst
@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.19
 .. toctree::
    :maxdepth: 1
 
+   CMP0113: Makefile generators do not repeat custom commands from target dependencies. </policy/CMP0113>
    CMP0112: Target file component generator expressions do not add target dependencies. </policy/CMP0112>
    CMP0111: An imported target with a missing location fails during generation. </policy/CMP0111>
    CMP0110: add_test() supports arbitrary characters in test names. </policy/CMP0110>
diff --git a/Help/policy/CMP0113.rst b/Help/policy/CMP0113.rst
new file mode 100644
index 0000000000..6f56902d9e
--- /dev/null
+++ b/Help/policy/CMP0113.rst
@@ -0,0 +1,43 @@
+CMP0113
+-------
+
+.. versionadded:: 3.19
+
+:ref:`Makefile Generators` do not repeat custom commands from target
+dependencies.
+
+Consider a chain of custom commands split across two dependent targets:
+
+.. code-block:: cmake
+
+  add_custom_command(OUTPUT output-not-created
+    COMMAND ... DEPENDS ...)
+  set_property(SOURCE output-not-created PROPERTY SYMBOLIC 1)
+  add_custom_command(OUTPUT output-created
+    COMMAND ... DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/output-not-created)
+  add_custom_target(first DEPENDS output-not-created)
+  add_custom_target(second DEPENDS output-created)
+  add_dependencies(second first)
+
+In CMake 3.18 and lower, the Makefile generators put a copy of both custom
+commands in the Makefile for target ``second`` even though its dependency on
+target ``first`` ensures that the first custom command runs before the second.
+Running ``make second`` would cause the first custom command to run once in
+the ``first`` target and then again in the ``second`` target.
+
+CMake 3.19 and above prefer to not duplicate custom commands in a target that
+are already generated in other targets on which the target depends (directly or
+indirectly).  This policy provides compatibility for projects that have not
+been updated to expect the new behavior.  In particular, projects that relied
+on the duplicate execution or that did not properly set the :prop_sf:`SYMBOLIC`
+source file property may be affected.
+
+The ``OLD`` behavior for this policy is to duplicate custom commands in
+dependent targets.  The ``NEW`` behavior of this policy is to not duplicate
+custom commands in dependent targets.
+
+This policy was introduced in CMake version 3.19.  Unlike many policies,
+CMake version |release| does *not* warn when this policy is not set and
+simply uses ``OLD`` behavior.
+
+.. include:: DEPRECATED.txt
diff --git a/Help/release/dev/custom-command-dedup.rst b/Help/release/dev/custom-command-dedup.rst
new file mode 100644
index 0000000000..65fa303301
--- /dev/null
+++ b/Help/release/dev/custom-command-dedup.rst
@@ -0,0 +1,5 @@
+custom-command-dedup
+--------------------
+
+* :ref:`Makefile Generators` no longer repeat custom commands from target
+  dependencies.  See policy :policy:`CMP0113`.
diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx
index c449450132..c877cf8593 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 095836e5dd..8286d673fc 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 bc0725e8fc..e1fe0e5c5c 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);
@@ -1337,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());
diff --git a/Source/cmMakefileTargetGenerator.h b/Source/cmMakefileTargetGenerator.h
index b7ec442805..1740d541b8 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;
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 5b286d475f..f9ec0d6b1c 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
diff --git a/Tests/RunCMake/Make/CMP0113-Common.cmake b/Tests/RunCMake/Make/CMP0113-Common.cmake
new file mode 100644
index 0000000000..8ce26c4c7e
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-Common.cmake
@@ -0,0 +1,17 @@
+add_custom_command(
+  OUTPUT output-not-created
+  COMMAND ${CMAKE_COMMAND} -E echo output-not-created
+  DEPENDS ${CMAKE_CURRENT_LIST_FILE}
+  VERBATIM
+  )
+
+add_custom_command(
+  OUTPUT output-created
+  COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_LIST_FILE} output-created
+  DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/output-not-created
+  VERBATIM
+  )
+
+add_custom_target(drive1 ALL DEPENDS output-not-created)
+add_custom_target(drive2 ALL DEPENDS output-created)
+add_dependencies(drive2 drive1)
diff --git a/Tests/RunCMake/Make/CMP0113-NEW-build-gnu-stderr.txt b/Tests/RunCMake/Make/CMP0113-NEW-build-gnu-stderr.txt
new file mode 100644
index 0000000000..0af354a05e
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-NEW-build-gnu-stderr.txt
@@ -0,0 +1,5 @@
+No rule to make target [^
+]*output-not-created[^
+]*, needed by [^
+]*output-created[^
+]*
diff --git a/Tests/RunCMake/Make/CMP0113-NEW-build-result.txt b/Tests/RunCMake/Make/CMP0113-NEW-build-result.txt
new file mode 100644
index 0000000000..d197c913c2
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-NEW-build-result.txt
@@ -0,0 +1 @@
+[^0]
diff --git a/Tests/RunCMake/Make/CMP0113-NEW-build-stderr.txt b/Tests/RunCMake/Make/CMP0113-NEW-build-stderr.txt
new file mode 100644
index 0000000000..8d98f9debd
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-NEW-build-stderr.txt
@@ -0,0 +1 @@
+.*
diff --git a/Tests/RunCMake/Make/CMP0113-NEW-build-stdout.txt b/Tests/RunCMake/Make/CMP0113-NEW-build-stdout.txt
new file mode 100644
index 0000000000..a6b851e9f5
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-NEW-build-stdout.txt
@@ -0,0 +1 @@
+Generating output-not-created.*Built target drive1
diff --git a/Tests/RunCMake/Make/CMP0113-NEW.cmake b/Tests/RunCMake/Make/CMP0113-NEW.cmake
new file mode 100644
index 0000000000..a2d1db53f3
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-NEW.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0113 NEW)
+include(CMP0113-Common.cmake)
diff --git a/Tests/RunCMake/Make/CMP0113-OLD-build-stdout.txt b/Tests/RunCMake/Make/CMP0113-OLD-build-stdout.txt
new file mode 100644
index 0000000000..f6e1f0f47c
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-OLD-build-stdout.txt
@@ -0,0 +1 @@
+Generating output-not-created.*Built target drive1.*Generating output-not-created.*Built target drive2
diff --git a/Tests/RunCMake/Make/CMP0113-OLD.cmake b/Tests/RunCMake/Make/CMP0113-OLD.cmake
new file mode 100644
index 0000000000..b6ab1203ff
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-OLD.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0113 OLD)
+include(CMP0113-Common.cmake)
diff --git a/Tests/RunCMake/Make/CMP0113-WARN-build-stdout.txt b/Tests/RunCMake/Make/CMP0113-WARN-build-stdout.txt
new file mode 100644
index 0000000000..f6e1f0f47c
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-WARN-build-stdout.txt
@@ -0,0 +1 @@
+Generating output-not-created.*Built target drive1.*Generating output-not-created.*Built target drive2
diff --git a/Tests/RunCMake/Make/CMP0113-WARN.cmake b/Tests/RunCMake/Make/CMP0113-WARN.cmake
new file mode 100644
index 0000000000..48e035a44c
--- /dev/null
+++ b/Tests/RunCMake/Make/CMP0113-WARN.cmake
@@ -0,0 +1,2 @@
+# Policy CMP0113 not set.
+include(CMP0113-Common.cmake)
diff --git a/Tests/RunCMake/Make/RunCMakeTest.cmake b/Tests/RunCMake/Make/RunCMakeTest.cmake
index 32aafca641..5562acabdc 100644
--- a/Tests/RunCMake/Make/RunCMakeTest.cmake
+++ b/Tests/RunCMake/Make/RunCMakeTest.cmake
@@ -41,3 +41,25 @@ run_VerboseBuild()
 
 run_cmake(CustomCommandDepfile-ERROR)
 run_cmake(IncludeRegexSubdir)
+
+function(run_CMP0113 val)
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/CMP0113-${val}-build)
+  run_cmake(CMP0113-${val})
+  set(RunCMake_TEST_NO_CLEAN 1)
+  set(_backup_lang "$ENV{LANG}")
+  set(_backup_lc_Messages "$ENV{LC_MESSAGES}")
+  if(MAKE_IS_GNU)
+    set(RunCMake-stderr-file CMP0113-${val}-build-gnu-stderr.txt)
+    set(ENV{LANG} "C")
+    set(ENV{LC_MESSAGES} "C")
+  endif()
+  run_cmake_command(CMP0113-${val}-build ${CMAKE_COMMAND} --build .)
+  set(ENV{LANG} "${_backup_lang}")
+  set(ENV{LC_MESSAGES} "${_backup_lc_messages}")
+endfunction()
+
+if(NOT RunCMake_GENERATOR STREQUAL "Watcom WMake")
+  run_CMP0113(WARN)
+  run_CMP0113(OLD)
+  run_CMP0113(NEW)
+endif()
diff --git a/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt b/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt
index fe13e813e5..b3f3a5ea7c 100644
--- a/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt
+++ b/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt
@@ -33,6 +33,7 @@
    \* CMP0105
    \* CMP0108
    \* CMP0112
+   \* CMP0113
 
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)