From aae781d3605505392878d82a0c715fe355059bda Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ingo=20M=C3=BCller?= <ingomueller@google.com>
Date: Thu, 26 Mar 2026 11:09:52 +0100
Subject: [PATCH] build(deps): refactor CMake files for finding GTest
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 third_party/CMakeLists.txt | 43 ++------------------------
 third_party/gtest.cmake    | 62 ++++++++++++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 54 deletions(-)

diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt
index b027aabe..94a1e1f7 100644
--- a/third_party/CMakeLists.txt
+++ b/third_party/CMakeLists.txt
@@ -4,7 +4,9 @@
 set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)
 
 set(SUBSTRAIT_CPP_PROTOBUF_DEFAULT_FETCH_TAG "v29.3")
+set(SUBSTRAIT_CPP_GTEST_DEFAULT_FETCH_TAG "v1.14.0")
 
+include(gtest.cmake)
 include(datetime.cmake)
 include(protobuf.cmake)
 
@@ -16,47 +18,6 @@ endif()
 
 add_subdirectory(fmt)
 
-if(WIN32)
-  # For Windows: Prevent overriding the parent project's compiler/linker settings
-  set(gtest_force_shared_crt
-      ON
-      CACHE BOOL "" FORCE)
-endif()
-
-find_package(GTest QUIET)
-if(NOT ${GTEST_FOUND})
-  message(STATUS "Retrieving external GoogleTest library.")
-  include(FetchContent)
-  fetchcontent_declare(
-    GTest
-    GIT_REPOSITORY https://github.com/google/googletest.git
-    GIT_TAG v1.14.0
-    OVERRIDE_FIND_PACKAGE)
-  fetchcontent_makeavailable(GTest)
-endif()
-if(MSVC)
-  # ------------------------------------------------------------------------------
-  # gtest MSVC fix
-  # ------------------------------------------------------------------------------
-  # For some reason, googletest has include path issues when built with MSVC.
-  # Specifically, this seems like some incorrect assumptions about include paths
-  # inside the gmock project.
-  # We can fix this by injecting the include paths here.
-  function(fix_gtest_include TARGET)
-    target_include_directories(
-      ${TARGET}
-      PUBLIC $<BUILD_INTERFACE:${gtest_SOURCE_DIR}>
-             $<BUILD_INTERFACE:${gtest_SOURCE_DIR}/include>
-             $<BUILD_INTERFACE:${gtest_SOURCE_DIR}/googletest>
-             $<BUILD_INTERFACE:${gtest_SOURCE_DIR}/googletest/include>
-             $<INSTALL_INTERFACE:include/${TARGET}>)
-  endfunction()
-  set(gtest_erroneous_targets gmock gmock_main)
-  foreach(target ${gtest_erroneous_targets})
-    fix_gtest_include(${target})
-  endforeach()
-endif()
-
 set(PROTOBUF_MATCHERS_BUILD_TESTING OFF)
 add_subdirectory(protobuf-matchers)
 
diff --git a/third_party/gtest.cmake b/third_party/gtest.cmake
index f43a0714..66c8c1b8 100644
--- a/third_party/gtest.cmake
+++ b/third_party/gtest.cmake
@@ -2,18 +2,54 @@
 
 include_guard(GLOBAL)
 
-include(FetchContent)
-fetchcontent_declare(
-  GTest
-  GIT_REPOSITORY https://github.com/google/googletest.git
-  GIT_TAG v1.14.0
-  OVERRIDE_FIND_PACKAGE)
+option(SUBSTRAIT_CPP_USE_SYSTEM_GTEST "Use system GTest via find_package" ON)
+option(SUBSTRAIT_CPP_FIND_GTEST_CONFIG
+       "Use CONFIG mode to find system GTest via find_package" ON)
+option(SUBSTRAIT_CPP_FETCH_GTEST "Download GTest via FetchContent if not found"
+       ON)
+set(SUBSTRAIT_CPP_GTEST_FETCH_TAG
+    ${SUBSTRAIT_CPP_GTEST_DEFAULT_FETCH_TAG}
+    CACHE STRING "Git tag or commit to use for GTest FetchContent")
 
-# Disable warnings for dependency targets.
-if(MSVC)
-  set(gtest_force_shared_crt ON)
-  add_compile_options("/W0")
-else()
-  add_compile_options("-w")
+# First use `find_package`. This allows downstream projects to inject their
+# version with `FetchContent_Declare(... OVERRIDE_FIND_PACKAGE`.
+if(SUBSTRAIT_CPP_USE_SYSTEM_GTEST)
+  if(SUBSTRAIT_CPP_FIND_GTEST_CONFIG)
+    find_package(GTest CONFIG)
+  else()
+    find_package(GTest)
+  endif()
+endif()
+
+# Now fall back to using `FetchContent`.
+if(NOT GTest_FOUND AND SUBSTRAIT_CPP_FETCH_GTEST)
+  message(STATUS "Fetching googletest version ${SUBSTRAIT_CPP_GTEST_FETCH_TAG}")
+  include(FetchContent)
+  fetchcontent_declare(
+    googletest
+    GIT_REPOSITORY https://github.com/google/googletest.git
+    GIT_TAG ${SUBSTRAIT_CPP_GTEST_FETCH_TAG}
+    SYSTEM OVERRIDE_FIND_PACKAGE CMAKE_ARGS
+    -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
+    -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER})
+  if(MSVC)
+    set(gtest_force_shared_crt
+        ON
+        CACHE BOOL "" FORCE)
+    add_compile_options("/W0")
+  else()
+    add_compile_options("-w")
+  endif()
+  fetchcontent_makeavailable(googletest)
+  fetchcontent_getproperties(googletest SOURCE_DIR GTest_SOURCE_DIR)
+  if(TARGET gmock AND NOT TARGET GTest::gmock)
+    add_library(GTest::gmock ALIAS gmock)
+  endif()
+  if(TARGET gmock_main AND NOT TARGET GTest::gmock_main)
+    add_library(GTest::gmock_main ALIAS gmock_main)
+  endif()
+endif()
+
+if(NOT TARGET GTest::gmock)
+  message(FATAL_ERROR "GTest is required but was not found or fetched.")
 endif()
-fetchcontent_makeavailable(GTest)