Remove global includes and use target include for all "common" modules. #6008
Conversation
| @@ -29,8 +29,6 @@ set(impl_incs | |||
| "include/pcl/${SUBSYS_NAME}/impl/morphology.hpp" | |||
| ) | |||
|
|
|||
| include_directories("${CMAKE_CURRENT_SOURCE_DIR}/include") | |||
There was a problem hiding this comment.
This module does not use PCL_ADD_LIBRARY, neither does geometry. Can this line still be removed?
There was a problem hiding this comment.
It did work due to the include_directories from within PCL_SUBSYS_DEPEND - I have removed it now and changed to use interface libraries for the header only modules.
There was a problem hiding this comment.
I removed the last changes, as it seemlingly only work like this, with the GPU and CUDA submodules. I couldn't "fix" it by adding include_directories in their respective submodules, which was propergated correctly to the other dependencies.
So my reasoning is to keep the include_directories in PCL_SUBSYS_DEPEND, until the latest PR which removes these and fix the remaining issues that aren't present until they are actually removed.
Want me to split the PRs even further or can we merge these 4 step by step?
larshg
force-pushed
the
updateincludes
branch
4 times, most recently
from
90e2b20 to
abeac82
Compare
|
|
||
| add_library(${LIB_NAME} INTERFACE) | ||
| target_include_directories(${LIB_NAME} INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/include") |
There was a problem hiding this comment.
Would it make sense to keep this line or replace it with target_include_directories(${LIB_NAME} INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> $<INSTALL_INTERFACE:${INCLUDE_INSTALL_ROOT}>) (at least temporarily for the next release)? If I understand correctly, removing this line would create the problem for geometry that #6006 tries to fix for kdtree and surface.
There was a problem hiding this comment.
I could also add the support for interface/header libraries in this PR?
But still keep the include_directories in PCL_SUBSYS_DEPEND.
Surpasses #6006 - sorry @Clemoot - but I thought about writing what you could change additionally, but realized it became a pretty large task.
I have more incoming after this PR.