From 13c7bb5b0cb08713b178169fe0d571a6e4689f39 Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Tue, 8 Apr 2025 09:32:46 -0400 Subject: cmGeneratorExpression: Update strip function to collect parsed expressions --- Source/cmGeneratorExpression.cxx | 42 ++++++++++++++++++++++++++++++++-------- Source/cmGeneratorExpression.h | 4 ++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/Source/cmGeneratorExpression.cxx b/Source/cmGeneratorExpression.cxx index 873ed7e..f0dcde7 100644 --- a/Source/cmGeneratorExpression.cxx +++ b/Source/cmGeneratorExpression.cxx @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -156,27 +157,40 @@ std::string cmGeneratorExpression::StripEmptyListElements( return result; } -static std::string stripAllGeneratorExpressions(std::string const& input) +static std::string extractAllGeneratorExpressions( + std::string const& input, + std::map>* collected) { std::string result; std::string::size_type pos = 0; std::string::size_type lastPos = pos; - int nestingLevel = 0; + // stack of { Generator Expression Name, Start Position of Value } + std::stack> genexps; while ((pos = input.find("$<", lastPos)) != std::string::npos) { result += input.substr(lastPos, pos - lastPos); pos += 2; - nestingLevel = 1; char const* c = input.c_str() + pos; + char const* cName = c; char const* const cStart = c; for (; *c; ++c) { if (cmGeneratorExpression::StartsWithGeneratorExpression(c)) { - ++nestingLevel; ++c; + cName = c + 1; continue; } - if (c[0] == '>') { - --nestingLevel; - if (nestingLevel == 0) { + if (c[0] == ':' && cName) { + genexps.push({ input.substr(pos + (cName - cStart), c - cName), + pos + (c + 1 - cStart) }); + cName = nullptr; + } else if (c[0] == '>') { + if (!cName && !genexps.empty()) { + if (collected) { + (*collected)[genexps.top().first].push_back(input.substr( + genexps.top().second, pos + c - cStart - genexps.top().second)); + } + genexps.pop(); + } + if (genexps.empty()) { break; } } @@ -188,12 +202,17 @@ static std::string stripAllGeneratorExpressions(std::string const& input) pos += traversed; lastPos = pos; } - if (nestingLevel == 0) { + if (genexps.empty()) { result += input.substr(lastPos); } return cmGeneratorExpression::StripEmptyListElements(result); } +static std::string stripAllGeneratorExpressions(std::string const& input) +{ + return extractAllGeneratorExpressions(input, nullptr); +} + static void prefixItems(std::string const& content, std::string& result, cm::string_view const& prefix) { @@ -375,6 +394,13 @@ std::string cmGeneratorExpression::Preprocess(std::string const& input, return std::string(); } +std::string cmGeneratorExpression::Collect( + std::string const& input, + std::map>& collected) +{ + return extractAllGeneratorExpressions(input, &collected); +} + cm::string_view::size_type cmGeneratorExpression::Find( cm::string_view const& input) { diff --git a/Source/cmGeneratorExpression.h b/Source/cmGeneratorExpression.h index 293459c..3ffc39d 100644 --- a/Source/cmGeneratorExpression.h +++ b/Source/cmGeneratorExpression.h @@ -63,6 +63,10 @@ public: PreprocessContext context, cm::string_view importPrefix = {}); + static std::string Collect( + std::string const& input, + std::map>& collected); + static void Split(std::string const& input, std::vector& output); -- cgit v0.12 From ebe487ea81003b934bc80b7fc4dbceb37cb44ef0 Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Fri, 11 Apr 2025 10:54:28 -0400 Subject: export: Allow export with LINK_ONLY library dependencies --- Source/cmExportPackageInfoGenerator.cxx | 85 ++++++++++++++++------ .../LinkInterfaceGeneratorExpression-result.txt | 1 + .../LinkInterfaceGeneratorExpression-stderr.txt | 6 ++ .../LinkInterfaceGeneratorExpression.cmake | 11 +++ .../ExportPackageInfo/LinkOnly-check.cmake | 12 +++ Tests/RunCMake/ExportPackageInfo/LinkOnly.cmake | 13 ++++ .../ExportPackageInfo/LinkOnlyRecursive-result.txt | 1 + .../ExportPackageInfo/LinkOnlyRecursive-stderr.txt | 6 ++ .../ExportPackageInfo/LinkOnlyRecursive.cmake | 11 +++ .../RunCMake/ExportPackageInfo/RunCMakeTest.cmake | 3 + 10 files changed, 126 insertions(+), 23 deletions(-) create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-result.txt create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-stderr.txt create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression.cmake create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkOnly-check.cmake create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkOnly.cmake create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-result.txt create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-stderr.txt create mode 100644 Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive.cmake diff --git a/Source/cmExportPackageInfoGenerator.cxx b/Source/cmExportPackageInfoGenerator.cxx index b5e0428..d486fec 100644 --- a/Source/cmExportPackageInfoGenerator.cxx +++ b/Source/cmExportPackageInfoGenerator.cxx @@ -2,6 +2,7 @@ file LICENSE.rst or https://cmake.org/licensing for details. */ #include "cmExportPackageInfoGenerator.h" +#include #include #include #include @@ -210,13 +211,15 @@ bool cmExportPackageInfoGenerator::GenerateInterfaceProperties( } namespace { -bool forbidGeneratorExpressions(std::string const& propertyName, - std::string const& propertyValue, - cmGeneratorTarget const* target) +bool ForbidGeneratorExpressions( + cmGeneratorTarget const* target, std::string const& propertyName, + std::string const& propertyValue, std::string& evaluatedValue, + std::map>& allowList) { - std::string const& evaluatedValue = cmGeneratorExpression::Preprocess( - propertyValue, cmGeneratorExpression::StripAllGeneratorExpressions); - if (evaluatedValue != propertyValue) { + size_t const allowedExpressions = allowList.size(); + evaluatedValue = cmGeneratorExpression::Collect(propertyValue, allowList); + if (evaluatedValue != propertyValue && + allowList.size() > allowedExpressions) { target->Makefile->IssueMessage( MessageType::FATAL_ERROR, cmStrCat("Property \"", propertyName, "\" of target \"", @@ -224,8 +227,32 @@ bool forbidGeneratorExpressions(std::string const& propertyName, "\" contains a generator expression. This is not allowed.")); return false; } + // Forbid Nested Generator Expressions + for (auto const& genexp : allowList) { + for (auto const& value : genexp.second) { + if (value.find("$<") != std::string::npos) { + target->Makefile->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat( + "$<", genexp.first, ":...> expression in \"", propertyName, + "\" of target \"", target->GetName(), + "\" contains a generator expression. This is not allowed.")); + return false; + } + } + } return true; } + +bool ForbidGeneratorExpressions(cmGeneratorTarget const* target, + std::string const& propertyName, + std::string const& propertyValue) +{ + std::map> allowList; + std::string evaluatedValue; + return ForbidGeneratorExpressions(target, propertyName, propertyValue, + evaluatedValue, allowList); +} } bool cmExportPackageInfoGenerator::NoteLinkedTarget( @@ -317,31 +344,43 @@ void cmExportPackageInfoGenerator::GenerateInterfaceLinkProperties( return; } - // TODO: Support $. - if (!forbidGeneratorExpressions(iter->first, iter->second, target)) { + // Extract any $ from the link libraries, and assert that no + // other generator expressions are present. + std::map> allowList = { { "LINK_ONLY", + {} } }; + std::string interfaceLinkLibraries; + if (!ForbidGeneratorExpressions(target, iter->first, iter->second, + interfaceLinkLibraries, allowList)) { result = false; return; } - std::vector buildRequires; - // std::vector linkRequires; TODO std::vector linkLibraries; + std::vector linkRequires; + std::vector buildRequires; - for (auto const& name : cmList{ iter->second }) { - auto const& ti = this->LinkTargets.find(name); - if (ti != this->LinkTargets.end()) { - if (ti->second.empty()) { - result = false; + auto addLibraries = [this, &linkLibraries, + &result](std::vector const& names, + std::vector& output) -> void { + for (auto const& name : names) { + auto const& ti = this->LinkTargets.find(name); + if (ti != this->LinkTargets.end()) { + if (ti->second.empty()) { + result = false; + } else { + output.emplace_back(ti->second); + } } else { - buildRequires.emplace_back(ti->second); + linkLibraries.emplace_back(name); } - } else { - linkLibraries.emplace_back(name); } - } + }; + + addLibraries(allowList["LINK_ONLY"], linkRequires); + addLibraries(cmList{ interfaceLinkLibraries }, buildRequires); buildArray(component, "requires", buildRequires); - // buildArray(component, "link_requires", linkRequires); TODO + buildArray(component, "link_requires", linkRequires); buildArray(component, "link_libraries", linkLibraries); } @@ -354,7 +393,7 @@ void cmExportPackageInfoGenerator::GenerateInterfaceCompileFeatures( return; } - if (!forbidGeneratorExpressions(iter->first, iter->second, target)) { + if (!ForbidGeneratorExpressions(target, iter->first, iter->second)) { result = false; return; } @@ -383,7 +422,7 @@ void cmExportPackageInfoGenerator::GenerateInterfaceCompileDefines( } // TODO: Support language-specific defines. - if (!forbidGeneratorExpressions(iter->first, iter->second, target)) { + if (!ForbidGeneratorExpressions(target, iter->first, iter->second)) { result = false; return; } @@ -414,7 +453,7 @@ void cmExportPackageInfoGenerator::GenerateInterfaceListProperty( return; } - if (!forbidGeneratorExpressions(prop, iter->second, target)) { + if (!ForbidGeneratorExpressions(target, prop, iter->second)) { result = false; return; } diff --git a/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-result.txt b/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-stderr.txt b/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-stderr.txt new file mode 100644 index 0000000..89e5adc --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression-stderr.txt @@ -0,0 +1,6 @@ +CMake Error in CMakeLists.txt: + Property "INTERFACE_LINK_LIBRARIES" of target "bar" contains a generator + expression. This is not allowed. + + +CMake Generate step failed. Build files cannot be regenerated correctly. diff --git a/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression.cmake b/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression.cmake new file mode 100644 index 0000000..654f15b --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkInterfaceGeneratorExpression.cmake @@ -0,0 +1,11 @@ +project(LinkInterfaceGeneratorExpression CXX) + +add_library(foo foo.cxx) +add_library(bar foo.cxx) +target_link_libraries(bar $<1:foo>) + +install(TARGETS foo EXPORT foo) +export(EXPORT foo PACKAGE_INFO foo) + +install(TARGETS bar EXPORT bar) +export(EXPORT bar PACKAGE_INFO bar) diff --git a/Tests/RunCMake/ExportPackageInfo/LinkOnly-check.cmake b/Tests/RunCMake/ExportPackageInfo/LinkOnly-check.cmake new file mode 100644 index 0000000..e541399 --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkOnly-check.cmake @@ -0,0 +1,12 @@ +include(${CMAKE_CURRENT_LIST_DIR}/Assertions.cmake) + +set(out_dir "${RunCMake_BINARY_DIR}/LinkOnly-build") + +file(READ "${out_dir}/bar.cps" content) +string(JSON component GET "${content}" "components" "bar") +expect_array("${component}" 2 "link_requires") +expect_value("${component}" "foo:linkOnlyOne" "link_requires" 0) +expect_value("${component}" "foo:linkOnlyTwo" "link_requires" 1) +expect_array("${component}" 1 "requires") +expect_value("${component}" "foo:foo" "requires" 0) +expect_missing("${component}" "foo:foo" "link_libraries") diff --git a/Tests/RunCMake/ExportPackageInfo/LinkOnly.cmake b/Tests/RunCMake/ExportPackageInfo/LinkOnly.cmake new file mode 100644 index 0000000..884c42e --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkOnly.cmake @@ -0,0 +1,13 @@ +project(LinkOnly CXX) + +add_library(linkOnlyOne foo.cxx) +add_library(linkOnlyTwo foo.cxx) +add_library(foo foo.cxx) +add_library(bar foo.cxx) +target_link_libraries(bar $ $ foo) + +install(TARGETS foo linkOnlyOne linkOnlyTwo EXPORT foo) +export(EXPORT foo PACKAGE_INFO foo) + +install(TARGETS bar EXPORT bar) +export(EXPORT bar PACKAGE_INFO bar) diff --git a/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-result.txt b/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-stderr.txt b/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-stderr.txt new file mode 100644 index 0000000..ccdd7ee --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive-stderr.txt @@ -0,0 +1,6 @@ +CMake Error in CMakeLists.txt: + \$ expression in "INTERFACE_LINK_LIBRARIES" of target "bar" + contains a generator expression. This is not allowed. + + +CMake Generate step failed. Build files cannot be regenerated correctly. diff --git a/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive.cmake b/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive.cmake new file mode 100644 index 0000000..014b945 --- /dev/null +++ b/Tests/RunCMake/ExportPackageInfo/LinkOnlyRecursive.cmake @@ -0,0 +1,11 @@ +project(LinkOnly CXX) + +add_library(foo foo.cxx) +add_library(bar foo.cxx) +target_link_libraries(bar $>) + +install(TARGETS foo EXPORT foo) +export(EXPORT foo PACKAGE_INFO foo) + +install(TARGETS bar EXPORT bar) +export(EXPORT bar PACKAGE_INFO bar) diff --git a/Tests/RunCMake/ExportPackageInfo/RunCMakeTest.cmake b/Tests/RunCMake/ExportPackageInfo/RunCMakeTest.cmake index b80ce53..939f27f 100644 --- a/Tests/RunCMake/ExportPackageInfo/RunCMakeTest.cmake +++ b/Tests/RunCMake/ExportPackageInfo/RunCMakeTest.cmake @@ -24,6 +24,8 @@ run_cmake(ReferencesWronglyImportedTarget) run_cmake(ReferencesWronglyNamespacedTarget) run_cmake(DependsMultipleDifferentNamespace) run_cmake(DependsMultipleDifferentSets) +run_cmake(LinkInterfaceGeneratorExpression) +run_cmake(LinkOnlyRecursive) # Test functionality run_cmake(Appendix) @@ -35,3 +37,4 @@ run_cmake(LowerCaseFile) run_cmake(Requirements) run_cmake(TargetTypes) run_cmake(DependsMultiple) +run_cmake(LinkOnly) -- cgit v0.12