search

BlueBrain / Rockets

REST and websockets C++ library
GNU Lesser General Public License v3.0
38 stars 8 forks source link

Do not find websockets as SYSTEM #37

Closed karjonas closed 5 years ago

karjonas commented 6 years ago

For some reason I needed to remove this when I compiled on Arch Linux. I tried this on my Ubuntu machine and it seems to work fine.

rdumusc commented 6 years ago

That's odd, the SYSTEM flag is passed to the CMake include_directories(). Normally it should not affect the search, but help ignore compiler warnings that could occur in the included header files. I don't think it's a big deal to remove it, but it still feels illogical. Do you have more details about the error you're seeing?

rdumusc commented 6 years ago

In fact, removing it fails the build on cscs VMs, see: https://bbpcode.epfl.ch/ci/job/oss.Rockets.github/109/build_type=Debug,platform=cscsviz-vm/console

rolandjitsu commented 6 years ago

Build is failing on the vm, could it be because of the flag that was removed?

karjonas commented 6 years ago

@rdumusc I will look into this when I am at my machine but I think the issue is related to the following SO question: https://stackoverflow.com/questions/37218953/isystem-on-a-system-include-directory-causes-errors

One solution then is to set a gcc pragma that disables the warnings when including the libwebsockets.h file like suggested here: https://stackoverflow.com/questions/6321839/how-to-disable-warnings-for-particular-include-files

@rolandjitsu This build is failing due to the flag yes

karjonas commented 6 years ago

@rdumusc Do you think this wrapper <rockets/websockets.h> include file is a valid solution?

rdumusc commented 6 years ago

Thanks, I'm still reading this article you sent. I'm wondering if we can't find a more generic fix in CMake to avoid this type of hacks in the code. We have a lot of -isystem which could in theory cause problems (boost, qt, etc..) so I'm not sure why you observed this just with libwebsockets.

rdumusc commented 6 years ago

Looks like an annoying change introduced in gcc6 in the behaviour of -isystem. I found the following relevant info regarding CMake: https://gitlab.kitware.com/cmake/cmake/issues/16291 But I haven't understood if filling those CMake variables, or manually NOT calling include_directories(SYSTEM) for paths that are already default in our common_find_package() will fix the problem or if we will lose the warning suppression as a result... I am also under the impression that arch-linux has some specificities in the include paths that lead to this situation... We would need to fully test this behavior on a newer system, starting with an Ubuntu 18.04 VM (gcc7). So I'd like to hold off this change until we're sure that no other option is available.