Conversation
This allows to use a packaged version of gtest, which is e.g important for distributions (e.g) Debian. The gtest submodule already defines CMake targets (GTest::gtest), so it is sufficient to declare target_link_libaries with that target, and it will automatically pick up the include dir as well.
to make it more clean what it means.
asmaloney
left a comment
There was a problem hiding this comment.
Thanks!
Could you please also add a small note about the new option in test/README.md? Maybe just a one-liner in the Turning Testing On section. "To use an external gtest package..."
| endif() | ||
|
|
||
| # GoogleTest from here: https://github.com/google/googletest | ||
| if ( USE_PACKAGED_GTEST ) |
There was a problem hiding this comment.
I prefer your old name, but prefixed with E57 like this library's other options.
"packaged" could mean with the one bundled with the library or an external package. "external" is clearly external to the library.
E57_USE_EXTERNAL_GTEST
I would also like this to be a bool option so it's visible in the cmake UI.
option( E57_USE_EXTERNAL_GTEST "Use an external gtest package" OFF )|
|
||
| # GoogleTest from here: https://github.com/google/googletest | ||
| if ( USE_PACKAGED_GTEST ) | ||
| find_package( GTest REQUIRED) |
There was a problem hiding this comment.
Could you please add a status message so it's clear it's using an external gtest?
Something like:
message( STATUS "[${PROJECT_NAME}] Using external gtest from XXX" )Where XXX comes from whatever path var the find_package creates for it.
Type
Summary
This allows to use a packaged version of gtest, which is e.g important for distributions (e.g) Debian.
The gtest submodule already defines CMake targets (GTest::gtest), so it is sufficient to declare target_link_libaries with that target, and it will automatically pick up the include dir as well.
Rationale
This is to make it easier to package for Debian - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1132004
Checklist