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.

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 @@ std::string cmPackageInfoReader::ResolvePath(std::string path) const
   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 @@ void cmPackageInfoReader::SetTargetProperties(
 {
   // 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* cmPackageInfoReader::AddLibraryComponent(
   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"];

Source/cmPackageInfoReader.h

@@ -91,6 +91,9 @@ private:
                                 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;

Tests/FindPackageCpsTest/CMakeLists.txt

@@ -314,13 +314,21 @@ endif()
 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()

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": {

Tests/FindPackageCpsTest/cps/DefaultConfigurationsTest@Default.cps

@@ -0,0 +1,10 @@
+{
+  "cps_version": "0.13",
+  "name": "DefaultConfigurationsTest",
+  "configuration": "Default",
+  "components": {
+    "Target1": {
+      "includes": [ "@prefix@/include" ]
+    }
+  }
+}