cmPackageInfoReader: Fix IMPORTED_CONFIGURATIONS Rework how we assign imported configurations to only add configurations that are actually imported. This requires a certain amount of cleverness to keep the order consistent with the package's specified default configurations, but doing this is important now that configuration selection (see policies CMP0199 and CMP0200) is more reliant on the IMPORTED_CONFIGURATIONS property being accurate, rather than focusing on whether configuration-specific properties are set.
diff --git a/Source/cmPackageInfoReader.cxx b/Source/cmPackageInfoReader.cxx index b684897..37d755e 100644 --- a/Source/cmPackageInfoReader.cxx +++ b/Source/cmPackageInfoReader.cxx
@@ -2,11 +2,13 @@ file LICENSE.rst or https://cmake.org/licensing for details. */ #include "cmPackageInfoReader.h" +#include <algorithm> #include <initializer_list> #include <limits> #include <unordered_map> #include <utility> +#include <cmext/algorithm> #include <cmext/string_view> #include <cm3p/json/reader.h> @@ -17,12 +19,14 @@ #include "cmsys/RegularExpression.hxx" #include "cmExecutionStatus.h" +#include "cmList.h" #include "cmListFileCache.h" #include "cmMakefile.h" #include "cmMessageType.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmTarget.h" +#include "cmValue.h" namespace { @@ -572,6 +576,49 @@ return path; } +void cmPackageInfoReader::AddTargetConfiguration( + cmTarget* target, cm::string_view configuration) const +{ + static std::string const icProp = "IMPORTED_CONFIGURATIONS"; + + std::string const& configUpper = cmSystemTools::UpperCase(configuration); + + // Get existing list of imported configurations. + cmList configs; + if (cmValue v = target->GetProperty(icProp)) { + configs.assign(cmSystemTools::UpperCase(*v)); + } else { + // If the existing list is empty, just add the new one and return. + target->SetProperty(icProp, configUpper); + return; + } + + if (cm::contains(configs, configUpper)) { + // If the configuration is already listed, we don't need to do anything. + return; + } + + // Add the new configuration. + configs.append(configUpper); + + // Rebuild the configuration list by extracting any configuration in the + // default configurations and reinserting it at the beginning of the list + // according to the order of the default configurations. + std::vector<std::string> newConfigs; + for (std::string const& c : this->DefaultConfigurations) { + auto ci = std::find(configs.begin(), configs.end(), c); + if (ci != configs.end()) { + newConfigs.emplace_back(std::move(*ci)); + configs.erase(ci); + } + } + for (std::string& c : configs) { + newConfigs.emplace_back(std::move(c)); + } + + target->SetProperty("IMPORTED_CONFIGURATIONS", cmJoin(newConfigs, ";"_s)); +} + void cmPackageInfoReader::SetImportProperty(cmTarget* target, cm::string_view property, cm::string_view configuration, @@ -607,9 +654,7 @@ { // Add configuration (if applicable). if (!configuration.empty()) { - target->AppendProperty("IMPORTED_CONFIGURATIONS", - cmSystemTools::UpperCase(configuration), - makefile->GetBacktrace()); + this->AddTargetConfiguration(target, configuration); } // Add compile and link features. @@ -694,12 +739,6 @@ cmTarget* const target = makefile->AddImportedTarget(name, type, false); target->SetOrigin(cmTarget::Origin::Cps); - // Set default configurations. - if (!this->DefaultConfigurations.empty()) { - target->SetProperty("IMPORTED_CONFIGURATIONS", - cmJoin(this->DefaultConfigurations, ";"_s)); - } - // Set target properties. this->SetTargetProperties(makefile, target, data, package, {}); auto const& cfgData = data["configurations"];
diff --git a/Source/cmPackageInfoReader.h b/Source/cmPackageInfoReader.h index 4597e83..440f571 100644 --- a/Source/cmPackageInfoReader.h +++ b/Source/cmPackageInfoReader.h
@@ -91,6 +91,9 @@ Json::Value const& data, std::string const& package) const; + void AddTargetConfiguration(cmTarget* target, + cm::string_view configuration) const; + void SetTargetProperties(cmMakefile* makefile, cmTarget* target, Json::Value const& data, std::string const& package, cm::string_view configuration) const;
diff --git a/Tests/FindPackageCpsTest/CMakeLists.txt b/Tests/FindPackageCpsTest/CMakeLists.txt index 43caab4..f810425 100644 --- a/Tests/FindPackageCpsTest/CMakeLists.txt +++ b/Tests/FindPackageCpsTest/CMakeLists.txt
@@ -314,13 +314,21 @@ find_package(DefaultConfigurationsTest) if(NOT DefaultConfigurationsTest_FOUND) message(SEND_ERROR "DefaultConfigurationsTest not found !") -elseif(NOT TARGET DefaultConfigurationsTest::Target) - message(SEND_ERROR "DefaultConfigurationsTest::Target missing !") +elseif(NOT TARGET DefaultConfigurationsTest::Target1) + message(SEND_ERROR "DefaultConfigurationsTest::Target1 missing !") +elseif(NOT TARGET DefaultConfigurationsTest::Target2) + message(SEND_ERROR "DefaultConfigurationsTest::Target2 missing !") else() - get_property(dct_configs - TARGET DefaultConfigurationsTest::Target PROPERTY IMPORTED_CONFIGURATIONS) - if(NOT "${dct_configs}" STREQUAL "DEFAULT;TEST") - message(SEND_ERROR "DefaultConfigurationsTest::Target has wrong configurations '${dct_configs}' !") + get_property(dct1_configs + TARGET DefaultConfigurationsTest::Target1 PROPERTY IMPORTED_CONFIGURATIONS) + if(NOT "${dct1_configs}" STREQUAL "DEFAULT;TEST") + message(SEND_ERROR "DefaultConfigurationsTest::Target1 has wrong configurations '${dct1_configs}' !") endif() - set(dct_configs) + get_property(dct2_configs + TARGET DefaultConfigurationsTest::Target2 PROPERTY IMPORTED_CONFIGURATIONS) + if(NOT "${dct2_configs}" STREQUAL "TEST") + message(SEND_ERROR "DefaultConfigurationsTest::Target2 has wrong configurations '${dct2_configs}' !") + endif() + set(dct1_configs) + set(dct2_configs) endif()
diff --git a/Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest.cps b/Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest.cps index 24e99ea..0eb39ac 100644 --- a/Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest.cps +++ b/Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest.cps
@@ -4,7 +4,15 @@ "cps_path": "@prefix@/cps", "configurations": [ "Default" ], "components": { - "Target": { + "Target1": { + "type": "interface", + "configurations": { + "Test": { + "includes": [ "@prefix@/include" ] + } + } + }, + "Target2": { "type": "interface", "configurations": { "Test": {
diff --git a/Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest@Default.cps b/Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest@Default.cps new file mode 100644 index 0000000..48e18e8 --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest@Default.cps
@@ -0,0 +1,10 @@ +{ + "cps_version": "0.13", + "name": "DefaultConfigurationsTest", + "configuration": "Default", + "components": { + "Target1": { + "includes": [ "@prefix@/include" ] + } + } +}