Commit 07ebc586 authored by Harald Sitter's avatar Harald Sitter 💣

Remove /KF5 include directory injection

This is ancient code that is outright wrong most of the time and at best
just incredibly unnecessary.
It is also not present in the great majority of frameworks due to this.

Its wrongness comes from the fact that it hardcodes the installation path,
which breaks relocatability of the KF5 tree as it will always attempt to
find the include dir $PREFIX/KF5 (e.g. /usr/include/KF5), which may or may
not exist given that the tree was relocated.
Worse yet, in a cross-building scenario we maybe for example
build on ARM and install to /usr but for cross building take the entire ARM
tree and shift it into /arm/usr/. If we then crossbuild on that tree the
bogus include list in this framework will make sure that we always search
in /usr/include/KF5 and thus potentially load a !ARM header simply because
the relevant ARM header was not installed etc.. Similarly of course a
build in $HOME can pick up /usr/include/KF5 headers because the home ones
are missing, causing unexpected results.
This happens whenever the KDE_INSTALL_INCLUDEDIR_KF5 var is absolute, which
it usually is.

On top of all that the premise of the code in question is flawed. It seeks
to add $PREFIX/$KF5INCLUDES to the search paths (e.g. /usr/include/KF5).
This is unnecessary because the target itself is properly installed via
cmake's install(TARGETS ... EXPORT ...) function [1]. This function has
smart functionality built in which will add the passed INCLUDES destination
to the INTERFACE_INCLUDE_DIRECTORIES property of the targets (i.e. what
the useless code wants to do) [2].
So what happens is that we install the target to the
KF5 locations, which has "include/KF5" as INCLUDES location,
thus causing the correct path to be added to the includes list of the
Targets.cmake file.
In particular thanks to more internal magic in cmake it will do so with
automatically resolved root paths such that the installed tree is
relocatable and able to relatively find the other KF5/* headers. So it
does what the code in question wants to do, just correctly.

Since cmake automatically takes care of injecting $prefix/include/KF5 we
can simply get rid of the wrong custom inejection code. This makes the
generated cmake file find the correct include/KF5/ directory and stops it
from always expecting a /usr/include/KF5/ directory to be present.

[1] https://cmake.org/cmake/help/v3.0/command/install.html
[2] https://cmake.org/cmake/help/v3.0/command/install.html
> The INCLUDES DESTINATION specifies a list of directories which will be
> added to the INTERFACE_INCLUDE_DIRECTORIES target property of the
> <targets> when exported by the install(EXPORT) command.

REVIEW: 129273
CHANGELOG: Improved relocatability of CMake export
parent fa167587
...@@ -146,12 +146,6 @@ target_link_libraries(KF5WaylandClient ...@@ -146,12 +146,6 @@ target_link_libraries(KF5WaylandClient
Qt5::Concurrent Qt5::Concurrent
) )
if(IS_ABSOLUTE "${KF5_INCLUDE_INSTALL_DIR}")
target_include_directories(KF5WaylandClient INTERFACE "$<INSTALL_INTERFACE:${KF5_INCLUDE_INSTALL_DIR}>" )
else()
target_include_directories(KF5WaylandClient INTERFACE "$<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/${KF5_INCLUDE_INSTALL_DIR}>" )
endif()
set_target_properties(KF5WaylandClient PROPERTIES VERSION ${KWAYLAND_VERSION_STRING} set_target_properties(KF5WaylandClient PROPERTIES VERSION ${KWAYLAND_VERSION_STRING}
SOVERSION ${KWAYLAND_SOVERSION} SOVERSION ${KWAYLAND_SOVERSION}
EXPORT_NAME WaylandClient EXPORT_NAME WaylandClient
......
...@@ -148,12 +148,6 @@ target_link_libraries(KF5WaylandServer ...@@ -148,12 +148,6 @@ target_link_libraries(KF5WaylandServer
Qt5::Concurrent Qt5::Concurrent
) )
if(IS_ABSOLUTE "${KF5_INCLUDE_INSTALL_DIR}")
target_include_directories(KF5WaylandServer INTERFACE "$<INSTALL_INTERFACE:${KF5_INCLUDE_INSTALL_DIR}>" )
else()
target_include_directories(KF5WaylandServer INTERFACE "$<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/${KF5_INCLUDE_INSTALL_DIR}>" )
endif()
set_target_properties(KF5WaylandServer PROPERTIES VERSION ${KWAYLAND_VERSION_STRING} set_target_properties(KF5WaylandServer PROPERTIES VERSION ${KWAYLAND_VERSION_STRING}
SOVERSION ${KWAYLAND_SOVERSION} SOVERSION ${KWAYLAND_SOVERSION}
EXPORT_NAME WaylandServer EXPORT_NAME WaylandServer
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment