1
0
Эх сурвалжийг харах

cmTarget: Overhaul GetMappedConfig

Create a brand new implementation of `cmTarget::GetMappedConfig` which
prioritized a target's `IMPORTED_CONFIGURATIONS` as the 'source of
truth' for what configurations are available. In particular, this means
that configuration selection when `IMPORTED_CONFIGURATIONS` is set does
not depend on the library type in any manner. The fallback logic also
uses a more consistent 'usability' criteria that should result in more
consistent configuration selection, particularly for `INTERFACE`
targets.

The previous implementation is retained as a separate method for users
requesting the OLD behavior.

Fixes: #27022
Matthew Woehlke 2 сар өмнө
parent
commit
05ae95c864

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

@@ -98,6 +98,7 @@ Policies Introduced by CMake 4.2
 .. toctree::
    :maxdepth: 1
 
+   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>
    CMP0198: CMAKE_PARENT_LIST_FILE is not defined in CMakeLists.txt. </policy/CMP0198>
 

+ 117 - 0
Help/policy/CMP0200.rst

@@ -0,0 +1,117 @@
+CMP0200
+-------
+
+.. versionadded:: 4.2
+
+Location and configuration selection for imported targets is more consistent.
+
+The way CMake historically selected the configuration to use for imported
+targets prioritized selection based on location properties for a candidate
+configuration and only considered :prop_tgt:`IMPORTED_CONFIGURATIONS` as a
+fallback.  This could result in incorrect configuration selection especially
+for ``INTERFACE`` libraries.
+
+CMake 4.2 and above consider :prop_tgt:`IMPORTED_CONFIGURATIONS` to be a
+definitive list of available configurations, regardless of whether a
+configuration specific location is provided for the library.  Additionally,
+CMake will respect non-configuration-specific locations when a configuration
+specific location is not specified.
+
+This policy provides compatibility with projects that rely on the historical
+behavior.  The policy setting applies to targets and is recorded at the point
+an imported target is created.  Accordingly, imported packages may override the
+policy set by the consumer for targets they create.  In particular, targets
+imported from |CPS| packages always use the ``NEW`` behavior.
+
+The ``OLD`` behavior for this policy is to retain the historic behavior.
+The ``NEW`` behavior prioritizes selection based on the advertised list of
+available configurations.  Both behaviors are described in detail below.
+
+.. |INTRODUCED_IN_CMAKE_VERSION| replace:: 4.2
+.. |WARNS_OR_DOES_NOT_WARN| replace:: warns
+.. include:: include/STANDARD_ADVICE.rst
+
+.. include:: include/DEPRECATED.rst
+
+Mapped configuration selection
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If :prop_tgt:`MAP_IMPORTED_CONFIG_<CONFIG>` (where ``<CONFIG>`` is the
+configuration of the consuming target) is set on an imported target, CMake
+would historically select from that list the first configuration which provides
+a configuration-specific location.  If no such configuration exists, CMake
+would selects the consuming target's configuration, if the imported target is
+an ``INTERFACE`` library.  Otherwise, CMake considers the target as not having
+a suitable configuration.
+
+For ``INTERFACE`` libraries which do not provide a location, this results in
+CMake always selecting the consuming target's configuration and effectively
+ignoring :prop_tgt:`MAP_IMPORTED_CONFIG_<CONFIG>`.  This behavior is not
+consistent with configuration selection for imported targets which provide a
+location.
+
+Under the ``NEW`` behavior, CMake selects the first configuration from the
+mapping which appears in :prop_tgt:`IMPORTED_CONFIGURATIONS`.  If
+:prop_tgt:`IMPORTED_CONFIGURATIONS` is not set, CMake selects the first
+configuration from the mapping which is "usable".  For non-``INTERFACE``
+libraries, "usable" means that a location (either configuration-specific or
+configuration-agnostic) is available.  ``INTERFACE`` libraries are always
+considered "usable".
+
+If no match is found, CMake considers the target as not having a suitable
+configuration.
+
+Non-mapped configuration selection
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If :prop_tgt:`MAP_IMPORTED_CONFIG_<CONFIG>` is *not* set, CMake would
+historically select the first configuration which provides a location out of
+the following:
+
+- The consuming target's configuration, or
+
+- The empty configuration, or
+
+- The list of configurations in :prop_tgt:`IMPORTED_CONFIGURATIONS`.
+
+As an implementation artifact, this results in CMake selecting the *last*
+configuration in :prop_tgt:`IMPORTED_CONFIGURATIONS` for ``INTERFACE``
+libraries which do not provide a location.  Again, this behavior is not
+consistent with configuration selection for imported targets which provide a
+location.
+
+Under the ``NEW`` behavior, if :prop_tgt:`IMPORTED_CONFIGURATIONS` is set,
+CMake will select the consuming target's configuration if present therein,
+otherwise CMake will select the first imported configuration.  If
+:prop_tgt:`IMPORTED_CONFIGURATIONS` is *not* set, CMake will select the
+consuming target's configuration if it is "usable" (as defined in the previous
+section); otherwise, CMake considers the target as not having a suitable
+configuration.
+
+Examples
+^^^^^^^^
+
+Consider the following imported library:
+
+.. code-block:: cmake
+
+  add_library(test INTERFACE IMPORTED)
+  set_target_properties(test PROPERTIES
+    IMPORTED_CONFIGURATIONS "RELEASE;DEBUG"
+    INTERFACE_COMPILE_DEFINITIONS "$<$<CONFIG:debug>:DEBUG>"
+  )
+
+Under the ``OLD`` policy, CMake will select the ``DEBUG`` configuration of
+``test`` (and thus define the symbol ``DEBUG``) for any target linking to
+``test``, because CMake does not consider any configuration "valid", and, as
+an implementation artifact, the last configuration considered is accepted.
+
+Under the ``NEW`` policy, the ``RELEASE`` configuration will be selected
+if the consuming project is built in any configuration other than ``Debug``
+(keeping in mind that configuration matching is case-insensitive).  This is
+because ``DEBUG`` will be preferred if the consumer's configuration is also
+``DEBUG``, but ``RELEASE`` will be preferred otherwise because it appears
+first in :prop_tgt:`IMPORTED_CONFIGURATIONS`, and its appearance therein makes
+it a "valid" configuration for an ``INTERFACE`` library.
+
+.. |CPS| replace:: Common Package Specification

