How to prevent included libraries from producing warnings?

[vittorioromeo]

If I have something like

CPMAddPackage(
    NAME quickjs
    GIT_REPOSITORY https://github.com/vittorioromeo/quickjs
    GIT_TAG 4eeb83a19b2772222f044655543337b563874d18
)

target_link_libraries(my_project PRIVATE quickjs)

then I will see warnings generated for quickjs's included headers, even though I am not the owner of that library. I would like to silence the warnings for the included headers (i.e. use them as system headers). I've tried

target_include_directories(my_project SYSTEM PRIVATE "f${quickjs_SOURCE_DIR}")

but it doesn't seem to work -- I still get warnings like before. How can I tell CPM to generate a target that includes its headers as system headers?

[iboB]

The warnings you're getting are from the build of the dependency itself. CPM.cmake adds the dependency's targets in your project, so it's built with it just as other targets (the ones you've created for example).

There is no way to influence the sources of the target with CPM.cmake

You can influence CMake options of the dependency. If they provide options for disabling warnings, then you can work with that

[Leon0402]

I'm not sure about what this is exactly, because there two things:

• The build of the dependencies target themself
• The build of your project including the dependencies target

I think the issue is about the latter. In normal CMake Code you can do something like this

get_property(catch2_include_dirs TARGET Catch2 PROPERTY INTERFACE_INCLUDE_DIRECTORIES)
set_property(TARGET Catch2 PROPERTY INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${catch2_include_dirs}")

An example for catch2 in this case

All header files will be treated as System Include headers now and the targets linking against the dependencies won't issue any warnings. (The build of Catch2 will issue some of course still ... but I think that's good).

Maybe this can be done via cpm? The user could provide the target (alias) names. With get_target_property(real_lib ${lib} ALIASED_TARGET) we could get the real library name and then execute the above code

[TheLartians]

If the library names are provided CPM could call execute the code, however imo this isn't exactly related to dependency management, as imo it's up to the preferences of the library authors / users and, as you've noted, can be easily changed by either. Personally, I prefer seeing the warnings as they still happen within my code due to the way C++ includes work.

[anders-wind]

Maybe we could have an option?

CPMAddPackage(
  NAME benchmark
  ...
  MARK_HEADERS_AS_SYSTEM ON
)

[Leon0402]

If the library names are provided CPM could call execute the code, however imo this isn't exactly related to dependency management, as imo it's up to the preferences of the library authors / users and, as you've noted, can be easily changed by either. Personally, I prefer seeing the warnings as they still happen within my code due to the way C++ includes work.

Actually CPM doesn't even need to know the targets. I got some code from here for querying targets: https://stackoverflow.com/a/62311397/10764260

So for my purpose my script looks like this:

function(get_all_targets var)
    set(targets)
    get_all_targets_recursive(targets ${CMAKE_CURRENT_SOURCE_DIR})
    set(${var} ${targets} PARENT_SCOPE)
endfunction()

macro(get_all_targets_recursive targets dir)
    get_property(subdirectories DIRECTORY ${dir} PROPERTY SUBDIRECTORIES)
    foreach(subdir ${subdirectories})
        get_all_targets_recursive(${targets} ${subdir})
    endforeach()

    get_property(current_targets DIRECTORY ${dir} PROPERTY BUILDSYSTEM_TARGETS)
    list(APPEND ${targets} ${current_targets})
endmacro()

function(make_all_targets_system)
    get_all_targets(all_targets)

    foreach(target ${all_targets})
        get_property(${target}_include_dirs TARGET ${target} PROPERTY INTERFACE_INCLUDE_DIRECTORIES)
        set_property(TARGET ${target} PROPERTY INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${${target}_include_dirs})
    endforeach()
endfunction()

CPM could, to be as flexible as possible, calculate before each call all existing targets and after the addPackage call, so it can find out new added targets.

I agree that it's something the user could do (although that could be said about CPM as well :-) After all it's a wrapper around FetchContent), but I wouldn't call it trivial to do or "beautiful" code. I would very much appreciate to not have such code in my end application cmake, but rather some nicer interface. Making it an option of CPM would be the most convenient way for end users.

I see how people can be of different opinions whether making libraries system libraries or not, but to name two arguments for adding this feature:

• Based on my experience a lot of people don't want warnings for libraries, so it's worth it to add an option for those
• Considering that almost all libraries added via find_package are system libraries, it seems kinda consistent to me to have the same behavior for FetchContent too. After all, you have no control over the libraries, so the warnings are of very little use in my experience. And in my projects with quite a few libraries, it's just to much warnings and hide the real warnings: Those that affect my code and those I can actually fix.

TBH I would actually even more like to see such a feature in FetchContent directly. But as you write in the readme: CPM is a good place to start with such features as they can land more easily in CPM and more people can profit from it.

In an ideal world, there would be a standard cmake variable for controlling this behavior that all libraries implement. But I think it's good to be realistic: A lot of libraries struggle with cmake and we should keep the requirements on these libraries low and implement as much as possible in CPM / cmake directly.

[iboB]

I am not against something in the spirit of @anders-wind 's proposal.

The name of the option should be MARK_INCLUDE_DIRS_AS_SYSTEM or something similar which indicates that we're talking about directories.

However I am against making it a default-on option. Disabling warnings should be a conscious decision done only after one has determined that fixing them is not desirable or possible.

[anders-wind]

Just having the option would be nice to me :)

[vittorioromeo]

I confirm that a MARK_INCLUDE_DIRS_AS_SYSTEM opt-in flag would be a very pragmatic solution that would 100% solve my original issue.

[TheLartians]

Great points made here, I think an opt-in flag using a script like @Leon0402 has provided may be the way to go.

[Leon0402]

Maybe it would be good to have a global variable additionally to control the behavior. I assume that in most cases you would either want all warning for all dependencies disabled or for none. So that would make the code a little bit easier for consumers.

[vittorioromeo]

@TheLartians: have you considered adding the flag? I just re-encountered this problem, googled for a solution, and realized I posted this issue in the past myself... :D

[TheLartians]

It's definitely a feature that we would like to add. Unfortunately I currently don't have the time to do so myself, but would be happy to review a PR adding it. 😉