Browse Source

GenEx: Fix TARGET_PROPERTY evaluation of transitive link properties

In commit bbba701899 (Link properties: must be transitive over private
dependency on static library, 2019-12-06, v3.17.0-rc1~323^2) and
commit af9d4f24ae (Link properties: must be transitive over private
dependency on static library, 2019-12-11, v3.17.0-rc1~305^2) we
neglected to implement CMP0099 NEW behavior for `TARGET_PROPERTY`
evaluation.  Add policy CMP0166 to fix this.
Brad King 1 year ago
parent
commit
ddb9442f48

+ 13 - 4
Help/manual/cmake-generator-expressions.7.rst

@@ -1282,7 +1282,7 @@ Compile Context
   .. versionadded:: 3.27
 
   Content of ``...``, when collecting
-  :ref:`usage requirements <Target Usage Requirements>`,
+  :ref:`transitive build properties <Transitive Build Properties>`,
   otherwise it is the empty string.  This is intended for use in an
   :prop_tgt:`INTERFACE_LINK_LIBRARIES` and :prop_tgt:`LINK_LIBRARIES` target
   properties, typically populated via the :command:`target_link_libraries` command.
@@ -1670,8 +1670,8 @@ Link Context
 
   .. versionadded:: 3.1
 
-  Content of ``...``, except while collecting
-  :ref:`usage requirements <Target Usage Requirements>`,
+  Content of ``...``, except while collecting usage requirements from
+  :ref:`transitive build properties <Transitive Build Properties>`,
   in which case it is the empty string.  This is intended for use in an
   :prop_tgt:`INTERFACE_LINK_LIBRARIES` target property, typically populated
   via the :command:`target_link_libraries` command, to specify private link
@@ -1788,7 +1788,16 @@ The expressions have special evaluation rules for some properties:
   of the value on the target itself with the values of the same properties on
   targets named by the target's :prop_tgt:`INTERFACE_LINK_LIBRARIES`.
   Evaluation is transitive over the closure of the target's
-  :prop_tgt:`INTERFACE_LINK_LIBRARIES`.
+  :prop_tgt:`INTERFACE_LINK_LIBRARIES`:
+
+  * For :ref:`Transitive Build Properties`, the transitive closure
+    *excludes* entries of :prop_tgt:`INTERFACE_LINK_LIBRARIES` guarded
+    by the :genex:`LINK_ONLY` generator expression.
+
+  * For :ref:`Transitive Link Properties`, the transitive closure is
+    *includes* entries of :prop_tgt:`INTERFACE_LINK_LIBRARIES` guarded
+    by the :genex:`LINK_ONLY` generator expression.
+    See policy :policy:`CMP0166`.
 
   Evaluation of :prop_tgt:`INTERFACE_LINK_LIBRARIES` itself is not transitive.
 

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

@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.30
 .. toctree::
    :maxdepth: 1
 
+   CMP0166: TARGET_PROPERTY evaluates link properties transitively over private dependencies of static libraries. </policy/CMP0166>
    CMP0165: enable_language() must not be called before project(). </policy/CMP0165>
    CMP0164: add_library() rejects SHARED libraries when not supported by the platform. </policy/CMP0164>
    CMP0163: The GENERATED source file property is now visible in all directories. </policy/CMP0163>

+ 6 - 0
Help/policy/CMP0099.rst

@@ -21,6 +21,12 @@ The ``OLD`` behavior for this policy is to not propagate interface link
 properties. The ``NEW`` behavior of this policy is to propagate interface link
 properties.
 
+.. versionadded:: 3.30
+
+  Policy :policy:`CMP0166` makes :genex:`TARGET_PROPERTY` evaluation of
+  these three transitive link properties follow private dependencies of
+  static libraries too.
+
 .. |INTRODUCED_IN_CMAKE_VERSION| replace:: 3.17
 .. |WARNS_OR_DOES_NOT_WARN| replace:: does *not* warn
 .. include:: STANDARD_ADVICE.txt

+ 40 - 0
Help/policy/CMP0166.rst

