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.

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>

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

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`.

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) {

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;
 };

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());

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;

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

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)

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[^
+]*

Tests/RunCMake/Make/CMP0113-NEW-build-result.txt

@@ -0,0 +1 @@
+[^0]

Tests/RunCMake/Make/CMP0113-NEW-build-stderr.txt

@@ -0,0 +1 @@
+.*

Tests/RunCMake/Make/CMP0113-NEW-build-stdout.txt

@@ -0,0 +1 @@
+Generating output-not-created.*Built target drive1

Tests/RunCMake/Make/CMP0113-NEW.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0113 NEW)
+include(CMP0113-Common.cmake)

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

Tests/RunCMake/Make/CMP0113-OLD.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0113 OLD)
+include(CMP0113-Common.cmake)

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

Tests/RunCMake/Make/CMP0113-WARN.cmake

@@ -0,0 +1,2 @@
+# Policy CMP0113 not set.
+include(CMP0113-Common.cmake)

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()

Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt

@@ -33,6 +33,7 @@
    \* CMP0105
    \* CMP0108
    \* CMP0112
+   \* CMP0113
 
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)