Просмотр исходного кода

GenEx: Partially restore pre-CMP0199 behavior of $<CONFIG>

Modify the implementation of policy CMP0199 to only remove the oddball
configuration map matching of `$<CONFIG>` in `NEW` mode, restoring the
old behavior of matching BOTH the consumer's configuration and the
selected configuration of the imported target. It turns out that users
are more dependent on the former than the latter, and while matching
more than one thing is still dodgy, we will likely need to introduce a
new generator expression to match the selected configuration of the
imported target.

Meanwhile, `$<CONFIG>` on targets imported from CPS still only matches
the selected configuration of the imported target, which is the behavior
specified by CPS. However, this can only happen for `$<CONFIG>`
expressions that were generated internally during import.

Update documentation and test cases accordingly.

Fixes: #27487
Fixes: #27495
Matthew Woehlke 1 неделя назад
Родитель
Сommit
35d5a4fd6d

+ 1 - 1
Help/manual/cmake-policies.7.rst

@@ -103,7 +103,7 @@ Policies Introduced by CMake 4.2
    CMP0202: PDB file names always include their target's per-config POSTFIX. </policy/CMP0202>
    CMP0201: Python::NumPy does not depend on Python::Development.Module. </policy/CMP0201>
    CMP0200: Location and configuration selection for imported targets is more consistent. </policy/CMP0200>
-   CMP0199: $<CONFIG> only matches the configuration of the consumed target. </policy/CMP0199>
+   CMP0199: $<CONFIG> does not match mapped configurations that are not selected. </policy/CMP0199>
    CMP0198: CMAKE_PARENT_LIST_FILE is not defined in CMakeLists.txt. </policy/CMP0198>
 
 Policies Introduced by CMake 4.1

+ 28 - 24
Help/policy/CMP0199.rst

@@ -3,7 +3,8 @@ CMP0199
 
 .. versionadded:: 4.2
 
-:genex:`$<CONFIG:cfgs>` only matches the configuration of the consumed target.
+:genex:`$<CONFIG:cfgs>` does not match mapped configurations that are not
+selected.
 
 Historically, when a :genex:`$<CONFIG:cfgs>` generator expression appeared in
 the properties of an imported target, it would match (that is, evaluate to
@@ -18,18 +19,21 @@ the properties of an imported target, it would match (that is, evaluate to
    (where ``<CONFIG>`` is the configuration of the consuming target),
    *whether or not such configurations are valid for the imported target*.
 
-This could result in expressions which are intended to be mutually exclusive
-being concurrently evaluated.  This would be especially problematic if the
-value of a compile definition is intended to be determined by the
-configuration, as this lack of exclusivity could result in redefinition.
+This can result in expressions which are intended to be mutually exclusive
+being concurrently evaluated.  This can be especially problematic if the value
+of a compile definition is intended to be determined by the configuration, as
+this lack of exclusivity could result in redefinition.
 
-CMake 4.2 and above prefer to consider *only* the configuration of the imported
-target being consumed; that is, (1) in the above list.
+CMake 4.2 and above prefer to consider *only* the configuration of the
+consuming target and (when applicable) the selected configuration of the
+imported target; that is, (2) and (1) in the above list.  Unfortunately,
+because users rely on both of these, this policy is not able to fully prevent
+multiple unique ``$<CONFIG:cfg>`` expressions from matching concurrently.
 
 This policy provides compatibility with projects that rely on the historical
 behavior.  The ``OLD`` behavior for this policy is to retain the historic
 behavior as described above.  The ``NEW`` behavior is to consider only the
-configuration of the imported target being consumed.
+configurations of the consuming and consumed targets.
 
 .. note::
 
@@ -43,8 +47,8 @@ configuration of the imported target being consumed.
   configuration, which is necessarily the same as the consuming target's
   configuration.)
 
-  For targets imported from |CPS| packages, the ``NEW`` behavior is used,
-  regardless of the policy setting.
+  For targets imported from |CPS| packages, **only** the configuration of the
+  consumed imported target is considered, regardless of the policy setting.
 
 .. |INTRODUCED_IN_CMAKE_VERSION| replace:: 4.2
 .. |WARNS_OR_DOES_NOT_WARN| replace:: warns
@@ -75,19 +79,19 @@ Consider the following imported libraries:
   )
 
 Assume that the consuming project is built in the ``Release`` configuration.
