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

Merge topic 'fix-cmp0199'

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

Acked-by: Kitware Robot <[email protected]>
Tested-by: buildbot <[email protected]>
Acked-by: Matthew Woehlke <[email protected]>
Merge-request: !11581
Brad King 1 месяц назад
Родитель
Сommit
08e1d1001a

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

@@ -116,7 +116,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

@@ -2836,57 +2836,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()) {
@@ -2906,13 +2875,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_",
@@ -2931,11 +2942,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)