From afb998704e67d3d3ce5b24c112cb06e770fca78d Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 14 Jul 2020 13:55:38 -0400 Subject: [PATCH] Remove filtering of allowed INTERFACE library properties Previously we disallowed use of arbitrary properties on INTERFACE libraries. The goal was to future-proof projects using them by not allowing properties to be set that may affect their future inclusion in the generated buildsystem. In order to prepare to actually include INTERFACE libraries in the generated buildsystem, drop the filter and allow arbitrary properties to be set. Issue: #19145 --- Help/manual/cmake-buildsystem.7.rst | 13 ---- Source/cmGeneratorTarget.cxx | 5 -- Source/cmGetPropertyCommand.cxx | 10 +-- Source/cmGetTargetPropertyCommand.cxx | 10 +-- Source/cmTarget.cxx | 10 --- Source/cmTargetPropertyComputer.cxx | 69 ------------------- Source/cmTargetPropertyComputer.h | 6 -- .../InterfaceLibrary/RunCMakeTest.cmake | 1 - .../InterfaceLibrary/whitelist-result.txt | 1 - .../InterfaceLibrary/whitelist-stderr.txt | 19 ----- .../RunCMake/InterfaceLibrary/whitelist.cmake | 25 ------- 11 files changed, 6 insertions(+), 163 deletions(-) delete mode 100644 Tests/RunCMake/InterfaceLibrary/whitelist-result.txt delete mode 100644 Tests/RunCMake/InterfaceLibrary/whitelist-stderr.txt delete mode 100644 Tests/RunCMake/InterfaceLibrary/whitelist.cmake diff --git a/Help/manual/cmake-buildsystem.7.rst b/Help/manual/cmake-buildsystem.7.rst index d8142a263c..6eea191638 100644 --- a/Help/manual/cmake-buildsystem.7.rst +++ b/Help/manual/cmake-buildsystem.7.rst @@ -975,19 +975,6 @@ This way, the build specification of ``exe1`` is expressed entirely as linked targets, and the complexity of compiler-specific flags is encapsulated in an ``INTERFACE`` library target. -The properties permitted to be set on or read from an ``INTERFACE`` library -are: - -* Properties matching ``INTERFACE_*`` -* Built-in properties matching ``COMPATIBLE_INTERFACE_*`` -* ``EXPORT_NAME`` -* ``EXPORT_PROPERTIES`` -* ``IMPORTED`` -* ``MANUALLY_ADDED_DEPENDENCIES`` -* ``NAME`` -* Properties matching ``IMPORTED_LIBNAME_*`` -* Properties matching ``MAP_IMPORTED_CONFIG_*`` - ``INTERFACE`` libraries may be installed and exported. Any content they refer to must be installed separately: diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 41cff01368..d39c4937bf 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -380,11 +380,6 @@ std::string cmGeneratorTarget::GetExportName() const cmProp cmGeneratorTarget::GetProperty(const std::string& prop) const { - if (!cmTargetPropertyComputer::PassesWhitelist( - this->GetType(), prop, this->Makefile->GetMessenger(), - this->GetBacktrace())) { - return nullptr; - } if (cmProp result = cmTargetPropertyComputer::GetProperty( this, prop, this->Makefile->GetMessenger(), this->GetBacktrace())) { return result; diff --git a/Source/cmGetPropertyCommand.cxx b/Source/cmGetPropertyCommand.cxx index cba770479c..cdfd8c883a 100644 --- a/Source/cmGetPropertyCommand.cxx +++ b/Source/cmGetPropertyCommand.cxx @@ -17,7 +17,6 @@ #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmTarget.h" -#include "cmTargetPropertyComputer.h" #include "cmTest.h" #include "cmake.h" @@ -364,12 +363,9 @@ bool HandleTargetMode(cmExecutionStatus& status, const std::string& name, cmProp prop_cstr = nullptr; cmListFileBacktrace bt = status.GetMakefile().GetBacktrace(); cmMessenger* messenger = status.GetMakefile().GetMessenger(); - if (cmTargetPropertyComputer::PassesWhitelist( - target->GetType(), propertyName, messenger, bt)) { - prop_cstr = target->GetComputedProperty(propertyName, messenger, bt); - if (!prop_cstr) { - prop_cstr = target->GetProperty(propertyName); - } + prop_cstr = target->GetComputedProperty(propertyName, messenger, bt); + if (!prop_cstr) { + prop_cstr = target->GetProperty(propertyName); } return StoreResult(infoType, status.GetMakefile(), variable, prop_cstr ? prop_cstr->c_str() : nullptr); diff --git a/Source/cmGetTargetPropertyCommand.cxx b/Source/cmGetTargetPropertyCommand.cxx index 8a304be0de..78a17d2c3e 100644 --- a/Source/cmGetTargetPropertyCommand.cxx +++ b/Source/cmGetTargetPropertyCommand.cxx @@ -12,7 +12,6 @@ #include "cmPolicies.h" #include "cmProperty.h" #include "cmTarget.h" -#include "cmTargetPropertyComputer.h" class cmMessenger; @@ -46,12 +45,9 @@ bool cmGetTargetPropertyCommand(std::vector const& args, cmProp prop_cstr = nullptr; cmListFileBacktrace bt = mf.GetBacktrace(); cmMessenger* messenger = mf.GetMessenger(); - if (cmTargetPropertyComputer::PassesWhitelist(tgt->GetType(), args[2], - messenger, bt)) { - prop_cstr = tgt->GetComputedProperty(args[2], messenger, bt); - if (!prop_cstr) { - prop_cstr = tgt->GetProperty(args[2]); - } + prop_cstr = tgt->GetComputedProperty(args[2], messenger, bt); + if (!prop_cstr) { + prop_cstr = tgt->GetProperty(args[2]); } if (prop_cstr) { prop = *prop_cstr; diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 0e5dfef1f1..67d7451f0e 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1144,11 +1144,6 @@ cmBacktraceRange cmTarget::GetLinkImplementationBacktraces() const void cmTarget::SetProperty(const std::string& prop, const char* value) { - if (!cmTargetPropertyComputer::PassesWhitelist( - this->GetType(), prop, impl->Makefile->GetMessenger(), - impl->Makefile->GetBacktrace())) { - return; - } #define MAKE_STATIC_PROP(PROP) static const std::string prop##PROP = #PROP MAKE_STATIC_PROP(C_STANDARD); MAKE_STATIC_PROP(CXX_STANDARD); @@ -1355,11 +1350,6 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) void cmTarget::AppendProperty(const std::string& prop, const std::string& value, bool asString) { - if (!cmTargetPropertyComputer::PassesWhitelist( - this->GetType(), prop, impl->Makefile->GetMessenger(), - impl->Makefile->GetBacktrace())) { - return; - } if (prop == "NAME") { impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, "NAME property is read-only\n"); diff --git a/Source/cmTargetPropertyComputer.cxx b/Source/cmTargetPropertyComputer.cxx index f37995c23d..b9c93659fb 100644 --- a/Source/cmTargetPropertyComputer.cxx +++ b/Source/cmTargetPropertyComputer.cxx @@ -3,15 +3,12 @@ #include "cmTargetPropertyComputer.h" -#include #include -#include #include "cmMessageType.h" #include "cmMessenger.h" #include "cmPolicies.h" #include "cmStateSnapshot.h" -#include "cmStringAlgorithms.h" bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( std::string const& tgtName, cmMessenger* messenger, @@ -44,69 +41,3 @@ bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( return messageType != MessageType::FATAL_ERROR; } - -bool cmTargetPropertyComputer::WhiteListedInterfaceProperty( - const std::string& prop) -{ - if (cmHasLiteralPrefix(prop, "INTERFACE_")) { - return true; - } - if (cmHasLiteralPrefix(prop, "_")) { - return true; - } - if (std::islower(prop[0])) { - return true; - } - static std::unordered_set const builtIns{ - "COMPATIBLE_INTERFACE_BOOL", - "COMPATIBLE_INTERFACE_NUMBER_MAX", - "COMPATIBLE_INTERFACE_NUMBER_MIN", - "COMPATIBLE_INTERFACE_STRING", - "DEPRECATION", - "EXPORT_NAME", - "EXPORT_PROPERTIES", - "IMPORTED", - "IMPORTED_GLOBAL", - "MANUALLY_ADDED_DEPENDENCIES", - "NAME", - "PRIVATE_HEADER", - "PUBLIC_HEADER", - "TYPE" - }; - - if (builtIns.count(prop)) { - return true; - } - - if (prop == "IMPORTED_CONFIGURATIONS" || prop == "IMPORTED_LIBNAME" || - cmHasLiteralPrefix(prop, "IMPORTED_LIBNAME_") || - cmHasLiteralPrefix(prop, "MAP_IMPORTED_CONFIG_")) { - return true; - } - - // This property should not be allowed but was incorrectly added in - // CMake 3.8. We can't remove it from the whitelist without breaking - // projects that try to set it. One day we could warn about this, but - // for now silently accept it. - if (prop == "NO_SYSTEM_FROM_IMPORTED") { - return true; - } - - return false; -} - -bool cmTargetPropertyComputer::PassesWhitelist( - cmStateEnums::TargetType tgtType, std::string const& prop, - cmMessenger* messenger, cmListFileBacktrace const& context) -{ - if (tgtType == cmStateEnums::INTERFACE_LIBRARY && - !WhiteListedInterfaceProperty(prop)) { - std::ostringstream e; - e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " - "The property \"" - << prop << "\" is not allowed."; - messenger->IssueMessage(MessageType::FATAL_ERROR, e.str(), context); - return false; - } - return true; -} diff --git a/Source/cmTargetPropertyComputer.h b/Source/cmTargetPropertyComputer.h index f87b7c2d28..bafa43be6b 100644 --- a/Source/cmTargetPropertyComputer.h +++ b/Source/cmTargetPropertyComputer.h @@ -35,12 +35,6 @@ public: return nullptr; } - static bool WhiteListedInterfaceProperty(const std::string& prop); - - static bool PassesWhitelist(cmStateEnums::TargetType tgtType, - std::string const& prop, cmMessenger* messenger, - cmListFileBacktrace const& context); - private: static bool HandleLocationPropertyPolicy(std::string const& tgtName, cmMessenger* messenger, diff --git a/Tests/RunCMake/InterfaceLibrary/RunCMakeTest.cmake b/Tests/RunCMake/InterfaceLibrary/RunCMakeTest.cmake index 5a6af1da68..b84bc33a07 100644 --- a/Tests/RunCMake/InterfaceLibrary/RunCMakeTest.cmake +++ b/Tests/RunCMake/InterfaceLibrary/RunCMakeTest.cmake @@ -3,7 +3,6 @@ include(RunCMake) run_cmake(invalid_name) run_cmake(target_commands) run_cmake(no_shared_libs) -run_cmake(whitelist) run_cmake(invalid_signature) run_cmake(global-interface) run_cmake(genex_link) diff --git a/Tests/RunCMake/InterfaceLibrary/whitelist-result.txt b/Tests/RunCMake/InterfaceLibrary/whitelist-result.txt deleted file mode 100644 index d00491fd7e..0000000000 --- a/Tests/RunCMake/InterfaceLibrary/whitelist-result.txt +++ /dev/null @@ -1 +0,0 @@ -1 diff --git a/Tests/RunCMake/InterfaceLibrary/whitelist-stderr.txt b/Tests/RunCMake/InterfaceLibrary/whitelist-stderr.txt deleted file mode 100644 index 577c0cc5d5..0000000000 --- a/Tests/RunCMake/InterfaceLibrary/whitelist-stderr.txt +++ /dev/null @@ -1,19 +0,0 @@ -CMake Error at whitelist.cmake:4 \(set_property\): - INTERFACE_LIBRARY targets may only have whitelisted properties. The - property "OUTPUT_NAME" is not allowed. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\) - - -CMake Error at whitelist.cmake:5 \(set_property\): - INTERFACE_LIBRARY targets may only have whitelisted properties. The - property "OUTPUT_NAME" is not allowed. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\) - - -CMake Error at whitelist.cmake:6 \(get_target_property\): - INTERFACE_LIBRARY targets may only have whitelisted properties. The - property "OUTPUT_NAME" is not allowed. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/InterfaceLibrary/whitelist.cmake b/Tests/RunCMake/InterfaceLibrary/whitelist.cmake deleted file mode 100644 index 0db6375311..0000000000 --- a/Tests/RunCMake/InterfaceLibrary/whitelist.cmake +++ /dev/null @@ -1,25 +0,0 @@ - -add_library(iface INTERFACE) - -set_property(TARGET iface PROPERTY OUTPUT_NAME output) -set_property(TARGET iface APPEND PROPERTY OUTPUT_NAME append) -get_target_property(outname iface OUTPUT_NAME) - -# Properties starting with `_` are allowed. -set_property(TARGET iface PROPERTY "_custom_property" output) -set_property(TARGET iface APPEND PROPERTY "_custom_property" append) -get_target_property(outname iface "_custom_property") - -# Properties starting with a lowercase letter are allowed. -set_property(TARGET iface PROPERTY "custom_property" output) -set_property(TARGET iface APPEND PROPERTY "custom_property" append) -get_target_property(outname iface "custom_property") - -# PUBLIC_HEADER / PRIVATE_HEADER properties are allowed -set_property(TARGET iface PROPERTY PUBLIC_HEADER foo.h) -set_property(TARGET iface APPEND PROPERTY PUBLIC_HEADER bar.h) -get_target_property(outname iface PUBLIC_HEADER) - -set_property(TARGET iface PROPERTY PRIVATE_HEADER foo.h) -set_property(TARGET iface APPEND PROPERTY PRIVATE_HEADER bar.h) -get_target_property(outname iface PRIVATE_HEADER)