-Under the ``OLD`` policy, a consumer of ``test1`` would see both ``DEBUG``
-and ``RELEASE`` defined; ``$<CONFIG:debug>`` evaluates to ``1`` because the
-selected configuration of ``test1`` is ``DEBUG``, and ``$<CONFIG:release>``
-evaluates to ``1`` because the consumer's configuration is ``Release``
-(keeping in mind that configuration matching is case-insensitive). Likewise,
-a consumer of ``test2`` would see all of ``DEBUG``, ``RELEASE`` and ``TEST``
-defined; ``$<CONFIG:debug>``, ``$<CONFIG:example>`` and ``$<CONFIG:test>`` all
-evaluate to ``1`` because all of these configurations appear in
-``MAP_IMPORTED_CONFIG_RELEASE``.
-
-Under the ``NEW`` policy, when ``test1`` is consumed, only ``$<CONFIG:debug>``
-will evaluate to ``1``. Similarly, when ``test2`` is consumed, only
-``$<CONFIG:test>`` will evaluate to ``1``. Both of these correspond to the
-configuration of the consumed library that is actually selected by CMake.
+A consumer of ``test1`` will see both ``DEBUG`` and ``RELEASE`` defined,
+regardless of the policy setting; ``$<CONFIG:debug>`` evaluates to ``1``
+because the selected configuration of ``test1`` is ``DEBUG``, and
+``$<CONFIG:release>`` evaluates to ``1`` because the consumer's configuration
+is ``Release`` (keeping in mind that configuration matching is
+case-insensitive).
+
+Under the ``OLD`` policy, a consumer of ``test2`` would see all of ``DEBUG``,
+``EXAMPLE`` and ``TEST`` defined; ``$<CONFIG:debug>``, ``$<CONFIG:example>``
+and ``$<CONFIG:test>`` all evaluate to ``1`` because all of these
+configurations appear in ``MAP_IMPORTED_CONFIG_RELEASE``.
+
+Under the ``NEW`` policy, a consumer of ``test2`` will see only ``TEST``
+defined.
 
 .. |CPS| replace:: Common Package Specification

+ 10 - 2
Help/release/4.2.rst

@@ -203,8 +203,8 @@ Other Changes
 =============
 
 * The :genex:`$<CONFIG:cfgs>` generator expression, when appearing on an
-  imported target, has been fixed to only match the configuration actually
-  being consumed.  See policy :policy:`CMP0199`.
+  imported target, has been fixed to not match configurations that are not
+  applicable.  See policy :policy:`CMP0199`.
 
 * Selection of configuration and location of imported targets is now more
   consistent.  See policy :policy:`CMP0200`.
@@ -236,3 +236,11 @@ Changes made since CMake 4.2.0 include the following.
 * This version made no changes to documented features or interfaces.
   Some implementation updates were made to support ecosystem changes
   and/or fix regressions.
+
+.. 4.2.2
+
+  * Policy :policy:`CMP0199`'s NEW behavior has been partially reverted.
+    In 4.2.0 and 4.2.1, ``$<CONFIG:cfgs>`` only matched the configuration
+    of the consumed target.  This broke existing use cases that rely on
+    matching the configuration of the consuming target, and so has been
+    partially reverted to match either as CMake 4.1 and below did.

+ 69 - 56
Source/cmGeneratorExpressionNode.cxx

@@ -2210,57 +2210,26 @@ static const struct ConfigurationTestNode : public cmGeneratorExpressionNode
       firstParam = false;
     }
 
-    // Determine the context(s) in which the expression should be evaluated. If
-    // CMPxxxx is NEW, the context is exactly one of the imported target's
-    // selected configuration, if applicable, or the consuming target's
-    // configuration, otherwise.
+    // Partially determine the context(s) in which the expression should be
+    // evaluated.
     //