+ 5 - 0
Help/release/dev/imported-config-selection.rst

@@ -0,0 +1,5 @@
+imported-config-selection
+-------------------------
+
+Location and configuration selection for imported targets is now more
+consistent.  See policy :policy:`CMP0200`.

+ 7 - 2
Source/cmPolicies.h

@@ -596,7 +596,11 @@ class cmMakefile;
   SELECT(                                                                     \
     POLICY, CMP0199,                                                          \
     "$<CONFIG:cfgs> only matches the configuration of the consumed target.",  \
-    4, 2, 0, WARN)
+    4, 2, 0, WARN)                                                            \
+  SELECT(POLICY, CMP0200,                                                     \
+         "Location and configuration selection for imported targets is more " \
+         "consistent.",                                                       \
+         4, 2, 0, WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
@@ -645,7 +649,8 @@ class cmMakefile;
   F(CMP0181)                                                                  \
   F(CMP0182)                                                                  \
   F(CMP0195)                                                                  \
-  F(CMP0199)
+  F(CMP0199)                                                                  \
+  F(CMP0200)
 
 #define CM_FOR_EACH_CUSTOM_COMMAND_POLICY(F)                                  \
   F(CMP0116)                                                                  \

+ 171 - 0
Source/cmTarget.cxx

@@ -3212,6 +3212,69 @@ bool cmTargetInternals::CheckImportedLibName(std::string const& prop,
 
 bool cmTarget::GetMappedConfig(std::string const& desired_config, cmValue& loc,
                                cmValue& imp, std::string& suffix) const
+{
+  switch (this->GetPolicyStatusCMP0200()) {
+    case cmPolicies::WARN:
+      if (this->GetMakefile()->PolicyOptionalWarningEnabled(
+            "CMAKE_POLICY_WARNING_CMP0200")) {
+        break;
+      }
+      CM_FALLTHROUGH;
+    case cmPolicies::OLD:
+      return this->GetMappedConfigOld(desired_config, loc, imp, suffix);
+    case cmPolicies::NEW:
+      return this->GetMappedConfigNew(desired_config, loc, imp, suffix);
+  }
+
+  cmValue newLoc;
+  cmValue newImp;
+  std::string newSuffix;
+
+  bool const newResult =
+    this->GetMappedConfigNew(desired_config, newLoc, newImp, newSuffix);
+
+  if (!this->GetMappedConfigOld(desired_config, loc, imp, suffix)) {
+    if (newResult) {
+      // NEW policy found a configuration, OLD did not.
+      auto newConfig = cm::string_view{ newSuffix }.substr(1);
+      std::string const err = cmStrCat(
+        cmPolicies::GetPolicyWarning(cmPolicies::CMP0200),
+        "\nConfiguration selection for imported target \"", this->GetName(),
+        "\" failed, but would select configuration \"", newConfig,
+        "\" under the NEW policy.\n");
+      this->GetMakefile()->IssueMessage(MessageType::AUTHOR_WARNING, err);
+    }
+
+    return false;
+  }
+
+  auto oldConfig = cm::string_view{ suffix }.substr(1);
+  if (!newResult) {
+    // NEW policy did not find a configuration, OLD did.
+    std::string const err =
+      cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0200),
+               "\nConfiguration selection for imported target \"",
+               this->GetName(), "\" selected configuration \"", oldConfig,
+               "\", but would fail under the NEW policy.\n");
+    this->GetMakefile()->IssueMessage(MessageType::AUTHOR_WARNING, err);
+  } else if (suffix != newSuffix) {
+    // OLD and NEW policies found different configurations.
+    auto newConfig = cm::string_view{ newSuffix }.substr(1);
+    std::string const err =
+      cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0200),
+               "\nConfiguration selection for imported target \"",
+               this->GetName(), "\" selected configuration \"", oldConfig,
+               "\", but would select configuration \"", newConfig,
+               "\" under the NEW policy.\n");
+    this->GetMakefile()->IssueMessage(MessageType::AUTHOR_WARNING, err);
+  }
+
+  return true;
+}
+
+bool cmTarget::GetMappedConfigOld(std::string const& desired_config,
+                                  cmValue& loc, cmValue& imp,
+                                  std::string& suffix) const
 {
   std::string config_upper;
   if (!desired_config.empty()) {
@@ -3339,3 +3402,111 @@ bool cmTarget::GetMappedConfig(std::string const& desired_config, cmValue& loc,
 
   return true;
 }
+
+cmValue cmTarget::GetLocation(std::string const& base,
+                              std::string const& suffix) const
+{
+  cmValue value = this->GetProperty(cmStrCat(base, suffix));
+  if (value || suffix.empty()) {
+    return value;
+  }
+  return this->GetProperty(base);
+}
+
+bool cmTarget::GetLocation(std::string const& config, cmValue& loc,
+                           cmValue& imp, std::string& suffix) const
+{
+  suffix = (config.empty() ? std::string{}
+                           : cmStrCat('_', cmSystemTools::UpperCase(config)));
+
+  // There may be only IMPORTED_IMPLIB for a shared library or an executable
+  // with exports.
+  bool const allowImp = (this->GetType() == cmStateEnums::SHARED_LIBRARY ||
+                         this->IsExecutableWithExports()) ||
+    (this->IsAIX() && this->IsExecutableWithExports()) ||
+    (this->GetMakefile()->PlatformSupportsAppleTextStubs() &&
+     this->IsSharedLibraryWithExports());
+
+  if (allowImp) {
+    imp = this->GetLocation("IMPORTED_IMPLIB", suffix);
+  }
+
+  switch (this->GetType()) {
+    case cmStateEnums::INTERFACE_LIBRARY:
+      loc = this->GetLocation("IMPORTED_LIBNAME", suffix);
+      break;
+    case cmStateEnums::OBJECT_LIBRARY:
+      loc = this->GetLocation("IMPORTED_OBJECTS", suffix);
+      break;
+    default:
+      loc = this->GetLocation("IMPORTED_LOCATION", suffix);
+      break;
+  }
+
+  return loc || imp || (this->GetType() == cmStateEnums::INTERFACE_LIBRARY);
+}
+
+bool cmTarget::GetMappedConfigNew(std::string const& desired_config,
+                                  cmValue& loc, cmValue& imp,
+                                  std::string& suffix) const
+{
+  // Get configuration mapping, if present.
+  cmList mappedConfigs;
+  if (!desired_config.empty()) {
+    std::string mapProp = cmStrCat("MAP_IMPORTED_CONFIG_",
+                                   cmSystemTools::UpperCase(desired_config));
+    if (cmValue mapValue = this->GetProperty(mapProp)) {
+      mappedConfigs.assign(*mapValue, cmList::EmptyElements::Yes);
+    }
+  }
+
+  // Get imported configurations, if specified.
+  if (cmValue iconfigs = this->GetProperty("IMPORTED_CONFIGURATIONS")) {
+    cmList const availableConfigs{ iconfigs };
+
+    if (!mappedConfigs.empty()) {
+      for (auto const& c : mappedConfigs) {
+        if (cm::contains(availableConfigs, c)) {
+          this->GetLocation(c, loc, imp, suffix);
+          return true;
+        }
+      }
+
+      // If a configuration mapping was specified, but no matching
+      // configuration was found, we don't want to try anything else.
+      return false;
+    }
+
+    // There is no mapping; try the requested configuration first.
+    if (cm::contains(availableConfigs, desired_config)) {
+      this->GetLocation(desired_config, loc, imp, suffix);
+      return true;
+    }
+
+    // If there is no mapping and the requested configuration is not one of
+    // the available configurations, just take the first available
+    // configuration.
+    this->GetLocation(availableConfigs[0], loc, imp, suffix);
+    return true;
+  }
+
+  if (!mappedConfigs.empty()) {
+    for (auto const& c : mappedConfigs) {
+      if (this->GetLocation(c, loc, imp, suffix)) {
+        return true;
+      }
+    }
+
+    // If a configuration mapping was specified, but no matching
+    // configuration was found, we don't want to try anything else.
+    return false;
+  }
+
+  // There is no mapping and no explicit list of configurations; the only
+  // configuration left to try is the requested configuration.
+  if (this->GetLocation(desired_config, loc, imp, suffix)) {
+    return true;
+  }
+
+  return false;
+}

+ 9 - 0
Source/cmTarget.h

@@ -350,6 +350,15 @@ private:
   // Internal representation details.
   friend class cmGeneratorTarget;
 
+  bool GetMappedConfigOld(std::string const& desired_config, cmValue& loc,
+                          cmValue& imp, std::string& suffix) const;
+  bool GetMappedConfigNew(std::string const& desired_config, cmValue& loc,
+                          cmValue& imp, std::string& suffix) const;
+  cmValue GetLocation(std::string const& base,
+                      std::string const& suffix) const;
+  bool GetLocation(std::string const& config, cmValue& loc, cmValue& imp,
+                   std::string& suffix) const;
+
   char const* GetSuffixVariableInternal(
     cmStateEnums::ArtifactType artifact) const;
   char const* GetPrefixVariableInternal(

+ 2 - 0
Tests/GeneratorExpression/CMakeLists.txt

@@ -260,6 +260,8 @@ add_custom_target(check-part3 ALL
     -Dconfig=$<CONFIGURATION>
     -Dtest_imported_includes=$<TARGET_PROPERTY:imported4,INCLUDE_DIRECTORIES>
     -Dtest_imported_fallback=$<STREQUAL:$<TARGET_FILE_NAME:importedFallback>,fallback_loc>
+    # NOTE: CMP0200 results in this evaluating to fallback_loc on platforms
+    # that do not have special handling of IMPORTED_IMPLIB.
     -Dtest_imported_fallback2=$<STREQUAL:$<TARGET_LINKER_FILE_NAME:importedFallback2>,special_imp>
     -Dtest_imported_fallback3=$<IF:$<PLATFORM_ID:Windows,CYGWIN,MSYS>,$<STREQUAL:$<TARGET_LINKER_FILE_NAME:importedFallback3>,imp_loc>,$<STREQUAL:$<TARGET_LINKER_FILE_NAME:importedFallback3>,fallback_loc>>
     -Dtest_imported_fallback4=$<IF:$<PLATFORM_ID:Windows,CYGWIN,MSYS>,$<STREQUAL:$<TARGET_LINKER_FILE_NAME:importedFallback4>,imp_loc>,$<STREQUAL:$<TARGET_LINKER_FILE_NAME:importedFallback4>,fallback_loc>>

+ 8 - 0
Tests/RunCMake/GeneratorExpression/CMP0199-NEW+CMP0200-NEW.cmake

@@ -0,0 +1,8 @@
+project(test-CMP0199-NEW C)
+
+cmake_policy(SET CMP0199 NEW)
+cmake_policy(SET CMP0200 NEW)
+
+include(CMP0199-cases.cmake)
+
+do_mapped_config_test(NEW EXPECT_TEST)

+ 5 - 2
Tests/RunCMake/GeneratorExpression/CMP0199-NEW.cmake

@@ -2,9 +2,12 @@ project(test-CMP0199-NEW C)
 
 cmake_policy(SET CMP0199 NEW)
 
+# Note: Under CMP0199 OLD, CMake (incorrectly) selects the RELEASE
+# configuration for the mapped config test. The CMP0199-NEW+CMP0200-NEW tests
+# the combination of fixes.
+cmake_policy(SET CMP0200 OLD)
+
 include(CMP0199-cases.cmake)
 
-# FIXME: CMake currently incorrectly selects the RELEASE configuration. See
-# https://gitlab.kitware.com/cmake/cmake/-/issues/27022.
 do_mapped_config_test(EXPECT_RELEASE)
 do_unique_config_test(EXPECT_DEBUG)

+ 9 - 0
Tests/RunCMake/GeneratorExpression/CMP0200-NEW.cmake

@@ -0,0 +1,9 @@
+project(test-CMP0200-OLD C)
+
+cmake_policy(SET CMP0199 NEW)
+cmake_policy(SET CMP0200 NEW)
+
+include(CMP0200-cases.cmake)
+
+do_match_config_test(EXPECT_RELEASE)
+do_first_config_test(EXPECT_TEST)

+ 9 - 0
Tests/RunCMake/GeneratorExpression/CMP0200-OLD.cmake

@@ -0,0 +1,9 @@
+project(test-CMP0200-OLD C)
+
+cmake_policy(SET CMP0199 NEW)
+cmake_policy(SET CMP0200 OLD)
+
+include(CMP0200-cases.cmake)
+
+do_match_config_test(EXPECT_DEBUG)
+do_first_config_test(EXPECT_DEBUG)

+ 11 - 0
Tests/RunCMake/GeneratorExpression/CMP0200-WARN-stderr.txt

@@ -0,0 +1,11 @@
+CMake Warning \(dev\) in CMakeLists\.txt:
+  Policy CMP0200 is not set: Location and configuration selection for
+  imported targets is more consistent\.  Run "cmake --help-policy CMP0200" for
+  policy details\.  Use the cmake_policy command to set the policy and
+  suppress this warning\.
+
+  Configuration selection for imported target "lib_test" selected
+  configuration "CAT", but would select configuration "DOG" under the NEW
+  policy\.
+
+This warning is for project developers\.  Use -Wno-dev to suppress it\.

+ 11 - 0
Tests/RunCMake/GeneratorExpression/CMP0200-WARN.cmake

@@ -0,0 +1,11 @@
+project(test-CMP0200-WARN C)
+
+set(CMAKE_POLICY_WARNING_CMP0200 ON)
+
+add_library(lib_test INTERFACE IMPORTED)
+set_target_properties(lib_test PROPERTIES
+  IMPORTED_CONFIGURATIONS "DOG;CAT"
+)
+
+add_executable(exe_test configtest.c)
+target_link_libraries(exe_test PRIVATE lib_test)

+ 33 - 0
Tests/RunCMake/GeneratorExpression/CMP0200-cases.cmake

@@ -0,0 +1,33 @@
+# 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)
+  add_library(lib_match INTERFACE IMPORTED)
+  set_target_properties(lib_match PROPERTIES
+    IMPORTED_CONFIGURATIONS "RELEASE;DEBUG"
+    INTERFACE_COMPILE_DEFINITIONS
+      "$<$<CONFIG:debug>:DEBUG>;$<$<CONFIG:release>:RELEASE>"
+  )
+
+  add_executable(exe_match configtest.c)
+  target_compile_definitions(exe_match PRIVATE ${ARGN})
+  target_link_libraries(exe_match PRIVATE lib_match)
+endfunction()
+
+# Under NEW, CMake should select the first of IMPORTED_CONFIGURATIONS if no
+# mapping exists and no configuration matching the current build type exists.
+function(do_first_config_test)
+  add_library(lib_first INTERFACE IMPORTED)
+  set_target_properties(lib_first PROPERTIES
+    IMPORTED_CONFIGURATIONS "TEST;DEBUG"
+    INTERFACE_COMPILE_DEFINITIONS
+      "$<$<CONFIG:debug>:DEBUG>;$<$<CONFIG:test>:TEST>"
+  )
+
+  add_executable(exe_first configtest.c)
+  target_compile_definitions(exe_first PRIVATE ${ARGN})
+  target_link_libraries(exe_first PRIVATE lib_first)
+endfunction()

+ 6 - 0
Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake

@@ -123,6 +123,12 @@ run_cmake(CMP0199-WARN)
 run_cmake_build(CMP0199-OLD)
 run_cmake_build(CMP0199-NEW)
 
+run_cmake(CMP0200-WARN)
+run_cmake_build(CMP0200-OLD)
+run_cmake_build(CMP0200-NEW)
+
+run_cmake_build(CMP0199-NEW+CMP0200-NEW)
+
 set(RunCMake_TEST_OPTIONS -DCMAKE_POLICY_DEFAULT_CMP0085:STRING=OLD)
 run_cmake(CMP0085-OLD)
 unset(RunCMake_TEST_OPTIONS)

+ 1 - 0
Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt

@@ -48,6 +48,7 @@
    \* CMP0182
    \* CMP0195
    \* CMP0199
+   \* CMP0200
 
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)