Conan: Consider using isystem for headers in dependencies via cmake targets.

[memsharded]

Right now the cmake from conanbuildinfo.cmake uses:

    set_property(TARGET {name} PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${{CONAN_INCLUDE_DIRS_{uname}}}
                                                                      $<$<CONFIG:Release>:${{CONAN_INCLUDE_DIRS_{uname}_RELEASE}}>
                                                                      $<$<CONFIG:RelWithDebInfo>:${{CONAN_INCLUDE_DIRS_{uname}_RELEASE}}>

which doesn't allow to set SYSTEM. But maybe it could change to target_include_directories.

Use the CONAN_SYSTEM_INCLUDES.

[grafikrobot]

Generally using -isystem should be reserved for the standard library use only. This came up in Boost about a decade ago when we use to use -isystem by default. But doing so caused some issues in ordering and parsing. So I would recommend never to use -isystem.

[db4]

@memsharded, you can use INTERFACE_SYSTEM_INCLUDE_DIRECTORIES property instead of INTERFACE_INCLUDE_DIRECTORIES (if you decide to go this way)

[memsharded]

The feedback from @grafikrobot is quite revealing. Do we still want this feature?

[db4]

I can do without it.

[ArekPiekarz]

After encouragement from drodri on Conan's Slack, I'm adding an example what happens when you don't treat library headers as system ones, in this case using Boost.Signals2.

Starting at the end result, I grouped and counted all warnings from compilation of "Hello World" using signals.

$ cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug ..
$ ninja > ninja.output.txt
$ wc --lines ninja-output.txt 
10629 ninja-output.txt
$ cat ninja-output.txt | ag --only-matching \[-W[a-z0-9-=]+?]$ | sort | uniq --count
      1 [-Wconversion]
      3 [-Wctor-dtor-privacy]
     33 [-Wold-style-cast]
      1 [-Wshadow]
     52 [-Wsuggest-override]
      4 [-Wswitch-enum]
     12 [-Wundef]
    314 [-Wzero-as-null-pointer-constant]

That's 10 thousand lines with 420 warnings from a simple program. I prefer to add -Werror to treat warnings as errors, so it's a serious issue. There are no warnings if set(CONAN_SYSTEM_INCLUDES ON) is added to CMakeLists.txt.

Note that since 2012 you cannot silence these warnings in GCC with pragma diagnostic ignore around library headers due to this bug - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431.

Right now I'm not using CMake Targets, but when I switch to them I would like to keep my projects warning-free.

Reference

conanfile.txt

[requires]
boost_signals2/1.66.0@bincrafters/testing

[options]
*:shared=True
python_dev_config:python=python3
boost_python:python_version=3.6

[generators]
cmake

[imports]
bin, *.dll -> ./bin
bin, *.so -> ./bin
., *.dll -> ./bin @ keep_path=False, root_package=boost_*
., *.so -> ./bin @ keep_path=False, root_package=boost_*

CMakeLists.txt

project(conan-boost-warnings-example)
cmake_minimum_required(VERSION 3.11)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
# set(CONAN_SYSTEM_INCLUDES ON)

add_compile_options(
    #-Werror
    -Wpedantic
    -Wall
    -Wextra
    -Wchkp
    -Wdouble-promotion
    -Wformat=2
    -Wformat-overflow=2
    -Wformat-signedness
    -Wformat-truncation=2
    -Wnull-dereference
    -Wimplicit-fallthrough=5
    -Wmissing-include-dirs
    -Wshift-overflow=2
    -Wswitch-enum
    -Wsync-nand
    -Wunused-parameter
    -Wunused-const-variable=1
    -Wunknown-pragmas
    -Wstringop-overflow=4
    -Wsuggest-override
    -Walloc-zero
    -Walloca
    -Warray-bounds=2
    -Wduplicated-branches
    -Wduplicated-cond
    -Wtrampolines
    -Wfloat-equal
    -Wshadow
    -Waligned-new=all
    -Wplacement-new=2
    -Wundef
    -Wunused-macros
    -Wcast-qual
    -Wcast-align
    -Wconditionally-supported
    -Wconversion
    -Wzero-as-null-pointer-constant
    -Wdate-time
    -Wuseless-cast
    -Wsign-conversion
    -Wsized-deallocation
    -Wlogical-op
    -Wmissing-declarations
    -Wnormalized=id
    -Wpacked
    -Wredundant-decls
    -Wrestrict
    -Winvalid-pch
    -Wvector-operation-performance
    -Wdisabled-optimization
    -Wstack-protector
# Additional warnings for C++ dialect
    -Wctor-dtor-privacy
    -Wnoexcept
    -Wnon-virtual-dtor
    -Wstrict-null-sentinel
    -Wold-style-cast
    -Woverloaded-virtual
    -Wsign-promo
)

# Setup CMake-Conan wrapper
if(NOT EXISTS "${CMAKE_BINARY_DIR}/conan.cmake")
   message(STATUS "Downloading conan.cmake from https://github.com/conan-io/cmake-conan")
   file(DOWNLOAD "https://raw.githubusercontent.com/conan-io/cmake-conan/v0.10/conan.cmake"
                 "${CMAKE_BINARY_DIR}/conan.cmake")
endif()
include(${CMAKE_BINARY_DIR}/conan.cmake)
conan_cmake_run(
    CONANFILE conanfile.txt
    BASIC_SETUP 
    BUILD missing)

add_executable(${PROJECT_NAME} main.cpp)
target_link_libraries(${PROJECT_NAME} ${CONAN_LIBS})

main.cpp

#include <boost/signals2.hpp>
#include <iostream>

struct HelloWorld
{
    void operator()() const
    {
        std::cout << "Hello, World!" << std::endl;
    }
};

int main()
{
    // Signal with no arguments and a void return value
    boost::signals2::signal<void ()> sig;

    // Connect a HelloWorld slot
    HelloWorld hello;
    sig.connect(hello);

    // Call all of the slots
    sig();

    return 0;
}

Conan profile:

[settings]
os=Linux
os_build=Linux
arch=x86_64
arch_build=x86_64
build_type=Debug
compiler=gcc
compiler.libcxx=libstdc++11
compiler.version=7
[options]
*:shared=True
[build_requires]
[env]

Environment
Conan version: 1.1.1
OS: Ubuntu 17.10
Compiler: GCC 7.2
Build system: CMake 3.11rc4, Ninja 1.7.2
(everything x64 bit)

[memsharded]

Thanks @ArekPiekarz for the feedback.

This sounds like a clear extra vote for using CONAN_SYSTEM_INCLUDES for the cmake targets approach too. As it would be opt-in, it is fine to add this behavior, shouldn't be difficult.

[blastrock]

Continuing our conversation from #1318 .

Sorry if I am missing something here, but are you all saying that conan currently doesn't use -isystem and you would like it to? I am observing the opposite behavior: conan uses -isystem and I don't want it to.

If you use conan's imported target, cmake will put -isystem, and this is because cmake treats imported targets as system includes by default, as explained in https://cmake.org/cmake/help/v3.8/prop_tgt/NO_SYSTEM_FROM_IMPORTED.html .

Whatever conclusion you reach here (making it opt-in or opt-out), I would like to configure such a behavior from a toolchain recipe (through an env variable or whatever) since I am writing a toolchain for a broken crosscompiler (see #269 for my specific issue). CONAN_SYSTEM_INCLUDES is a variable that must be set in the CMakeLists of each project IIUC, so I would need something else.

[memsharded]

Oh, then, I was mistaken, this about NO_SYSTEM_FROM_IMPORTED is very clarifying.

The problem with trying to set things to cmake from dependencies, is that, contrary to autotools or other build systems, cmake won't use environment variables (besides CC/CXX, etc), so it is not possible to do it transparently.

We would need to consider other ways, like having some set of cmake variables that can be passed by the CMake helper while invoking cmake or something like that.

[kepam]

Why not just use system as parameter to .requires like private or override?
Actually I always wondered why build_requires have separate field instead of just parameter build in requires.

[memsharded]

I don't think isystem is a characteristic of the dependency graph. It only affects the final consumer, I see it more like a compiler flag. It might not even be a thing for some build systems and compilers.

"private" and "override" are strong characteristics of a generic dependency graph. They are concepts that apply to dependency graphs for other languages and affect how the graph is computed. "build_requires" are also special edges in the dependency graph, as they are not retrieved at all if it is not necessary to build from sources, and they define their own isolated dependency sub-graph.

[Sebiee]

Right now, if I use the target approach, I can't include my dependencies headers without the flag -isystem. The variable CONAN_SYSTEM_INCLUDES has no effect. The only thing I can do is:

set(CMAKE_NO_SYSTEM_FROM_IMPORTED True)
    include( ${CMAKE_BINARY_DIR}/conanbuildinfo.cmake )
    CONAN_BASIC_SETUP(TARGETS)

But that will make all my dependencies be included without isystem. I do want some to be treated like system deps...

[memsharded]

@Sebiee Yes, it is true. The CONAN_SYSTEM_INCLUDES right now only applies to global configuration, not to targets. This could be fixed.

[Sebiee]

How? I searched for a solution within CMake, but it seems that there's no solution since the property NO_SYSTEM_FROM_IMPORTED cannot be use on interface_library targets...

[memsharded]

It seems conan could use INTERFACE_SYSTEM_INCLUDE_DIRECTORIES property, as pointed out by @db4.

[Sebiee]

Well, I don't see how that would work in order to make the dirs not be treated as system. I tried to manually solve this, but with no success. It seems to me that CMake currently don't have a solution for this.

Anyway, I'm looking forward to see if you can get this solved. It would be useful to me.

[rasjani]

@memsharded Information regarding how/where to use CONAN_SYSTEM_INCLUDES was/is rather lacking but i do think i got it now - if i read the code, only globally defined includes will be marked as system but if i used cmake targets to bring in package's includes, there this has no effect right ?

Anyway, one more corner case where i would prefer for conan to have some form to handle also cmake targets for system includes:

Static analyzers will typically have feature that system include paths wont be scanned (or at least reported). If i have plenty of conan packages with header-only libraries, my reports will contain findings from those conan packages. Since at least atm im relying on cmake to generate a compile_commands.json, it would be nice to have some sort of configure/generator time switch to enforce also cmake targets to mark their own include directories as system.