-    // If CMPxxxx is OLD, we evaluate first in the context of the consuming
-    // target, then, if the consumed target is imported, we evaluate based on
-    // the mapped configurations (this logic is... problematic; see comment
-    // below), then finally based on the selected configuration of the imported
-    // target.
+    // If CMPxxxx is NEW, the context is exactly one of the imported target's
+    // selected configuration, if applicable and if the target was imported
+    // from CPS, or the consuming target's configuration otherwise. Here, we
+    // determine if we are in that 'otherwise' branch.
+    //
+    // Longer term, we need a way for non-CPS users to match the selected
+    // configuration of the imported target. At that time, CPS should switch
+    // to that mechanism and the CPS-specific logic here should be dropped.
+    // (We can do that because CPS doesn't use generator expressions directly;
+    // rather, CMake generates them on import.)
     bool const targetIsImported =
       (eval->CurrentTarget && eval->CurrentTarget->IsImported());
-    bool const oldPolicy = [&] {
-      if (!targetIsImported) {
-        // For non-imported targets, there is no behavior difference between
-        // the OLD and NEW policy.
-        return false;
-      }
-      cmTarget const* const t = eval->CurrentTarget->Target;
-      if (t->GetOrigin() == cmTarget::Origin::Cps) {
-        // Generator expressions appearing on targets imported from CPS should
-        // always be evaluated according to the selected configuration of the
-        // imported target, i.e. the NEW policy.
-        return false;
-      }
-      switch (eval->HeadTarget->GetPolicyStatusCMP0199()) {
-        case cmPolicies::WARN:
-          if (eval->Context.LG->GetMakefile()->PolicyOptionalWarningEnabled(
-                "CMAKE_POLICY_WARNING_CMP0199")) {
-            std::string const err =
-              cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0199),
-                       "\nEvaluation of $<CONFIG> for imported target  \"",
-                       eval->CurrentTarget->GetName(), "\", used by \"",
-                       eval->HeadTarget->GetName(),
-                       "\", may match multiple configurations.\n");
-            eval->Context.LG->GetCMakeInstance()->IssueMessage(
-              MessageType ::AUTHOR_WARNING, err, eval->Backtrace);
-          }
-          CM_FALLTHROUGH;
-        case cmPolicies::OLD:
-          return true;
-        case cmPolicies::NEW:
-          return false;
-      }
-
-      // Should be unreachable
-      assert(false);
-      return false;
-    }();
+    bool const useConsumerConfig =
+      (targetIsImported &&
+       eval->CurrentTarget->Target->GetOrigin() != cmTarget::Origin::Cps);
 
