If gtest targets are already defined, use them. #777
Conversation
cmake/HandleGTest.cmake
Outdated
| # This allows a project to include both googletest and benchmark | ||
| # as top-level git submodule. | ||
| set(GTEST_BOTH_LIBRARIES gtest gmock gmock_main) | ||
| elseif (IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/googletest) |
There was a problem hiding this comment.
What should happen when you've explicitly checked out googletest into benchmark/, but also have already defined the targets? It's not clear what version we should use.
Also, what happens the second time this CMake file is processed? AKA, the first pass we create the targets ourselves. The second pass we detect that the targets are present?
There was a problem hiding this comment.
If gtest/gmock targets are already defined, we should not use benchmark/googletest. Otherwise CMake complains about duplicated targets, as it requires target names being globally unique [https://cmake.org/cmake/help/v3.9/policy/CMP0002.html].
CMake Error at benchmark/googletest/googletest/cmake/internal_utils.cmake:145 (add_library):
add_library cannot create target "gtest" because another target with the
same name already exists. The existing target is a static library created
in source directory
"/home/schen/git/recipes/benchmark/googletest/googletest".
See documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
benchmark/googletest/googletest/cmake/internal_utils.cmake:197 (cxx_library_with_type)
benchmark/googletest/googletest/CMakeLists.txt:125 (cxx_library)
I don't quite understand how this file could be processed twice. It's only included by root/CMakeLists.txt. If root/CMakeLists.txt is added twice, I am pretty sure we will have the same duplicate targets error as above.
|
I think this should be ok to have, but needs a rebase. |
|
In particular, the entirely of the patch should now simply be diff --git a/CMakeLists.txt b/CMakeLists.txt
index 856bc98..74b9934 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -267,7 +267,9 @@ add_subdirectory(src)
if (BENCHMARK_ENABLE_TESTING)
enable_testing()
- if (BENCHMARK_ENABLE_GTEST_TESTS)
+ if (BENCHMARK_ENABLE_GTEST_TESTS AND
+ NOT (TARGET gtest AND TARGET gtest_main AND
+ TARGET gmock AND TARGET gmock_main))
include(GoogleTest)
endif()
add_subdirectory(test)
(^ i'd merge that |
This allows a project to include both googletest and benchmark as top-level git submodule. This allows incorporating Benchmark to an existing CMake project that already incorporates googletest. https://github.com/google/googletest/blob/master/googletest/README.md#incorporating-into-an-existing-cmake-project https://github.com/abseil/abseil-cpp/tree/master/CMake#incorporating-abseil-into-a-cmake-project
|
Done rebasing. |
This allows a project to include both googletest and benchmark as top-level git submodule. This allows incorporating Benchmark to an existing CMake project that already incorporates googletest. https://github.com/google/googletest/blob/master/googletest/README.md#incorporating-into-an-existing-cmake-project https://github.com/abseil/abseil-cpp/tree/master/CMake#incorporating-abseil-into-a-cmake-project
This allows a project to include both googletest and benchmark as top-level git submodule.
This allows incorporating Benchmark to an existing CMake project that already incorporates googletest.
https://github.com/google/googletest/blob/master/googletest/README.md#incorporating-into-an-existing-cmake-project
https://github.com/abseil/abseil-cpp/tree/master/CMake#incorporating-abseil-into-a-cmake-project