From 37af6c33116af75a9538861268edab0ddc202d79 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 20 Dec 2021 10:03:17 -0500 Subject: target_link_libraries: Optionally require only target names Optionally verify that items in `LINK_LIBRARIES` and `INTERFACE_LINK_LIBRARIES` that can be target names are actually target names. Add a `LINK_LIBRARIES_ONLY_TARGETS` target property and corresponding `CMAKE_LINK_LIBRARIES_ONLY_TARGETS` variable to enable this new check. Fixes: #22858 --- Help/manual/cmake-properties.7.rst | 1 + Help/manual/cmake-variables.7.rst | 1 + Help/policy/CMP0028.rst | 2 + Help/prop_tgt/LINK_LIBRARIES_ONLY_TARGETS.rst | 55 +++++++++++++++++++++ Help/release/dev/link-only-targets.rst | 7 +++ .../variable/CMAKE_LINK_LIBRARIES_ONLY_TARGETS.rst | 10 ++++ Source/cmGeneratorTarget.cxx | 49 +++++++++++++++++++ Source/cmGeneratorTarget.h | 1 + Source/cmTarget.cxx | 3 ++ Tests/RunCMake/LinkItemValidation/CMakeLists.txt | 3 ++ .../LinkItemValidation/OnlyTargets-result.txt | 1 + .../LinkItemValidation/OnlyTargets-stderr.txt | 40 ++++++++++++++++ .../RunCMake/LinkItemValidation/OnlyTargets.cmake | 56 ++++++++++++++++++++++ .../RunCMake/LinkItemValidation/RunCMakeTest.cmake | 2 + Tests/RunCMake/LinkItemValidation/main.c | 4 ++ 15 files changed, 235 insertions(+) create mode 100644 Help/prop_tgt/LINK_LIBRARIES_ONLY_TARGETS.rst create mode 100644 Help/release/dev/link-only-targets.rst create mode 100644 Help/variable/CMAKE_LINK_LIBRARIES_ONLY_TARGETS.rst create mode 100644 Tests/RunCMake/LinkItemValidation/OnlyTargets-result.txt create mode 100644 Tests/RunCMake/LinkItemValidation/OnlyTargets-stderr.txt create mode 100644 Tests/RunCMake/LinkItemValidation/OnlyTargets.cmake create mode 100644 Tests/RunCMake/LinkItemValidation/main.c diff --git a/Help/manual/cmake-properties.7.rst b/Help/manual/cmake-properties.7.rst index 5e18e10748..8db7161a58 100644 --- a/Help/manual/cmake-properties.7.rst +++ b/Help/manual/cmake-properties.7.rst @@ -304,6 +304,7 @@ Properties on Targets /prop_tgt/LINK_INTERFACE_MULTIPLICITY /prop_tgt/LINK_INTERFACE_MULTIPLICITY_CONFIG /prop_tgt/LINK_LIBRARIES + /prop_tgt/LINK_LIBRARIES_ONLY_TARGETS /prop_tgt/LINK_OPTIONS /prop_tgt/LINK_SEARCH_END_STATIC /prop_tgt/LINK_SEARCH_START_STATIC diff --git a/Help/manual/cmake-variables.7.rst b/Help/manual/cmake-variables.7.rst index 3c50117d32..253ebc4b70 100644 --- a/Help/manual/cmake-variables.7.rst +++ b/Help/manual/cmake-variables.7.rst @@ -220,6 +220,7 @@ Variables that Change Behavior /variable/CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT /variable/CMAKE_LIBRARY_PATH /variable/CMAKE_LINK_DIRECTORIES_BEFORE + /variable/CMAKE_LINK_LIBRARIES_ONLY_TARGETS /variable/CMAKE_MFC_FLAG /variable/CMAKE_MAXIMUM_RECURSION_DEPTH /variable/CMAKE_MESSAGE_CONTEXT diff --git a/Help/policy/CMP0028.rst b/Help/policy/CMP0028.rst index ab38229939..dcd39d8994 100644 --- a/Help/policy/CMP0028.rst +++ b/Help/policy/CMP0028.rst @@ -13,6 +13,8 @@ on disk. Previously, if a target was not found with a matching name, the name was considered to refer to a file on disk. This can lead to confusing error messages if there is a typo in what should be a target name. +See also the :prop_tgt:`LINK_LIBRARIES_ONLY_TARGETS` target property. + The ``OLD`` behavior for this policy is to search for targets, then files on disk, even if the search term contains double-colons. The ``NEW`` behavior for this policy is to issue a ``FATAL_ERROR`` if a link dependency contains diff --git a/Help/prop_tgt/LINK_LIBRARIES_ONLY_TARGETS.rst b/Help/prop_tgt/LINK_LIBRARIES_ONLY_TARGETS.rst new file mode 100644 index 0000000000..78fbb02d81 --- /dev/null +++ b/Help/prop_tgt/LINK_LIBRARIES_ONLY_TARGETS.rst @@ -0,0 +1,55 @@ +LINK_LIBRARIES_ONLY_TARGETS +--------------------------- + +.. versionadded:: 3.23 + +Enforce that link items that can be target names are actually existing targets. + +Set this property to a true value to enable additional checks on the contents +of the :prop_tgt:`LINK_LIBRARIES` and :prop_tgt:`INTERFACE_LINK_LIBRARIES` +target properties, typically populated by :command:`target_link_libraries`. +CMake will verify that link items that might be target names actually name +existing targets. An item is considered a possible target name if: + +* it does not contain a ``/`` or ``\``, and +* it does not start in ``-``, and +* (for historical reasons) it does not start in ``$`` or `````. + +This property is initialized by the value of the +:variable:`CMAKE_LINK_LIBRARIES_ONLY_TARGETS` variable when a non-imported +target is created. The property may be explicitly enabled on an imported +target to check its link interface. + +For example, the following code: + +.. code-block:: cmake + + set(CMAKE_LINK_LIBRARIES_ONLY_TARGETS ON) + add_executable(myLib STATIC myLib.c) + add_executable(myExe myExe.c) + target_link_libraries(myExe PRIVATE miLib) # typo for myLib + +will produce a CMake-time error that ``miLib`` is not a target. + +In order to link toolchain-provided libraries by name while still +enforcing ``LINK_LIBRARIES_ONLY_TARGETS``, use an +:ref:`imported ` +:ref:`Interface Library ` with the +:prop_tgt:`IMPORTED_LIBNAME` target property: + +.. code-block:: cmake + + add_library(toolchain::m INTERFACE IMPORTED) + set_property(TARGET toolchain::m PROPERTY IMPORTED_LIBNAME "m") + target_link_libraries(myExe PRIVATE toolchain::m) + +See also policy :policy:`CMP0028`. + +.. note:: + + If :prop_tgt:`INTERFACE_LINK_LIBRARIES` contains generator expressions, + its actual list of link items may depend on the type and properties of + the consuming target. In such cases CMake may not always detect names + of missing targets that only appear for specific consumers. + A future version of CMake with improved heuristics may start triggering + errors on projects accepted by previous versions of CMake. diff --git a/Help/release/dev/link-only-targets.rst b/Help/release/dev/link-only-targets.rst new file mode 100644 index 0000000000..7901a25228 --- /dev/null +++ b/Help/release/dev/link-only-targets.rst @@ -0,0 +1,7 @@ +link-only-targets +----------------- + +* The :variable:`CMAKE_LINK_LIBRARIES_ONLY_TARGETS` variable and + corresponding :prop_tgt:`LINK_LIBRARIES_ONLY_TARGETS` target + property were added to optionally require that all link items + that can be target names are actually names of existing targets. diff --git a/Help/variable/CMAKE_LINK_LIBRARIES_ONLY_TARGETS.rst b/Help/variable/CMAKE_LINK_LIBRARIES_ONLY_TARGETS.rst new file mode 100644 index 0000000000..513c3d0154 --- /dev/null +++ b/Help/variable/CMAKE_LINK_LIBRARIES_ONLY_TARGETS.rst @@ -0,0 +1,10 @@ +CMAKE_LINK_LIBRARIES_ONLY_TARGETS +--------------------------------- + +.. versionadded:: 3.23 + +Set this variable to initialize the :prop_tgt:`LINK_LIBRARIES_ONLY_TARGETS` +property of non-imported targets when they are created. Setting it to true +enables an additional check that all items named by +:command:`target_link_libraries` that can be target names are actually names +of existing targets. See the target property documentation for details. diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 0036e69560..b651104a65 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -6247,6 +6247,18 @@ cmComputeLinkInformation* cmGeneratorTarget::GetLinkInformation( void cmGeneratorTarget::CheckLinkLibraries() const { + bool linkLibrariesOnlyTargets = + this->GetPropertyAsBool("LINK_LIBRARIES_ONLY_TARGETS"); + + // Evaluate the link interface of this target if needed for extra checks. + if (linkLibrariesOnlyTargets) { + std::vector const& configs = + this->Makefile->GetGeneratorConfigs(cmMakefile::IncludeEmptyConfig); + for (std::string const& config : configs) { + this->GetLinkInterfaceLibraries(config, this, LinkInterfaceFor::Link); + } + } + // Check link the implementation for each generated configuration. for (auto const& hmp : this->LinkImplMap) { HeadToLinkImplementationMap const& hm = hmp.second; @@ -6260,6 +6272,10 @@ void cmGeneratorTarget::CheckLinkLibraries() const if (!this->VerifyLinkItemColons(LinkItemRole::Implementation, item)) { return; } + if (linkLibrariesOnlyTargets && + !this->VerifyLinkItemIsTarget(LinkItemRole::Implementation, item)) { + return; + } } } @@ -6276,6 +6292,10 @@ void cmGeneratorTarget::CheckLinkLibraries() const if (!this->VerifyLinkItemColons(LinkItemRole::Interface, item)) { return; } + if (linkLibrariesOnlyTargets && + !this->VerifyLinkItemIsTarget(LinkItemRole::Interface, item)) { + return; + } } } } @@ -6329,6 +6349,35 @@ bool cmGeneratorTarget::VerifyLinkItemColons(LinkItemRole role, return false; } +bool cmGeneratorTarget::VerifyLinkItemIsTarget(LinkItemRole role, + cmLinkItem const& item) const +{ + if (item.Target) { + return true; + } + std::string const& str = item.AsStr(); + if (!str.empty() && + (str[0] == '-' || str[0] == '$' || str[0] == '`' || + str.find_first_of("/\\") != std::string::npos)) { + return true; + } + + std::string e = cmStrCat("Target \"", this->GetName(), + "\" has LINK_LIBRARIES_ONLY_TARGETS enabled, but ", + role == LinkItemRole::Implementation + ? "it links to" + : "its link interface contains", + ":\n ", item.AsStr(), "\nwhich is not a target. ", + missingTargetPossibleReasons); + cmListFileBacktrace backtrace = item.Backtrace; + if (backtrace.Empty()) { + backtrace = this->GetBacktrace(); + } + this->LocalGenerator->GetCMakeInstance()->IssueMessage( + MessageType::FATAL_ERROR, e, backtrace); + return false; +} + void cmGeneratorTarget::GetTargetVersion(int& major, int& minor) const { int patch; diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 096e2eaac9..5f317c45ef 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -982,6 +982,7 @@ private: Implementation, Interface, }; + bool VerifyLinkItemIsTarget(LinkItemRole role, cmLinkItem const& item) const; bool VerifyLinkItemColons(LinkItemRole role, cmLinkItem const& item) const; // Cache import information from properties for each configuration. diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 38361cc0a3..c03a252357 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -471,6 +471,9 @@ cmTarget::cmTarget(std::string const& name, cmStateEnums::TargetType type, initProp(property); } } + if (!this->IsImported()) { + initProp("LINK_LIBRARIES_ONLY_TARGETS"); + } } // Save the backtrace of target construction. diff --git a/Tests/RunCMake/LinkItemValidation/CMakeLists.txt b/Tests/RunCMake/LinkItemValidation/CMakeLists.txt index 4f867df1c0..185cd91c10 100644 --- a/Tests/RunCMake/LinkItemValidation/CMakeLists.txt +++ b/Tests/RunCMake/LinkItemValidation/CMakeLists.txt @@ -1,3 +1,6 @@ cmake_minimum_required(VERSION 2.8.12) +if(NOT RunCMake_TEST MATCHES "^CMP0028") + cmake_minimum_required(VERSION 3.22) +endif() project(${RunCMake_TEST} CXX) include(${RunCMake_TEST}.cmake NO_POLICY_SCOPE) # policy used at end of dir diff --git a/Tests/RunCMake/LinkItemValidation/OnlyTargets-result.txt b/Tests/RunCMake/LinkItemValidation/OnlyTargets-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/LinkItemValidation/OnlyTargets-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/LinkItemValidation/OnlyTargets-stderr.txt b/Tests/RunCMake/LinkItemValidation/OnlyTargets-stderr.txt new file mode 100644 index 0000000000..bbb0170231 --- /dev/null +++ b/Tests/RunCMake/LinkItemValidation/OnlyTargets-stderr.txt @@ -0,0 +1,40 @@ +^CMake Error at OnlyTargets\.cmake:11 \(target_link_libraries\): + Target "exe" has LINK_LIBRARIES_ONLY_TARGETS enabled, but it links to: + + non_target_in_exe + + which is not a target\. Possible reasons include: +( + \*[^ +]+)* + +Call Stack \(most recent call first\): + CMakeLists\.txt:[0-9]+ \(include\) ++ +CMake Error at OnlyTargets\.cmake:21 \(target_link_libraries\): + Target "iface" has LINK_LIBRARIES_ONLY_TARGETS enabled, but its link + interface contains: + + non_target_in_iface + + which is not a target\. Possible reasons include: +( + \*[^ +]+)* + +Call Stack \(most recent call first\): + CMakeLists\.txt:[0-9]+ \(include\) ++ +CMake Error at OnlyTargets\.cmake:30 \(target_link_libraries\): + Target "iface_imported_checked" has LINK_LIBRARIES_ONLY_TARGETS enabled, + but its link interface contains: + + non_target_in_iface_imported_checked + + which is not a target\. Possible reasons include: +( + \*[^ +]+)* + +Call Stack \(most recent call first\): + CMakeLists\.txt:[0-9]+ \(include\) diff --git a/Tests/RunCMake/LinkItemValidation/OnlyTargets.cmake b/Tests/RunCMake/LinkItemValidation/OnlyTargets.cmake new file mode 100644 index 0000000000..941731841f --- /dev/null +++ b/Tests/RunCMake/LinkItemValidation/OnlyTargets.cmake @@ -0,0 +1,56 @@ +enable_language(C) + +set(CMAKE_LINK_LIBRARIES_ONLY_TARGETS 1) + +# Use imported interface library to name toolchain-provided libraries. +add_library(toolchain::m INTERFACE IMPORTED) +set_property(TARGET toolchain::m PROPERTY IMPORTED_LIBNAME "m") + +# Linking directly warns. +add_executable(exe main.c) +target_link_libraries(exe PRIVATE + -lflag_in_exe # accepted + /abs/path/in_exe # accepted + rel/path/in_exe # accepted + toolchain::m # accepted + non_target_in_exe # rejected + ) + +# Link interfaces warn. +add_library(iface INTERFACE) +target_link_libraries(iface INTERFACE + -lflag_in_iface # accepted + /abs/path/in_iface # accepted + rel/path/in_iface # accepted + non_target_in_iface # rejected + ) + +# Imported target link interfaces warn if explicitly enabled. +add_library(iface_imported_checked INTERFACE IMPORTED) +target_link_libraries(iface_imported_checked INTERFACE + -lflag_iface_imported_checked # accepted + /abs/path/in_iface_imported_checked # accepted + rel/path/in_iface_imported_checked # accepted + non_target_in_iface_imported_checked # rejected + ) +set_property(TARGET iface_imported_checked PROPERTY LINK_LIBRARIES_ONLY_TARGETS 1) + +# Linking directly does not warn if explicitly disabled. +add_executable(exe_not_checked main.c) +target_link_libraries(exe_not_checked PRIVATE + non_target_in_exe_not_checked + ) +set_property(TARGET exe_not_checked PROPERTY LINK_LIBRARIES_ONLY_TARGETS 0) + +# Link interfaces do not warn if explicitly disabled. +add_library(iface_not_checked INTERFACE) +target_link_libraries(iface_not_checked INTERFACE + non_target_in_iface_not_checked + ) +set_property(TARGET iface_not_checked PROPERTY LINK_LIBRARIES_ONLY_TARGETS 0) + +# Imported target link interfaces do not warn if not explicitly enabled. +add_library(iface_imported_default INTERFACE IMPORTED) +target_link_libraries(iface_imported_default INTERFACE + non_target_in_iface_imported_default + ) diff --git a/Tests/RunCMake/LinkItemValidation/RunCMakeTest.cmake b/Tests/RunCMake/LinkItemValidation/RunCMakeTest.cmake index 0c72ca2961..c423f6ac69 100644 --- a/Tests/RunCMake/LinkItemValidation/RunCMakeTest.cmake +++ b/Tests/RunCMake/LinkItemValidation/RunCMakeTest.cmake @@ -6,3 +6,5 @@ run_cmake(CMP0028-WARN) run_cmake(CMP0028-NEW-iface) run_cmake(CMP0028-OLD-iface) run_cmake(CMP0028-WARN-iface) + +run_cmake(OnlyTargets) diff --git a/Tests/RunCMake/LinkItemValidation/main.c b/Tests/RunCMake/LinkItemValidation/main.c new file mode 100644 index 0000000000..8488f4e58f --- /dev/null +++ b/Tests/RunCMake/LinkItemValidation/main.c @@ -0,0 +1,4 @@ +int main(void) +{ + return 0; +} -- cgit v1.2.1