Remove global includes and use target include for Apps #6009
Conversation
larshg
changed the title
larshg
changed the title
larshg
force-pushed
the
updateincludesapps
branch
from
784a150 to
56499d3
Compare
|
Could you rebase this on master, please? |
larshg
force-pushed
the
updateincludesapps
branch
from
56499d3 to
7d8a618
Compare
apps/CMakeLists.txt
Outdated
| PCL_ADD_EXECUTABLE(pcl_test_search_speed COMPONENT ${SUBSYS_NAME} SOURCES src/test_search.cpp) | ||
| target_link_libraries(pcl_test_search_speed pcl_common pcl_io pcl_search pcl_kdtree pcl_visualization) | ||
|
|
||
| PCL_ADD_EXECUTABLE(pcl_nn_classification_example COMPONENT ${SUBSYS_NAME} SOURCES src/nn_classification_example.cpp) | ||
| target_link_libraries(pcl_nn_classification_example pcl_common pcl_io pcl_features pcl_kdtree) | ||
| target_link_libraries(pcl_nn_classification_example pcl_apps pcl_common pcl_io pcl_features pcl_kdtree) |
There was a problem hiding this comment.
I don't understand why this has to link to pcl_apps when it didn't have to before? (same for some more executables below)
There was a problem hiding this comment.
Because it includes pcl/apps/vfh_nn_classifier.h - which resides in apps "library".
There was a problem hiding this comment.
Line 21 the old include_directories made this available to all the executables afterwards.
There was a problem hiding this comment.
But vfh_nn_classifier.h seems to be a standalone header with every function fully defined, so no corresponding cpp file that would make linking to pcl_apps necessary. Wouldn't it be better to add target_include_directories(pcl_nn_classification_example ...) instead of unnecessarily adding pcl_apps as a dependency?
There was a problem hiding this comment.
Yeah, that should do it aswell of course. I'll change asap.
follows after #6008