@@ -0,0 +1,40 @@
+CMP0166
+-------
+
+.. versionadded:: 3.30
+
+:genex:`TARGET_PROPERTY` evaluates link properties transitively over private
+dependencies of static libraries.
+
+In CMake 3.29 and below, the :genex:`TARGET_PROPERTY` generator expression
+evaluates properties :prop_tgt:`INTERFACE_LINK_OPTIONS`,
+:prop_tgt:`INTERFACE_LINK_DIRECTORIES`, and :prop_tgt:`INTERFACE_LINK_DEPENDS`
+as if they were :ref:`Transitive Build Properties` rather than
+:ref:`Transitive Link Properties`, even when policy :policy:`CMP0099` is
+set to ``NEW``.  Private dependencies of static libraries, which appear in
+their :prop_tgt:`INTERFACE_LINK_LIBRARIES` guarded by :genex:`LINK_ONLY`
+generator expressions, are not followed.  This is inconsistent with
+evaluation of the same target properties during buildsystem generation.
+
+CMake 3.30 and above prefer that :genex:`TARGET_PROPERTY` evaluates
+properties :prop_tgt:`INTERFACE_LINK_OPTIONS`,
+:prop_tgt:`INTERFACE_LINK_DIRECTORIES`, and :prop_tgt:`INTERFACE_LINK_DEPENDS`
+as :ref:`Transitive Link Properties` such that private dependencies of static
+libraries, which appear in their :prop_tgt:`INTERFACE_LINK_LIBRARIES` guarded
+by :genex:`LINK_ONLY` generator expressions, are followed.
+This policy provides compatibility for projects that have not been updated
+to expect the new behavior.
+
+The ``OLD`` behavior for this policy is for :genex:`TARGET_PROPERTY` to
+evaluate properties :prop_tgt:`INTERFACE_LINK_OPTIONS`,
+:prop_tgt:`INTERFACE_LINK_DIRECTORIES`, and :prop_tgt:`INTERFACE_LINK_DEPENDS`
+as if they were :ref:`Transitive Build Properties` by not following private
+dependencies of static libraries.  The ``NEW`` behavior for this policy is
+to evaluate them as :ref:`Transitive Link Properties` by following private
+dependencies of static libraries.
+
+.. |INTRODUCED_IN_CMAKE_VERSION| replace:: 3.30
+.. |WARNS_OR_DOES_NOT_WARN| replace:: does *not* warn
+.. include:: STANDARD_ADVICE.txt
+
+.. include:: DEPRECATED.txt

+ 8 - 0
Help/release/dev/genex-link-properties.rst

@@ -0,0 +1,8 @@
+genex-link-properties
+---------------------
+
+* The :genex:`TARGET_PROPERTY` generator expression now evaluates target
+  properties :prop_tgt:`INTERFACE_LINK_OPTIONS`,
+  :prop_tgt:`INTERFACE_LINK_DIRECTORIES`, and
+  :prop_tgt:`INTERFACE_LINK_DEPENDS` correctly by following private
+  dependencies of static libraries.  See policy :policy:`CMP0166`.

+ 12 - 3
Source/cmGeneratorTarget.cxx

@@ -89,11 +89,11 @@ const std::map<cm::string_view, TransitiveProperty>
     { "INCLUDE_DIRECTORIES"_s,
       { "INTERFACE_INCLUDE_DIRECTORIES"_s, LinkInterfaceFor::Usage } },
     { "LINK_DEPENDS"_s,
-      { "INTERFACE_LINK_DEPENDS"_s, LinkInterfaceFor::Usage } },
+      { "INTERFACE_LINK_DEPENDS"_s, LinkInterfaceFor::Link } },
     { "LINK_DIRECTORIES"_s,
-      { "INTERFACE_LINK_DIRECTORIES"_s, LinkInterfaceFor::Usage } },
+      { "INTERFACE_LINK_DIRECTORIES"_s, LinkInterfaceFor::Link } },
     { "LINK_OPTIONS"_s,
-      { "INTERFACE_LINK_OPTIONS"_s, LinkInterfaceFor::Usage } },
+      { "INTERFACE_LINK_OPTIONS"_s, LinkInterfaceFor::Link } },
     { "PRECOMPILE_HEADERS"_s,
       { "INTERFACE_PRECOMPILE_HEADERS"_s, LinkInterfaceFor::Usage } },
     { "SOURCES"_s, { "INTERFACE_SOURCES"_s, LinkInterfaceFor::Usage } },
