From 45fedf0e176d354b8cb4d3eed4a1ef9bf3943094 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 4 Sep 2020 09:51:15 -0400 Subject: 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 | 1 + Help/policy/CMP0113.rst | 43 ++++++++++++++++++++++ Help/release/dev/custom-command-dedup.rst | 5 +++ Source/cmLocalUnixMakefileGenerator3.cxx | 10 +++++ Source/cmLocalUnixMakefileGenerator3.h | 10 +++++ Source/cmMakefileTargetGenerator.cxx | 35 ++++++++++++++++++ Source/cmMakefileTargetGenerator.h | 2 + Source/cmPolicies.h | 7 +++- Tests/RunCMake/Make/CMP0113-Common.cmake | 17 +++++++++ .../RunCMake/Make/CMP0113-NEW-build-gnu-stderr.txt | 5 +++ Tests/RunCMake/Make/CMP0113-NEW-build-result.txt | 1 + Tests/RunCMake/Make/CMP0113-NEW-build-stderr.txt | 1 + Tests/RunCMake/Make/CMP0113-NEW-build-stdout.txt | 1 + Tests/RunCMake/Make/CMP0113-NEW.cmake | 2 + Tests/RunCMake/Make/CMP0113-OLD-build-stdout.txt | 1 + Tests/RunCMake/Make/CMP0113-OLD.cmake | 2 + Tests/RunCMake/Make/CMP0113-WARN-build-stdout.txt | 1 + Tests/RunCMake/Make/CMP0113-WARN.cmake | 2 + Tests/RunCMake/Make/RunCMakeTest.cmake | 22 +++++++++++ .../RunCMake/TargetPolicies/PolicyList-stderr.txt | 1 + 20 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 Help/policy/CMP0113.rst create mode 100644 Help/release/dev/custom-command-dedup.rst create mode 100644 Tests/RunCMake/Make/CMP0113-Common.cmake create mode 100644 Tests/RunCMake/Make/CMP0113-NEW-build-gnu-stderr.txt create mode 100644 Tests/RunCMake/Make/CMP0113-NEW-build-result.txt create mode 100644 Tests/RunCMake/Make/CMP0113-NEW-build-stderr.txt create mode 100644 Tests/RunCMake/Make/CMP0113-NEW-build-stdout.txt create mode 100644 Tests/RunCMake/Make/CMP0113-NEW.cmake create mode 100644 Tests/RunCMake/Make/CMP0113-OLD-build-stdout.txt create mode 100644 Tests/RunCMake/Make/CMP0113-OLD.cmake create mode 100644 Tests/RunCMake/Make/CMP0113-WARN-build-stdout.txt create mode 100644 Tests/RunCMake/Make/CMP0113-WARN.cmake diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index cd1d4d3..3821dc3 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. CMP0112: Target file component generator expressions do not add target dependencies. CMP0111: An imported target with a missing location fails during generation. CMP0110: add_test() supports arbitrary characters in test names. diff --git a/Help/policy/CMP0113.rst b/Help/policy/CMP0113.rst new file mode 100644 index 0000000..6f56902 --- /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 0000000..65fa303 --- /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 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 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& GetCommandsVisited( + cmGeneratorTarget const* target) + { + return this->CommandsVisited[target]; + }; + + std::map> + CommandsVisited; }; diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx index bc0725e..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(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 no_depends; + std::vector 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 b7ec442..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; 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 diff --git a/Tests/RunCMake/Make/CMP0113-Common.cmake b/Tests/RunCMake/Make/CMP0113-Common.cmake new file mode 100644 index 0000000..8ce26c4 --- /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 0000000..0af354a --- /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 0000000..d197c91 --- /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 0000000..8d98f9d --- /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 0000000..a6b851e --- /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 0000000..a2d1db5 --- /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 0000000..f6e1f0f --- /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 0000000..b6ab120 --- /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 0000000..f6e1f0f --- /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 0000000..48e035a --- /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 32aafca..5562aca 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 fe13e81..b3f3a5e 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\) -- cgit v0.12