From c1e812ad4ff3f175dfc7c0de52b56dbaff9d3d16 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Feb 2022 16:53:15 -0500 Subject: target_link_libraries: Improve tolerance of unquoted generator expressions Prior to commit 1d709ea2f5 (cmGeneratorTarget: Propagate backtraces from INTERFACE_LINK_LIBRARIES, 2021-12-15, v3.23.0-rc1~228^2), the value of `INTERFACE_LINK_LIBRARIES` was a single string. If an unquoted generator expression passed to `target_link_libraries` contained `;` and became multiple arguments, they would all accumulate as a single `;`-separated list in the value of `INTERFACE_LINK_LIBRARIES`. Since that commit, each argument to `target_link_libraries` is placed in `INTERFACE_LINK_LIBRARIES` as a separate entry, as has long been the case for `LINK_LIBRARIES`. That behavior change broke projects that were accidentally relying on accumulation in `INTERFACE_LINK_LIBRARIES` to produce complete generator expressions containing `;`. Teach `target_link_libraries` to accumulate consecutive non-keyword arguments into a single entry before placing them in `LINK_LIBRARIES` or `INTERFACE_LINK_LIBRARIES`. For example, treat `a b c` as if they were written as `"a;b;c"`. This restores the previously accidental support for unquoted generator expressions in `INTERFACE_LINK_LIBRARIES`, and also enables it for `LINK_LIBRARIES`. For now, do not drop the `target_link_libraries` documentation that recommends quoting generator expressions. Quoting is still important to populate `LINK_LIBRARIES` in CMake 3.22 and below, and is also good practice to keep generator expressions whole. Fixes: #23203 --- Source/cmTargetLinkLibrariesCommand.cxx | 42 +++++++++++++++++++--- .../target_link_libraries/cmp0022/CMakeLists.txt | 10 ++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index 1cd8a7e..94fcdd9 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -2,11 +2,14 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmTargetLinkLibrariesCommand.h" +#include #include #include #include #include +#include + #include "cmExecutionStatus.h" #include "cmGeneratorExpression.h" #include "cmGlobalGenerator.h" @@ -178,6 +181,28 @@ bool cmTargetLinkLibrariesCommand(std::vector const& args, // specification if the keyword is encountered as the first argument. ProcessingState currentProcessingState = ProcessingLinkLibraries; + // Accumulate consectuive non-keyword arguments into one entry in + // order to handle unquoted generator expressions containing ';'. + cm::optional currentEntry; + auto processCurrentEntry = [&]() -> bool { + if (currentEntry) { + assert(!haveLLT); + if (!tll.HandleLibrary(currentProcessingState, *currentEntry, + GENERAL_LibraryType)) { + return false; + } + currentEntry = cm::nullopt; + } + return true; + }; + auto extendCurrentEntry = [¤tEntry](std::string const& arg) { + if (currentEntry) { + currentEntry = cmStrCat(*currentEntry, ';', arg); + } else { + currentEntry = arg; + } + }; + // Keep this list in sync with the keyword dispatch below. static std::unordered_set const keywords{ "LINK_INTERFACE_LIBRARIES", @@ -195,6 +220,11 @@ bool cmTargetLinkLibrariesCommand(std::vector const& args, // of debug and optimized that can be used. for (unsigned int i = 1; i < args.size(); ++i) { if (keywords.count(args[i])) { + // A keyword argument terminates any preceding accumulated entry. + if (!processCurrentEntry()) { + return false; + } + // Process this keyword argument. if (args[i] == "LINK_INTERFACE_LIBRARIES") { currentProcessingState = ProcessingPlainLinkInterface; @@ -285,18 +315,22 @@ bool cmTargetLinkLibrariesCommand(std::vector const& args, } else if (haveLLT) { // The link type was specified by the previous argument. haveLLT = false; + assert(!currentEntry); if (!tll.HandleLibrary(currentProcessingState, args[i], llt)) { return false; } llt = GENERAL_LibraryType; } else { - if (!tll.HandleLibrary(currentProcessingState, args[i], - GENERAL_LibraryType)) { - return false; - } + // Accumulate this argument in the current entry. + extendCurrentEntry(args[i]); } } + // Process the last accumulated entry, if any. + if (!processCurrentEntry()) { + return false; + } + // Make sure the last argument was not a library type specifier. if (haveLLT) { mf.IssueMessage(MessageType::FATAL_ERROR, diff --git a/Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt b/Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt index 83103cf..ca6309b 100644 --- a/Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt +++ b/Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt @@ -1,3 +1,4 @@ +cmake_policy(SET CMP0028 NEW) include(GenerateExportHeader) set(CMAKE_INCLUDE_CURRENT_DIR ON) @@ -17,6 +18,15 @@ assert_property(cmp0022ifacelib INTERFACE_LINK_LIBRARIES "") add_executable(cmp0022exe cmp0022exe.cpp) target_link_libraries(cmp0022exe cmp0022lib) +# Test adding unquoted genex with ';' to LINK_LIBRARIES and INTERFACE_LINK_LIBRARIES. +target_link_libraries(cmp0022lib + PUBLIC $<0:imp::missing1;imp::missing2> + PRIVATE $<0:imp::missing3;imp::missing4> + INTERFACE $<0:imp::missing5;imp::missing6> + ) +assert_property(cmp0022lib INTERFACE_LINK_LIBRARIES "cmp0022ifacelib;$<0:imp::missing1;imp::missing2>;$<0:imp::missing5;imp::missing6>") +assert_property(cmp0022lib LINK_LIBRARIES "cmp0022ifacelib;$<0:imp::missing1;imp::missing2>;$<0:imp::missing3;imp::missing4>") + add_library(staticlib1 STATIC staticlib1.cpp) generate_export_header(staticlib1) add_library(staticlib2 STATIC staticlib2.cpp) -- cgit v0.12