cmPackageInfoReader: Fix handling of "definitions"

Rewrite cmPackageInfoReader's parsing of the "definitions" attribute.
The old logic (having been originally adapted from proof-of-concept
parsing code circa 2023) was parsing the attribute according to its
specification as of CPS 0.11, but the representation was changed in
CPS 0.12. Add a test to verify that definitions are being imported
correctly. Remove unnecessary setting and resetting of CMAKE_PREFIX_PATH
in the test.

Source/cmPackageInfoReader.cxx

@@ -49,6 +49,25 @@ std::unordered_map<std::string, std::string> Languages = {
   // clang-format on
 };
 
+enum LanguageGlobOption
+{
+  DisallowGlob,
+  AllowGlob,
+};
+
+cm::string_view MapLanguage(cm::string_view lang,
+                            LanguageGlobOption glob = AllowGlob)
+{
+  if (glob == AllowGlob && lang == "*"_s) {
+    return "*"_s;
+  }
+  auto const li = Languages.find(cmSystemTools::LowerCase(lang));
+  if (li != Languages.end()) {
+    return li->second;
+  }
+  return {};
+}
+
 std::string GetRealPath(std::string const& path)
 {
   return cmSystemTools::GetRealPath(path);
@@ -244,6 +263,64 @@ void AddLinkFeature(cmMakefile* makefile, cmTarget* target,
   }
 }
 
+std::string BuildDefinition(std::string const& name, Json::Value const& value)
+{
+  if (!value.isNull() && value.isConvertibleTo(Json::stringValue)) {
+    return cmStrCat(name, '=', value.asString());
+  }
+  return name;
+}
+
+void AddDefinition(cmMakefile* makefile, cmTarget* target,
+                   cm::string_view configuration,
+                   std::string const& definition)
+{
+  AppendProperty(makefile, target, "COMPILE_DEFINITIONS"_s, configuration,
+                 definition);
+}
+
+using DefinitionLanguageMap = std::map<cm::string_view, Json::Value>;
+using DefinitionsMap = std::map<std::string, DefinitionLanguageMap>;
+
+void AddDefinitions(cmMakefile* makefile, cmTarget* target,
+                    cm::string_view configuration,
+                    DefinitionsMap const& definitions)
+{
+  for (auto const& di : definitions) {
+    auto const& g = di.second.find("*"_s);
+    if (g != di.second.end()) {
+      std::string const& def = BuildDefinition(di.first, g->second);
+      if (di.second.size() == 1) {
+        // Only the non-language-specific definition exists.
+        AddDefinition(makefile, target, configuration, def);
+        continue;
+      }
+
+      // Create a genex to apply this definition to all languages except
+      // those that override it.
+      std::vector<cm::string_view> excludedLanguages;
+      for (auto const& li : di.second) {
+        if (li.first != "*"_s) {
+          excludedLanguages.emplace_back(li.first);
+        }
+      }
+      AddDefinition(makefile, target, configuration,
+                    cmStrCat("$<$<NOT:$<COMPILE_LANGUAGE:"_s,
+                             cmJoin(excludedLanguages, ","_s), ">>:"_s, def,
+                             '>'));
+    }
+
+    // Add language-specific definitions.
+    for (auto const& li : di.second) {
+      if (li.first != "*"_s) {
+        AddDefinition(makefile, target, configuration,
+                      cmStrCat("$<$<COMPILE_LANGUAGE:"_s, li.first, ">:"_s,
+                               BuildDefinition(di.first, li.second), '>'));
+      }
+    }
+  }
+}
+
 } // namespace
 
 std::unique_ptr<cmPackageInfoReader> cmPackageInfoReader::Read(
@@ -386,10 +463,24 @@ void cmPackageInfoReader::SetTargetProperties(
   }
 
   // Add compile definitions.
-  for (std::string const& def : ReadList(data, "definitions")) {
-    AppendProperty(makefile, target, "COMPILE_DEFINITIONS"_s, configuration,
-                   def);
+  Json::Value const& defs = data["definitions"];
+  DefinitionsMap definitionsMap;
+  for (auto ldi = defs.begin(), lde = defs.end(); ldi != lde; ++ldi) {
+    cm::string_view const originalLang = IterKey(ldi);
+    cm::string_view const lang = MapLanguage(originalLang);
+    if (lang.empty()) {
+      makefile->IssueMessage(MessageType::WARNING,
+                             cmStrCat("ignoring unknown language "_s,
+                                      originalLang, " in definitions for "_s,
+                                      target->GetName()));
+      continue;
+    }
+
+    for (auto di = ldi->begin(), de = ldi->end(); di != de; ++di) {
+      definitionsMap[di.name()].emplace(lang, *di);
+    }
   }
+  AddDefinitions(makefile, target, configuration, definitionsMap);
 
   // Add include directories.
   for (std::string inc : ReadList(data, "includes")) {
@@ -408,11 +499,11 @@ void cmPackageInfoReader::SetTargetProperties(
                             data["link_name"]);
 
   // Add link languages.
-  for (std::string const& lang : ReadList(data, "link_languages")) {
-    auto const li = Languages.find(cmSystemTools::LowerCase(lang));
-    if (li != Languages.end()) {
+  for (std::string const& originalLang : ReadList(data, "link_languages")) {
+    cm::string_view const lang = MapLanguage(originalLang, DisallowGlob);
+    if (!lang.empty()) {
       AppendProperty(makefile, target, "LINK_LANGUAGES"_s, configuration,
-                     li->second);
+                     std::string{ lang });
     }
   }
 

Tests/FindPackageCpsTest/CMakeLists.txt

@@ -6,6 +6,9 @@ set(CMAKE_EXPERIMENTAL_FIND_CPS_PACKAGES "e82e467b-f997-4464-8ace-b00808fff261")
 # Protect tests from running inside the default install prefix.
 set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/NotDefaultPrefix")
 
+# Use the test directory to Look for packages.
+set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR})
+
 # Disable built-in search paths.
 set(CMAKE_FIND_USE_PACKAGE_ROOT_PATH OFF)
 set(CMAKE_FIND_USE_CMAKE_ENVIRONMENT_PATH OFF)
@@ -21,7 +24,6 @@ add_executable(FindPackageCpsTest FindPackageTest.cxx)
 ###############################################################################
 # Test a basic package search.
 # It should NOT find the package's CMake package file.
-set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR})
 
 find_package(Sample CONFIG)
 if(NOT Sample_FOUND)
