[vtk-developers] Should VTK targets have INTERFACE_{INCLUDE_DIRECTORIES, COMPILE_DEFINITIONS} set?

[Elvis Stansvik]

2017-05-28 17:30 GMT+02:00 Elvis Stansvik <elvis.stansvik at orexplore.com>:
> Hi all,
>
> When linking against a VTK module through an imported target, e.g
>
>   find_package(VTK 8.0.0 REQUIRED COMPONENTS vtkCommonCore)
>   ...
>   target_link_libraries(myapp vtkCommonCore)
>
> I must still manually make sure I use
>
>   target_include_directories(myapp PUBLIC ${VTK_INCLUDE_DIRS})
>   ...
>   target_compile_definitions(myapp PUBLIC ${VTK_DEFINITIONS})
>
> to get the include directories and definitions.
>
> This is in contrast to e.g. Qt's CMake files, which makes sure targets
> have INTERFACE_INCLUDE_DIRECTORIES and INTERFACE_COMPILE_DEFINITIONS
> set on its targets. For example, QtSvg (from Qt5SvgConfig.cmake):
>
>     set_property(TARGET Qt5::Svg PROPERTY
>       INTERFACE_INCLUDE_DIRECTORIES ${_Qt5Svg_OWN_INCLUDE_DIRS})
>     set_property(TARGET Qt5::Svg PROPERTY
>       INTERFACE_COMPILE_DEFINITIONS QT_SVG_LIB)
>
> Now, this is not really a bit inconvenience, I just have to make sure
> to add ${VTK_INCLUDE_DIRS} and ${VTK_DEFINITIONS}.
>
> However, I recently started using the libclang-based
> include-what-you-use tool [1] to help sanitize my header inclusions.
> The tool will make suggestions such as (taking a Qt header as example
> here):
>
> /home/estan/orexplore/insight/src/model/HoleLoader.cpp should add these lines:
> #include <QFileInfo> // for QFileInfo
>
> I noticed that for a VTK header, it'll instead suggest e.g:
>
> /home/estan/orexplore/insight/src/model/Hole.cpp should add these lines:
> #include "vtkSmartPointer.h" // for vtkSmartPointer
>
> Notice the "" instead of <>.
>
> I dug into include-what-you-use to find out why this is happening, and
> it's because on the compile line, the Qt include directories are given
> with -isystem, while the VTK include directories are given with -I,
> and -isystem vs -I is what the tool uses (for lack of other
> information) to determine whether to suggest the inclusion with <> or
> with "".
>
> I believe that maybe the -I flags added for VTK would become -isystem,
> if they were brought in through the INTERFACE_INCLUDE_DIRECTORIES
> mechanism, same as Qt.
>
> So all this lead me to two questions regarding VTK:
>
> 1. Should VTK's installed CMake files perhaps make sure
> INTERFACE_INCLUDE_DIRECTORIES and INTERFACE_INCLUDE_DEFINITIONS are
> set on its exported targets?
>
> 2. If not, anyone know if there's something that could be done to make
> the flags end up as -isystem flags instead of -I when using
> ${VTK_INCLUDE_DIRS}?

Disregard this question: I realized that I could simply use SYSTEM in
my target_include_directories(...) to make CMake treat it as a system
include directory, which makes it use -isystem on compilers that
support it.

So then it's only question 1 left. I think it would be nice to have
INTERFACE_{INCLUDE_DIRECTORIES,DEFINITIONS} set on the targets, just
to avoid having to add ${VTK_INCLUDE_DIRS} and ${VTK_DEFINITIONS}
manually.

Elvis

>
> Cheers,
> Elvis
>
> [1] https://github.com/include-what-you-use/include-what-you-use