CPM.cmake
CPM.cmake copied to clipboard
heads up: CMake 3.25 `SYSTEM` option for FetchContent
Here's the motivating issue in CMake's issue tracker. FetchContent uses ExternalProject_Add, which uses add_subdirectory, which is unlike importing via find_package in that the targets that get added will not be IMPORTED, and will therefore have their target_include_directories not marked as SYSTEM, which means compilers will check those third party headers to warn for things that the user only wanted warnings for for their own project's code (not for third party header code).
Here's the ticket tracking the solution that was proposed and is now being implemented. In summary,
- Add a SYSTEM property for targets
- ...
- As a helper based on the
SYSTEMproperty, implementadd_subdirectory(.. SYSTEM)andFetchContent_Declare(.. SYSTEM BOOL)
At the time of this writing, these changes are not merged in yet, and I'm not sure, but I don't think CMake 3.25 will be released until November, so it's still quite early to be mentioning this. I thought it would be nice to just put it on the radar so it doesn't go undetected.
I'm guessing there will be some policy variable that can be checked to see if the CPM user's CMake version supports these new arguments, and if so, to add a similar parameter/option to CPMAddPackage.
Thanks for the heads up! It looks like CMake's support for external libraries is slowly improving!
Perhaps it would be a good idea to add a document with CMake policies that impact CPM's behaviour and discuss how to deal with them. Another FetchContent related policy change is handled in #372 .
I asked about the appropriate way to detect whether the new feature exists for a particular CMake version in the CMake ticket (here). Here's a quote of Craig's answer:
If something relies on a feature that was introduced in a particular CMake version, then a conditional of the form
if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.xx")would be the simplest way to check for the availability of that feature. That will only be reliable once a feature makes its way into an official release though.
CMake 3.25 just released the SYSTEM property: https://cmake.org/cmake/help/v3.25/release/3.25.html#properties
Hello there Any status on this feature? It would be highly appreciated :)
@TheLartians we need this feature to prevent workarounds like this: https://github.com/ClausKlein/ModernCmakeStarter/pull/3
I think it might be enough to add the SYSTEM property to cpm_add_subdirectory, however I'm currently not sure if there are any implications we should think about before implementing this change.
however I'm currently not sure if there are any implications we should think about before implementing this change.
How about adding it as an opt-in parameter first and then survey users in your discussions page on whether they think it would make a good default?
I think it might be enough to add the
SYSTEMproperty to cpm_add_subdirectory, however I'm currently not sure if there are any implications we should think about before implementing this change.
NO, note about your ModernCppStarter: Developers what to see their Warnings while develop from all via test or standalone!
see i.e. https://github.com/cpm-cmake/testpack-fibonacci/pull/1
Fair point, I think for the normal syntax having SYSTEM as an opt-in parameter should be the safest approach. Perhaps it might make sense to enable it for the shorthand syntax though, similar to the way we currently enable EXCLUDE_FROM_ALL.