From 41a6b4a53ba844ef986b0bc4efe8938b97eea810 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 22 Mar 2022 09:49:11 -0400 Subject: INTERFACE_LINK_LIBRARIES_DIRECT: Honor link dependencies through LINK_ONLY In commit f3ad061858 (Add usage requirements to update direct link dependencies, 2022-01-12, v3.23.0-rc1~44^2), we evaluated the transitive closure of `INTERFACE_LINK_LIBRARIES` as a non-linking usage requirement. That left out `INTERFACE_LINK_LIBRARIES_DIRECT` link dependencies that appear behind private dependencies of a static library, guarded by the `$` generator expression. At the time, that decision was intentional, in order to prevent arbitrary usage requirements from leaking out of `PRIVATE` dependencies. Since then, we've revised evaluation of `LINK_LIBRARIES` to distinguish between collecting link dependencies and other usage requirements. Use that information when following `INTERFACE_LINK_LIBRARIES` to collect the matching kind of requirements from `INTERFACE_LINK_LIBRARIES_DIRECT`. Fixes: #22496 --- Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst | 35 +++++++++++++++------- Source/cmGeneratorTarget.cxx | 14 +++++---- Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt | 9 ++++-- .../exe_use_static_A_private.c | 8 ++--- .../exe_use_static_A_public_explicit.c | 23 ++++++++++++++ 5 files changed, 66 insertions(+), 23 deletions(-) create mode 100644 Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c diff --git a/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst b/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst index aa8cc2e..b8b73df 100644 --- a/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst +++ b/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst @@ -56,11 +56,27 @@ on ``X``'s dependents: target_link_libraries(X PUBLIC Y) then ``Y`` is placed in ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES`, - so ``Y``'s usage requirements, including ``INTERFACE_PROPERTY_LINK_DIRECT`` - and ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, are propagated - to ``X``'s dependents. + so ``Y``'s usage requirements, including ``INTERFACE_PROPERTY_LINK_DIRECT``, + ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, and the usage requirements + declared by the direct link dependencies they add, are propagated to + ``X``'s dependents. -* If ``X`` links ``Y`` privately: +* If ``X`` is a static library or object library, and links ``Y`` privately: + + .. code-block:: cmake + + target_link_libraries(X PRIVATE Y) + + then ``$`` is placed in ``X``'s + :prop_tgt:`INTERFACE_LINK_LIBRARIES`. ``Y``'s linking requirements, + including ``INTERFACE_PROPERTY_LINK_DIRECT``, + ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, and the transitive link + dependencies declared by the direct link dependencies they add, are + propagated to ``X``'s dependents. However, ``Y``'s non-linking + usage requirements are blocked by the :genex:`LINK_ONLY` generator + expression, and are not propagated to ``X``'s dependents. + +* If ``X`` is a shared library or executable, and links ``Y`` privately: .. code-block:: cmake @@ -68,13 +84,10 @@ on ``X``'s dependents: then ``Y`` is not placed in ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES`, so ``Y``'s usage requirements, even ``INTERFACE_PROPERTY_LINK_DIRECT`` - and ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, are not propagated - to ``X``'s dependents. - (If ``X`` is a static library or object library, then ``$`` - is placed in ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES`, but the - :genex:`LINK_ONLY` generator expression block ``Y``'s usage requirements.) + and ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, are not propagated to + ``X``'s dependents. -* In either case, the content of ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES` +* In all cases, the content of ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES` is not affected by ``Y``'s ``INTERFACE_PROPERTY_LINK_DIRECT`` or ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``. @@ -184,7 +197,7 @@ be an intermediate library: .. code-block:: cmake add_library(app_impl STATIC app_impl.cpp) - target_link_libraries(app_impl PUBLIC Foo) + target_link_libraries(app_impl PRIVATE Foo) add_executable(app main.cpp) target_link_libraries(app PRIVATE app_impl) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 17a521a..6c804b5 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -8035,6 +8035,7 @@ class TransitiveLinkImpl { cmGeneratorTarget const* Self; std::string const& Config; + LinkInterfaceFor ImplFor; cmLinkImplementation& Impl; std::set Emitted; @@ -8045,9 +8046,10 @@ class TransitiveLinkImpl public: TransitiveLinkImpl(cmGeneratorTarget const* self, std::string const& config, - cmLinkImplementation& impl) + LinkInterfaceFor implFor, cmLinkImplementation& impl) : Self(self) , Config(config) + , ImplFor(implFor) , Impl(impl) { } @@ -8064,8 +8066,8 @@ void TransitiveLinkImpl::Follow(cmGeneratorTarget const* target) } // Get this target's usage requirements. - cmLinkInterfaceLibraries const* iface = target->GetLinkInterfaceLibraries( - this->Config, this->Self, LinkInterfaceFor::Usage); + cmLinkInterfaceLibraries const* iface = + target->GetLinkInterfaceLibraries(this->Config, this->Self, this->ImplFor); if (!iface) { return; } @@ -8129,9 +8131,10 @@ void TransitiveLinkImpl::Compute() void ComputeLinkImplTransitive(cmGeneratorTarget const* self, std::string const& config, + LinkInterfaceFor implFor, cmLinkImplementation& impl) { - TransitiveLinkImpl transitiveLinkImpl(self, config, impl); + TransitiveLinkImpl transitiveLinkImpl(self, config, implFor, impl); transitiveLinkImpl.Compute(); } } @@ -8140,7 +8143,6 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries( const std::string& config, cmOptionalLinkImplementation& impl, cmGeneratorTarget const* head, LinkInterfaceFor implFor) const { - static_cast(implFor); cmLocalGenerator const* lg = this->LocalGenerator; cmMakefile const* mf = lg->GetMakefile(); cmBTStringRange entryRange = this->Target->GetLinkImplementationEntries(); @@ -8245,7 +8247,7 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries( // Update the list of direct link dependencies from usage requirements. if (head == this) { - ComputeLinkImplTransitive(this, config, impl); + ComputeLinkImplTransitive(this, config, implFor, impl); } // Get the list of configurations considered to be DEBUG. diff --git a/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt b/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt index b06a2fb..dec131d 100644 --- a/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt +++ b/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt @@ -52,7 +52,7 @@ set_property(TARGET A APPEND PROPERTY INTERFACE_LINK_LIBRARIES_DIRECT add_library(static_A_public STATIC static_A_public.c) target_link_libraries(static_A_public PUBLIC A) -# Uses A's usage requirements, but does not propagate them. +# Uses A's usage requirements, but propagates only the linking requirements. # Does not use the exe-only usage requirement. Does use the optional one. add_library(static_A_private STATIC static_A_private.c) target_link_libraries(static_A_private PRIVATE A) @@ -63,10 +63,15 @@ add_executable(exe_use_static_A_public exe_use_static_A_public.c) target_link_libraries(exe_use_static_A_public PRIVATE static_A_public) set_property(TARGET exe_use_static_A_public PROPERTY A_LINK_OPTIONAL 1) -# Does not use A's usage requirements. +# Does not use A's usage requirements, but does use its non-optional linking requirements. add_executable(exe_use_static_A_private exe_use_static_A_private.c) target_link_libraries(exe_use_static_A_private PRIVATE static_A_private) +# Uses A's usage requirements, including an optional one, but overrides the link ordering. +add_executable(exe_use_static_A_public_explicit exe_use_static_A_public_explicit.c) +target_link_libraries(exe_use_static_A_public_explicit PRIVATE static_A_public A direct_from_A direct_from_A_for_exe direct_from_A_optional) +set_property(TARGET exe_use_static_A_public_explicit PROPERTY A_LINK_OPTIONAL 1) + #---------------------------------------------------------------------------- # Test how original and injected dependencies get ordered. diff --git a/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c index 024e96e..12cf309 100644 --- a/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c +++ b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c @@ -9,15 +9,15 @@ #endif extern void static_A_private(void); -extern void not_direct_from_A(void); -extern void not_direct_from_A_for_exe(void); +extern void direct_from_A(void); +extern void direct_from_A_for_exe(void); extern void not_direct_from_A_optional(void); int main(void) { static_A_private(); - not_direct_from_A(); - not_direct_from_A_for_exe(); + direct_from_A(); + direct_from_A_for_exe(); not_direct_from_A_optional(); return 0; } diff --git a/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c new file mode 100644 index 0000000..f698a24 --- /dev/null +++ b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c @@ -0,0 +1,23 @@ +#ifndef DEF_DIRECT_FROM_A +# error "DEF_DIRECT_FROM_A incorrectly not defined" +#endif +#ifndef DEF_DIRECT_FROM_A_FOR_EXE +# error "DEF_DIRECT_FROM_A_FOR_EXE incorrectly not defined" +#endif +#ifndef DEF_DIRECT_FROM_A_OPTIONAL +# error "DEF_DIRECT_FROM_A_OPTIONAL incorrectly not defined" +#endif + +extern void static_A_public(void); +extern void not_direct_from_A(void); +extern void not_direct_from_A_for_exe(void); +extern void not_direct_from_A_optional(void); + +int main(void) +{ + static_A_public(); + not_direct_from_A(); + not_direct_from_A_for_exe(); + not_direct_from_A_optional(); + return 0; +} -- cgit v0.12