-    if (!targetIsImported || oldPolicy) {
+    if (!targetIsImported || useConsumerConfig) {
       // Does the consuming target's configuration match any of the arguments?
       for (auto const& param : parameters) {
         if (eval->Context.Config.empty()) {
@@ -2280,13 +2249,55 @@ static const struct ConfigurationTestNode : public cmGeneratorExpressionNode
       std::string suffix;
       if (eval->CurrentTarget->Target->GetMappedConfig(eval->Context.Config,
                                                        loc, imp, suffix)) {
+        // Finish determine the context(s) in which the expression should be
+        // evaluated. Note that we use the consumer's policy, so that end users
+        // can override the imported target's policy. This may be needed if
+        // upstream has changed their policy version without realizing that
+        // consumers were depending on the OLD behavior.
+        bool const oldPolicy = [&] {
+          if (!useConsumerConfig) {
+            // Targets imported from CPS shall use only the selected
+            // configuration of the imported target.
+            return false;
+          }
+          cmLocalGenerator const* const lg = eval->Context.LG;
+          switch (eval->HeadTarget->GetPolicyStatusCMP0199()) {
+            case cmPolicies::WARN:
+              if (lg->GetMakefile()->PolicyOptionalWarningEnabled(
+                    "CMAKE_POLICY_WARNING_CMP0199")) {
+                std::string const err =
+                  cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0199),
+                           "\nEvaluation of $<CONFIG> for imported target  \"",
+                           eval->CurrentTarget->GetName(), "\", used by \"",
+                           eval->HeadTarget->GetName(),
+                           "\", may match multiple configurations.\n");
+                lg->GetCMakeInstance()->IssueMessage(
+                  MessageType ::AUTHOR_WARNING, err, eval->Backtrace);
+              }
+              CM_FALLTHROUGH;
+            case cmPolicies::OLD:
+              return true;
+            case cmPolicies::NEW:
+              return false;
+          }
+
+          // Should be unreachable
+          assert(false);
+          return false;
+        }();
+
         if (oldPolicy) {
+          // If CMPxxxx is OLD (and we aren't dealing with a target imported
+          // form CPS), we already evaluated in the context of the consuming
+          // target. Next, for imported targets, we will evaluate based on the
+          // mapped configurations.
+          //
           // If the target has a MAP_IMPORTED_CONFIG_<CONFIG> property for the
-          // consumer's <CONFIG>, match *any* config in that list, regardless
-          // of whether it's valid or of what GetMappedConfig actually picked.
-          // This will result in $<CONFIG> producing '1' for multiple configs,
-          // and is almost certainly wrong, but it's what CMake did for a very
-          // long time, and... Hyrum's Law.
+          // consumer's <CONFIG>, we will match *any* config in that list,
+          // regardless of whether it's valid or of what GetMappedConfig
+          // actually picked. This will result in $<CONFIG> producing '1' for
+          // multiple configs, and is almost certainly wrong, but it's what
+          // CMake did for a very long time, and... Hyrum's Law.
           cmList mappedConfigs;
           std::string mapProp =
             cmStrCat("MAP_IMPORTED_CONFIG_",
@@ -2305,11 +2316,13 @@ static const struct ConfigurationTestNode : public cmGeneratorExpressionNode
           }
         }
 
-        // This imported target has an appropriate location for this (possibly
-        // mapped) config.
+        // Finally, check if we selected (possibly via mapping) a configuration
+        // for this imported target, and if we should evaluate the expression
+        // in the context of the same.
+        //
+        // For targets imported from CPS, this is the only context we evaluate
+        // the expression.
         if (!suffix.empty()) {
-          // Use the (possibly mapped) configuration of the imported location
-          // that was selected.
           for (auto const& param : parameters) {
             if (cmStrCat('_', cmSystemTools::UpperCase(param)) == suffix) {
               return "1";

+ 4 - 4
Source/cmPolicies.h

@@ -593,10 +593,10 @@ class cmMakefile;
   SELECT(POLICY, CMP0198,                                                     \
          "CMAKE_PARENT_LIST_FILE is not defined in CMakeLists.txt.", 4, 2, 0, \
          WARN)                                                                \
-  SELECT(                                                                     \
-    POLICY, CMP0199,                                                          \
-    "$<CONFIG:cfgs> only matches the configuration of the consumed target.",  \
-    4, 2, 0, WARN)                                                            \
+  SELECT(POLICY, CMP0199,                                                     \
+         "$<CONFIG:cfgs> does not match mapped configurations that are not "  \
+         "selected.",                                                         \
+         4, 2, 0, WARN)                                                       \
   SELECT(POLICY, CMP0200,                                                     \
          "Location and configuration selection for imported targets is more " \
          "consistent.",                                                       \

+ 1 - 1
Tests/RunCMake/GeneratorExpression/CMP0199-NEW+CMP0200-NEW.cmake

@@ -5,4 +5,4 @@ cmake_policy(SET CMP0200 NEW)
 
 include(CMP0199-cases.cmake)
 
-do_mapped_config_test(NEW EXPECT_TEST)
+do_mapped_config_test(NEW EXPECT_RELEASE EXPECT_TEST)

+ 0 - 1
Tests/RunCMake/GeneratorExpression/CMP0199-NEW.cmake

@@ -10,4 +10,3 @@ cmake_policy(SET CMP0200 OLD)
 include(CMP0199-cases.cmake)
 
 do_mapped_config_test(EXPECT_RELEASE)
-do_unique_config_test(EXPECT_DEBUG)

+ 0 - 1
Tests/RunCMake/GeneratorExpression/CMP0199-OLD.cmake

@@ -5,4 +5,3 @@ cmake_policy(SET CMP0199 OLD)
 include(CMP0199-cases.cmake)
 
 do_mapped_config_test(EXPECT_RELEASE EXPECT_DEBUG EXPECT_TEST)
-do_unique_config_test(EXPECT_RELEASE EXPECT_DEBUG)

+ 4 - 3
Tests/RunCMake/GeneratorExpression/CMP0199-WARN-stderr.txt

@@ -1,7 +1,8 @@
 CMake Warning \(dev\) at CMP0199-WARN\.cmake:[0-9]+ \(target_link_libraries\):
-  Policy CMP0199 is not set: \$<CONFIG:cfgs> only matches the configuration of
-  the consumed target\.  Run "cmake --help-policy CMP0199" for policy details\.
-  Use the cmake_policy command to set the policy and suppress this warning\.
+  Policy CMP0199 is not set: \$<CONFIG:cfgs> does not match mapped
+  configurations that are not selected\.  Run "cmake --help-policy CMP0199"
+  for policy details\.  Use the cmake_policy command to set the policy and
+  suppress this warning\.
 
   Evaluation of \$<CONFIG> for imported target "lib_test", used by "exe_test",
   may match multiple configurations\.

+ 4 - 19
Tests/RunCMake/GeneratorExpression/CMP0199-cases.cmake

@@ -1,6 +1,7 @@
-# Under CMP0199 OLD, $<CONFIG> matches not just the selected configuration, but
-# every entry in MAP_IMPORTED_CONFIG_<CONFIG>. Under NEW, it should only match
-# the selected configuration.
+# Under CMP0199 OLD, $<CONFIG> matches the selected configuration and every
+# entry in MAP_IMPORTED_CONFIG_<CONFIG>. Under NEW, it should only match the
+# configuration of the consuming target and the selected configuration of the
+# library being consumed.
 function(do_mapped_config_test)
   add_library(lib_mapped INTERFACE IMPORTED)
   set_target_properties(lib_mapped PROPERTIES
@@ -14,19 +15,3 @@ function(do_mapped_config_test)
   target_compile_definitions(exe_mapped PRIVATE ${ARGN})
   target_link_libraries(exe_mapped PRIVATE lib_mapped)
 endfunction()
-
-# Under CMake CMP0199 OLD, $<CONFIG> matches both the consumer's configuration
-# AND the selected configuration of the library being consumed. Under NEW, it
-# should only match the selected configuration.
-function(do_unique_config_test)
-  add_library(lib_unique INTERFACE IMPORTED)
-  set_target_properties(lib_unique PROPERTIES
-    IMPORTED_CONFIGURATIONS "DEBUG"
-    INTERFACE_COMPILE_DEFINITIONS
-      "$<$<CONFIG:debug>:DEBUG>;$<$<CONFIG:release>:RELEASE>"
-  )
-
-  add_executable(exe_unique configtest.c)
-  target_compile_definitions(exe_unique PRIVATE ${ARGN})
-  target_link_libraries(exe_unique PRIVATE lib_unique)
-endfunction()

+ 1 - 1
Tests/RunCMake/GeneratorExpression/CMP0200-OLD.cmake

@@ -5,5 +5,5 @@ cmake_policy(SET CMP0200 OLD)
 
 include(CMP0200-cases.cmake)
 
-do_match_config_test(EXPECT_DEBUG)
+do_match_config_test(EXPECT_DEBUG EXPECT_RELEASE)
 do_first_config_test(EXPECT_DEBUG)

+ 9 - 1
Tests/RunCMake/GeneratorExpression/CMP0200-cases.cmake

@@ -1,7 +1,15 @@
+# TODO: $<CONFIG> matches both the conumer's configuration AND the selected
+# configuration of the imported target. This is ungood, and eventually we need
+# a way to match only the selected configuration of the imported target. For
+# historic reasons, that will probably not be $<CONFIG>, which means $<CONFIG>
+# should eventually stop matching the selected configuration of the imported
+# target. When that happens, this test should be changed to use the new
+# mechanism, and the test cases adjusted accordingly.
+
 # Under CMP0200 OLD, CMake fails to select a valid configuration for an
 # imported INTERFACE library with no location, and will (as an implementation
 # artifact) select the last configuration in IMPORTED_CONFIGURATIONS.
-
+#
 # Under NEW, CMake should select a configuration which matches the current
 # build type, if available in IMPORTED_CONFIGURATIONS.
 function(do_match_config_test)