Add per-target property that toggles whether include paths are system header paths.
Summary
CMake's IMPORTED targets always add their include paths as system includes (-isystem
instead of -I
). This prevents packages that are tightly integrated with a toolchain from being able to provide imported targets that will work properly when a non-standard version is added to a project with find_package
.
Adding a per-target property that toggles the system-vs-nonsystem inclusion of headers would address this issue, and would be generally useful even for non-IMPORTED targets by allowing users to control the include order.
Motivation
The Thrust project is shipped with the CUDA compiler and is also provided as a stand-alone open-source repository.
When packaged with the CUDA ToolKit, the Thrust headers are in ${CTK_PATH}/include/thrust
. When the CUDA compiler is invoked, it automatically injects ${CTK_PATH}/include
in-between the -I
and -isystem
paths. Since IMPORTED targets' include paths are always treated as system headers, any IMPORTED targets we provide will be hidden by the implicit ${CTK_PATH}/include/thrust
include directory.
Workarounds
Currently, Thrust's thrust-config.cmake
file must jump through hoops to provide a namespaced target that will override the version of Thrust installed to ${CTK_PATH}
. See https://github.com/NVIDIA/thrust/blob/main/thrust/cmake/thrust-config.cmake#L363-L379:
function(_thrust_declare_interface_alias alias_name ugly_name)
# 1) Only IMPORTED and ALIAS targets can be placed in a namespace.
# 2) When an IMPORTED library is linked to another target, its include
# directories are treated as SYSTEM includes.
# 3) nvcc will automatically check the CUDA Toolkit include path *before* the
# system includes. This means that the Toolkit Thrust will *always* be used
# during compilation, and the include paths of an IMPORTED Thrust::Thrust
# target will never have any effect.
# 4) This behavior can be fixed by setting the property NO_SYSTEM_FROM_IMPORTED
# on EVERY target that links to Thrust::Thrust. This would be a burden and a
# footgun for our users. Forgetting this would silently pull in the wrong thrust!
# 5) A workaround is to make a non-IMPORTED library outside of the namespace,
# configure it, and then ALIAS it into the namespace (or ALIAS and then
# configure, that seems to work too).
add_library(${ugly_name} INTERFACE)
add_library(${alias_name} ALIAS ${ugly_name})
endfunction()
If Thrust were able to declare an IMPORTED target directly and change (a hypothetical) property to force non-system inclusion, this would be far simpler and less of a hacky fix than what we're currently forced to do.
The current property NO_SYSTEM_FROM_IMPORTED
is fragile, non-obvious, and a burden for users, as described above. We can't expect our users to set this flag on all targets that use Thrust, as it is error-prone and may break the inclusion of other dependencies.