-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ThirdParty Libs: IMPORTED #389
Conversation
oh wait, that's not working with Dang, that's a CMake 3.11.0+ feature! Updated accordingly. Maybe even needs 3.11.2+ |
4256a6e
to
d017783
Compare
The nlohmann-json library should be marked as `SYSTEM` linked lib. This will cause it to be linked with `-isystem`, allowing us to use warnings in openPMD that are stricter than the ones in the dependency (since their headers do not throw warnings anymore).
CMake thirdParty libraries should be marked as imported. We do so by creating `openPMD::thirdparty::` targets that are imported only and depend on `find_package` or `add_subdirectory` targets to decouple them. Imported targets are always SYSTEM libraries: https://cmake.org/cmake/help/v3.8/prop_tgt/NO_SYSTEM_FROM_IMPORTED.html That allows us to use stricter warnings in our project than third party headers support.
Modifying properties of `IMPORTED` targets is a CMake 3.11.0+ feature: https://cmake.org/cmake/help/v3.11/release/3.11.html#commands
d017783
to
0c85a24
Compare
Hi @ax3l, cmake_minimum_required(VERSION 3.0)
project(openPMD_test)
add_executable(openPMD_mpi_write mpi_write.cpp)
find_package(MPI REQUIRED)
add_definitions(-DOMPI_SKIP_MPICXX)
find_package(openPMD REQUIRED)
target_link_libraries(openPMD_mpi_write PRIVATE openPMD::openPMD MPI::MPI_C MPI::MPI_CXX) Fails with the output:
The openPMD build configuration is as follows:
Do you have an idea whether the issue is on my side? When I checkout the last commit before this one has been merged, everything works fine. |
I think it's on my side, indeed there might be a |
CMake thirdParty libraries should be marked as imported. We do so by creating
openPMD::thirdparty::
targets that are imported only and depend onfind_package
oradd_subdirectory
targets to decouple them.Imported targets are always
SYSTEM
libraries: https://cmake.org/cmake/help/v3.8/prop_tgt/NO_SYSTEM_FROM_IMPORTED.htmlThat allows us to use stricter warnings in our project than third party headers support.
Original issue:
The nlohmann-json library should be marked as
SYSTEM
linked lib. This will cause it to be linked with-isystem
, allowing us to use warnings in openPMD that are stricter than the ones in the dependency (since their headers do not throw warnings anymore).nlohmann/json#1339