Skip to content

Commit 2b3c664

Browse files
ingomueller-netingomueller-net
committed
build(deps): refactor CMake files for finding GTest
This PR refactors the CMake logic that finds or fetches the GTest/googletest dependency. It follows the same pattern applied by #138 for the protobuf library. The PR also removes a redundant logic of finding/fetching that dependency, which previously existed twice. Signed-off-by: Ingo Müller <ingomueller@google.com>
1 parent 83b2ec1 commit 2b3c664

2 files changed

Lines changed: 74 additions & 53 deletions

File tree

‎third_party/CMakeLists.txt‎

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)
55

66
set(SUBSTRAIT_CPP_PROTOBUF_DEFAULT_FETCH_TAG "v29.3")
7+
set(SUBSTRAIT_CPP_GTEST_DEFAULT_FETCH_TAG "v1.14.0")
78

9+
include(gtest.cmake)
810
include(datetime.cmake)
911
include(protobuf.cmake)
1012

@@ -16,47 +18,6 @@ endif()
1618

1719
add_subdirectory(fmt)
1820

19-
if(WIN32)
20-
# For Windows: Prevent overriding the parent project's compiler/linker settings
21-
set(gtest_force_shared_crt
22-
ON
23-
CACHE BOOL "" FORCE)
24-
endif()
25-
26-
find_package(GTest QUIET)
27-
if(NOT ${GTEST_FOUND})
28-
message(STATUS "Retrieving external GoogleTest library.")
29-
include(FetchContent)
30-
fetchcontent_declare(
31-
GTest
32-
GIT_REPOSITORY https://github.com/google/googletest.git
33-
GIT_TAG v1.14.0
34-
OVERRIDE_FIND_PACKAGE)
35-
fetchcontent_makeavailable(GTest)
36-
endif()
37-
if(MSVC)
38-
# ------------------------------------------------------------------------------
39-
# gtest MSVC fix
40-
# ------------------------------------------------------------------------------
41-
# For some reason, googletest has include path issues when built with MSVC.
42-
# Specifically, this seems like some incorrect assumptions about include paths
43-
# inside the gmock project.
44-
# We can fix this by injecting the include paths here.
45-
function(fix_gtest_include TARGET)
46-
target_include_directories(
47-
${TARGET}
48-
PUBLIC $<BUILD_INTERFACE:${gtest_SOURCE_DIR}>
49-
$<BUILD_INTERFACE:${gtest_SOURCE_DIR}/include>
50-
$<BUILD_INTERFACE:${gtest_SOURCE_DIR}/googletest>
51-
$<BUILD_INTERFACE:${gtest_SOURCE_DIR}/googletest/include>
52-
$<INSTALL_INTERFACE:include/${TARGET}>)
53-
endfunction()
54-
set(gtest_erroneous_targets gmock gmock_main)
55-
foreach(target ${gtest_erroneous_targets})
56-
fix_gtest_include(${target})
57-
endforeach()
58-
endif()
59-
6021
set(PROTOBUF_MATCHERS_BUILD_TESTING OFF)
6122
add_subdirectory(protobuf-matchers)
6223

‎third_party/gtest.cmake‎

Lines changed: 72 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,78 @@
22

33
include_guard(GLOBAL)
44

5-
include(FetchContent)
6-
fetchcontent_declare(
7-
GTest
8-
GIT_REPOSITORY https://github.com/google/googletest.git
9-
GIT_TAG v1.14.0
10-
OVERRIDE_FIND_PACKAGE)
5+
option(SUBSTRAIT_CPP_USE_SYSTEM_GTEST "Use system GTest via find_package" ON)
6+
option(SUBSTRAIT_CPP_FIND_GTEST_CONFIG
7+
"Use CONFIG mode to find system GTest via find_package" ON)
8+
option(SUBSTRAIT_CPP_FETCH_GTEST "Download GTest via FetchContent if not found"
9+
ON)
10+
set(SUBSTRAIT_CPP_GTEST_FETCH_TAG
11+
${SUBSTRAIT_CPP_GTEST_DEFAULT_FETCH_TAG}
12+
CACHE STRING "Git tag or commit to use for GTest FetchContent")
1113

12-
# Disable warnings for dependency targets.
14+
# First use `find_package`. This allows downstream projects to inject their
15+
# version with `FetchContent_Declare(... OVERRIDE_FIND_PACKAGE`.
16+
if(SUBSTRAIT_CPP_USE_SYSTEM_GTEST)
17+
if(SUBSTRAIT_CPP_FIND_GTEST_CONFIG)
18+
find_package(GTest CONFIG)
19+
else()
20+
find_package(GTest)
21+
endif()
22+
endif()
23+
24+
# Now fall back to using `FetchContent`.
25+
if(NOT GTest_FOUND AND SUBSTRAIT_CPP_FETCH_GTEST)
26+
message(STATUS "Fetching googletest version ${SUBSTRAIT_CPP_GTEST_FETCH_TAG}")
27+
include(FetchContent)
28+
fetchcontent_declare(
29+
googletest
30+
GIT_REPOSITORY https://github.com/google/googletest.git
31+
GIT_TAG ${SUBSTRAIT_CPP_GTEST_FETCH_TAG}
32+
SYSTEM OVERRIDE_FIND_PACKAGE CMAKE_ARGS
33+
-DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
34+
-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER})
35+
if(MSVC)
36+
set(gtest_force_shared_crt
37+
ON
38+
CACHE BOOL "" FORCE)
39+
add_compile_options("/W0")
40+
else()
41+
add_compile_options("-w")
42+
endif()
43+
fetchcontent_makeavailable(googletest)
44+
fetchcontent_getproperties(googletest SOURCE_DIR GTest_SOURCE_DIR)
45+
if(TARGET gmock AND NOT TARGET GTest::gmock)
46+
add_library(GTest::gmock ALIAS gmock)
47+
endif()
48+
if(TARGET gmock_main AND NOT TARGET GTest::gmock_main)
49+
add_library(GTest::gmock_main ALIAS gmock_main)
50+
endif()
51+
endif()
52+
53+
if(NOT TARGET GTest::gmock)
54+
message(FATAL_ERROR "GTest is required but was not found or fetched.")
55+
endif()
56+
57+
# XXX: Verify if this is still necessary.
1358
if(MSVC)
14-
set(gtest_force_shared_crt ON)
15-
add_compile_options("/W0")
16-
else()
17-
add_compile_options("-w")
59+
# ------------------------------------------------------------------------------
60+
# gtest MSVC fix
61+
# ------------------------------------------------------------------------------
62+
# For some reason, googletest has include path issues when built with MSVC.
63+
# Specifically, this seems like some incorrect assumptions about include paths
64+
# inside the gmock project.
65+
# We can fix this by injecting the include paths here.
66+
function(fix_gtest_include TARGET)
67+
target_include_directories(
68+
${TARGET}
69+
PUBLIC $<BUILD_INTERFACE:${gtest_SOURCE_DIR}>
70+
$<BUILD_INTERFACE:${gtest_SOURCE_DIR}/include>
71+
$<BUILD_INTERFACE:${gtest_SOURCE_DIR}/googletest>
72+
$<BUILD_INTERFACE:${gtest_SOURCE_DIR}/googletest/include>
73+
$<INSTALL_INTERFACE:include/${TARGET}>)
74+
endfunction()
75+
set(gtest_erroneous_targets gmock gmock_main)
76+
foreach(target ${gtest_erroneous_targets})
77+
fix_gtest_include(${target})
78+
endforeach()
1879
endif()
19-
fetchcontent_makeavailable(GTest)

0 commit comments

Comments
 (0)