diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index 4dbabedeb2..5b0d129f1b 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/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. CMP0199: $ only matches the configuration of the consumed target. CMP0198: CMAKE_PARENT_LIST_FILE is not defined in CMakeLists.txt. diff --git a/Help/policy/CMP0200.rst b/Help/policy/CMP0200.rst new file mode 100644 index 0000000000..cf2dbde8db --- /dev/null +++ b/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_` (where ```` 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_`. 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_` 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 "$<$: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 diff --git a/Help/release/dev/imported-config-selection.rst b/Help/release/dev/imported-config-selection.rst new file mode 100644 index 0000000000..07ee6a86ee --- /dev/null +++ b/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`. diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index f9b28cfbe2..e6ddb29c59 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -596,7 +596,11 @@ class cmMakefile; SELECT( \ POLICY, CMP0199, \ "$ 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) \ diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index d87fd7c41f..3debbb198a 100644 --- a/Source/cmTarget.cxx +++ b/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; +} diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 4d75f45d3e..e5dffe8897 100644 --- a/Source/cmTarget.h +++ b/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( diff --git a/Tests/GeneratorExpression/CMakeLists.txt b/Tests/GeneratorExpression/CMakeLists.txt index 4d73bf94e1..23715506bc 100644 --- a/Tests/GeneratorExpression/CMakeLists.txt +++ b/Tests/GeneratorExpression/CMakeLists.txt @@ -260,6 +260,8 @@ add_custom_target(check-part3 ALL -Dconfig=$ -Dtest_imported_includes=$ -Dtest_imported_fallback=$,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=$,special_imp> -Dtest_imported_fallback3=$,$,imp_loc>,$,fallback_loc>> -Dtest_imported_fallback4=$,$,imp_loc>,$,fallback_loc>> diff --git a/Tests/RunCMake/GeneratorExpression/CMP0199-NEW+CMP0200-NEW.cmake b/Tests/RunCMake/GeneratorExpression/CMP0199-NEW+CMP0200-NEW.cmake new file mode 100644 index 0000000000..2285854569 --- /dev/null +++ b/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) diff --git a/Tests/RunCMake/GeneratorExpression/CMP0199-NEW.cmake b/Tests/RunCMake/GeneratorExpression/CMP0199-NEW.cmake index 5c1dd39402..3efdc0e0d4 100644 --- a/Tests/RunCMake/GeneratorExpression/CMP0199-NEW.cmake +++ b/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) diff --git a/Tests/RunCMake/GeneratorExpression/CMP0200-NEW.cmake b/Tests/RunCMake/GeneratorExpression/CMP0200-NEW.cmake new file mode 100644 index 0000000000..13a45526a8 --- /dev/null +++ b/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) diff --git a/Tests/RunCMake/GeneratorExpression/CMP0200-OLD.cmake b/Tests/RunCMake/GeneratorExpression/CMP0200-OLD.cmake new file mode 100644 index 0000000000..104e7e3f38 --- /dev/null +++ b/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) diff --git a/Tests/RunCMake/GeneratorExpression/CMP0200-WARN-stderr.txt b/Tests/RunCMake/GeneratorExpression/CMP0200-WARN-stderr.txt new file mode 100644 index 0000000000..aa0402ca95 --- /dev/null +++ b/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\. diff --git a/Tests/RunCMake/GeneratorExpression/CMP0200-WARN.cmake b/Tests/RunCMake/GeneratorExpression/CMP0200-WARN.cmake new file mode 100644 index 0000000000..a91bc55875 --- /dev/null +++ b/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) diff --git a/Tests/RunCMake/GeneratorExpression/CMP0200-cases.cmake b/Tests/RunCMake/GeneratorExpression/CMP0200-cases.cmake new file mode 100644 index 0000000000..0503c517ab --- /dev/null +++ b/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 + "$<$:DEBUG>;$<$: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 + "$<$:DEBUG>;$<$:TEST>" + ) + + add_executable(exe_first configtest.c) + target_compile_definitions(exe_first PRIVATE ${ARGN}) + target_link_libraries(exe_first PRIVATE lib_first) +endfunction() diff --git a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake index 0c68d391f1..db825b7be3 100644 --- a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake +++ b/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) diff --git a/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt b/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt index 46af24397c..6e94e10440 100644 --- a/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt +++ b/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt @@ -48,6 +48,7 @@ \* CMP0182 \* CMP0195 \* CMP0199 + \* CMP0200 Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\)