@@ -38,11 +40,8 @@ elseif(NOT Sample_VERSION_TWEAK EQUAL 0)
   message(SEND_ERROR "Sample wrong tweak version ${Sample_VERSION_TWEAK} !")
 endif()
 
-set(CMAKE_PREFIX_PATH)
-
 ###############################################################################
 # Test glob sorting.
-set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR})
 
 set(SortLib_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
 set(CMAKE_FIND_PACKAGE_SORT_ORDER NAME)
@@ -93,11 +92,9 @@ unset(SortFramework_VERSION)
 
 unset(CMAKE_FIND_PACKAGE_SORT_ORDER)
 unset(CMAKE_FIND_PACKAGE_SORT_DIRECTION)
-set(CMAKE_PREFIX_PATH)
 
 ###############################################################################
 # Find a package that actually has some content.
-set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR})
 
 find_package(Foo CONFIG)
 if(NOT Foo_FOUND)
@@ -123,3 +120,20 @@ else()
   endif()
   set(rt_includes)
 endif()
+
+###############################################################################
+# Test importing of compile definitions.
+
+find_package(defs CONFIG REQUIRED)
+
+if(CMAKE_GENERATOR STREQUAL "Xcode" OR CMAKE_GENERATOR MATCHES "Visual Studio")
+  # VS/Xcode generators cannot use different definitions within the same
+  # target, so we must split the test into one target per language.
+  add_library(defs-test-c STATIC defs-test-c.c)
+  add_library(defs-test-cxx STATIC defs-test-cxx.cxx)
+  target_link_libraries(defs-test-c defs::defs)
+  target_link_libraries(defs-test-cxx defs::defs)
+else()
+  add_library(defs-test STATIC defs-test-c.c defs-test-cxx.cxx)
+  target_link_libraries(defs-test defs::defs)
+endif()

Tests/FindPackageCpsTest/cps/defs.cps

@@ -0,0 +1,27 @@
+{
+  "cps_version": "0.13",
+  "name": "defs",
+  "cps_path": "@prefix@/cps",
+  "components": {
+    "defs": {
+      "type": "interface",
+      "definitions": {
+        "c": {
+          "ONLY_IN_C": null,
+          "OVERRIDE1": "1",
+          "OVERRIDE2": "1"
+        },
+        "cxx": {
+          "ONLY_IN_CXX": null,
+          "OVERRIDE1": "2"
+        },
+        "*": {
+          "NOVALUE": null,
+          "EMPTYVALUE": "",
+          "OVERRIDE1": "0",
+          "OVERRIDE2": "0"
+        }
+      }
+    }
+  }
+}

Tests/FindPackageCpsTest/defs-test-c.c

@@ -0,0 +1,37 @@
+#ifndef OVERRIDE1
+#  error OVERRIDE1 is not defined
+#endif
+#if OVERRIDE1 != 1
+#  error OVERRIDE1 has the wrong value
+#endif
+
+#ifndef OVERRIDE2
+#  error OVERRIDE2 is not defined
+#endif
+#if OVERRIDE2 != 1
+#  error OVERRIDE2 has the wrong value
+#endif
+
+#ifndef ONLY_IN_C
+#  error ONLY_IN_C is not defined in C sources
+#endif
+#ifdef ONLY_IN_CXX
+#  error ONLY_IN_CXX is defined in C sources
+#endif
+
+#ifndef NOVALUE
+#  error NOVALUE is not defined
+#endif
+#if !defined(__BORLANDC__)
+#  if !NOVALUE
+#    error NOVALUE evaluated as a Boolean is not true
+#  endif
+#endif
+
+#ifndef EMPTYVALUE
+#  error EMPTYVALUE is not defined
+#endif
+
+#if 3 - EMPTYVALUE - 3 != 6
+#  error EMPTYVALUE is not empty
+#endif

Tests/FindPackageCpsTest/defs-test-cxx.cxx

@@ -0,0 +1,37 @@
+#ifndef OVERRIDE1
+#  error OVERRIDE1 is not defined
+#endif
+#if OVERRIDE1 != 2
+#  error OVERRIDE1 has the wrong value
+#endif
+
+#ifndef OVERRIDE2
+#  error OVERRIDE2 is not defined
+#endif
+#if OVERRIDE2 != 0
+#  error OVERRIDE2 has the wrong value
+#endif
+
+#ifdef ONLY_IN_C
+#  error ONLY_IN_C is defined in C++ sources
+#endif
+#ifndef ONLY_IN_CXX
+#  error ONLY_IN_CXX is not defined in C++ sources
+#endif
+
+#ifndef NOVALUE
+#  error NOVALUE is not defined
+#endif
+#if !defined(__BORLANDC__)
+#  if !NOVALUE
+#    error NOVALUE evaluated as a Boolean is not true
+#  endif
+#endif
+
+#ifndef EMPTYVALUE
+#  error EMPTYVALUE is not defined
+#endif
+
+#if 3 - EMPTYVALUE - 3 != 6
+#  error EMPTYVALUE is not empty
+#endif