@@ -1557,6 +1557,15 @@ cmGeneratorTarget::IsTransitiveProperty(cm::string_view prop,
   auto i = BuiltinTransitiveProperties.find(prop);
   if (i != BuiltinTransitiveProperties.end()) {
     result = i->second;
+    if (result->InterfaceFor != cmGeneratorTarget::LinkInterfaceFor::Usage) {
+      cmPolicies::PolicyStatus cmp0166 =
+        lg->GetPolicyStatus(cmPolicies::CMP0166);
+      if ((cmp0166 == cmPolicies::WARN || cmp0166 == cmPolicies::OLD) &&
+          (prop == "LINK_DIRECTORIES"_s || prop == "LINK_DEPENDS"_s ||
+           prop == "LINK_OPTIONS"_s)) {
+        result->InterfaceFor = cmGeneratorTarget::LinkInterfaceFor::Usage;
+      }
+    }
   } else if (cmHasLiteralPrefix(prop, "COMPILE_DEFINITIONS_")) {
     cmPolicies::PolicyStatus cmp0043 =
       lg->GetPolicyStatus(cmPolicies::CMP0043);

+ 5 - 1
Source/cmPolicies.h

@@ -508,7 +508,11 @@ class cmMakefile;
          3, 30, 0, cmPolicies::WARN)                                          \
   SELECT(POLICY, CMP0165,                                                     \
          "enable_language() must not be called before project().", 3, 30, 0,  \
-         cmPolicies::WARN)
+         cmPolicies::WARN)                                                    \
+  SELECT(POLICY, CMP0166,                                                     \
+         "TARGET_PROPERTY evaluates link properties transitively over "       \
+         "private dependencies of static libraries.",                         \
+         3, 30, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \

+ 1 - 1
Tests/RunCMake/GenEx-TARGET_PROPERTY/CMakeLists.txt

@@ -3,4 +3,4 @@ if(RunCMake_TEST STREQUAL "LOCATION")
   cmake_minimum_required(VERSION 2.8.12) # Leave CMP0026 unset.
 endif()
 project(${RunCMake_TEST} NONE)
-include(${RunCMake_TEST}.cmake)
+include(${RunCMake_TEST}.cmake NO_POLICY_SCOPE)

+ 2 - 1
Tests/RunCMake/GenEx-TARGET_PROPERTY/RunCMakeTest.cmake

@@ -14,7 +14,8 @@ run_cmake(LinkImplementationCycle6)
 run_cmake(LOCATION)
 run_cmake(SOURCES)
 run_cmake(TransitiveBuild)
-run_cmake(TransitiveLink)
+run_cmake(TransitiveLink-CMP0166-OLD)
+run_cmake(TransitiveLink-CMP0166-NEW)
 
 block()
   run_cmake(Scope)

+ 8 - 0
Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-NEW-check.cmake

@@ -0,0 +1,8 @@
+set(expect [[
+# file\(GENERATE\) produced:
+main LINK_LIBRARIES: 'foo1' # not transitive
+main LINK_DIRECTORIES: '[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/dirM;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/dir1;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/dir2'
+main LINK_OPTIONS: '-optM;-opt1;-opt2'
+main LINK_DEPENDS: '[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-NEW-build/depM;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-NEW-build/dep1;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-NEW-build/dep2'
+]])
+include(${CMAKE_CURRENT_LIST_DIR}/TransitiveLink-check-common.cmake)

+ 2 - 0
Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-NEW.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0166 NEW)
+include(TransitiveLink-common.cmake)

+ 8 - 0
Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-OLD-check.cmake

@@ -0,0 +1,8 @@
+set(expect [[
+# file\(GENERATE\) produced:
+main LINK_LIBRARIES: 'foo1' # not transitive
+main LINK_DIRECTORIES: '[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/dirM;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/dir1'
+main LINK_OPTIONS: '-optM;-opt1'
+main LINK_DEPENDS: '[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-OLD-build/depM;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-OLD-build/dep1'
+]])
+include(${CMAKE_CURRENT_LIST_DIR}/TransitiveLink-check-common.cmake)

+ 2 - 0
Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-CMP0166-OLD.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0166 OLD)
+include(TransitiveLink-common.cmake)

+ 0 - 9
Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-check.cmake → Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-check-common.cmake

@@ -1,12 +1,3 @@
-# FIXME: TARGET_PROPERTY evaluation does not pierce LINK_ONLY
-set(expect [[
-# file\(GENERATE\) produced:
-main LINK_LIBRARIES: 'foo1' # not transitive
-main LINK_DIRECTORIES: '[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/dirM;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/dir1'
-main LINK_OPTIONS: '-optM;-opt1'
-main LINK_DEPENDS: '[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-build/depM;[^';]*/Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-build/dep1'
-]])
-
 string(REGEX REPLACE "\r\n" "\n" expect "${expect}")
 string(REGEX REPLACE "\n+$" "" expect "${expect}")
 

+ 0 - 0
Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink.cmake → Tests/RunCMake/GenEx-TARGET_PROPERTY/TransitiveLink-common.cmake