From c63bae4eb5caff41fecc343b7290adf1ec001ec4 Mon Sep 17 00:00:00 2001 From: varunu28 Date: Thu, 30 Oct 2025 09:56:36 -0700 Subject: [PATCH 1/2] (fix) Fixing build issue for googletest on Mac --- CMakeLists.txt | 3 +++ third_party/CMakeLists.txt | 3 +++ third_party/googletest/googlemock/CMakeLists.txt | 5 +++-- third_party/googletest/googletest/CMakeLists.txt | 5 +++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fdec31a0a..bc7f0783e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -150,6 +150,9 @@ set(BUSTUB_THIRD_PARTY_INCLUDE_DIR include_directories(${BUSTUB_SRC_INCLUDE_DIR} ${BUSTUB_TEST_INCLUDE_DIR} ${BUSTUB_THIRD_PARTY_INCLUDE_DIR}) include_directories(BEFORE src) # This is needed for gtest. +# This ensures that the bundled headers are searched before system paths +include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googletest/include) +include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googlemock/include) function(disable_target_warnings NAME) target_compile_options(${NAME} PRIVATE "-w") diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index 03dd8ce05..dfdfed075 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -2,6 +2,9 @@ add_subdirectory(murmur3) add_subdirectory(libpg_query) set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) # don't override our compiler/linker options when building gtest +# Prevent system googletest headers from being used - add our include paths with higher priority +include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googletest/include) +include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googlemock/include) add_subdirectory(googletest) add_subdirectory(fmt) diff --git a/third_party/googletest/googlemock/CMakeLists.txt b/third_party/googletest/googlemock/CMakeLists.txt index a9aa0723f..45a094a1f 100644 --- a/third_party/googletest/googlemock/CMakeLists.txt +++ b/third_party/googletest/googlemock/CMakeLists.txt @@ -102,10 +102,11 @@ else() endif() string(REPLACE ";" "$" dirs "${gmock_build_include_dirs}") -target_include_directories(gmock SYSTEM INTERFACE +# Don't add SYSTEM as CMake uses -isystem compiler flag which will override the bundled headers & use system specific headers +target_include_directories(gmock INTERFACE "$" "$/${CMAKE_INSTALL_INCLUDEDIR}>") -target_include_directories(gmock_main SYSTEM INTERFACE +target_include_directories(gmock_main INTERFACE "$" "$/${CMAKE_INSTALL_INCLUDEDIR}>") diff --git a/third_party/googletest/googletest/CMakeLists.txt b/third_party/googletest/googletest/CMakeLists.txt index caafa8c76..051a5806c 100644 --- a/third_party/googletest/googletest/CMakeLists.txt +++ b/third_party/googletest/googletest/CMakeLists.txt @@ -141,10 +141,11 @@ endif() cxx_library(gtest_main "${cxx_strict}" src/gtest_main.cc) set_target_properties(gtest_main PROPERTIES VERSION ${GOOGLETEST_VERSION}) string(REPLACE ";" "$" dirs "${gtest_build_include_dirs}") -target_include_directories(gtest SYSTEM INTERFACE +# Don't add SYSTEM as CMake uses -isystem compiler flag which will override the bundled headers & use system specific headers +target_include_directories(gtest INTERFACE "$" "$/${CMAKE_INSTALL_INCLUDEDIR}>") -target_include_directories(gtest_main SYSTEM INTERFACE +target_include_directories(gtest_main INTERFACE "$" "$/${CMAKE_INSTALL_INCLUDEDIR}>") if(CMAKE_SYSTEM_NAME MATCHES "QNX") From 2ce4d3b45266f20db2c726d0b3b2c03c149482b9 Mon Sep 17 00:00:00 2001 From: varunu28 Date: Fri, 31 Oct 2025 05:41:51 -0700 Subject: [PATCH 2/2] Updating the check to be applied only when system googletest exists --- CMakeLists.txt | 7 ++++-- third_party/CMakeLists.txt | 7 ++++-- .../googletest/googlemock/CMakeLists.txt | 24 +++++++++++++------ .../googletest/googletest/CMakeLists.txt | 24 +++++++++++++------ 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bc7f0783e..8c26404a8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -151,8 +151,11 @@ set(BUSTUB_THIRD_PARTY_INCLUDE_DIR include_directories(${BUSTUB_SRC_INCLUDE_DIR} ${BUSTUB_TEST_INCLUDE_DIR} ${BUSTUB_THIRD_PARTY_INCLUDE_DIR}) include_directories(BEFORE src) # This is needed for gtest. # This ensures that the bundled headers are searched before system paths -include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googletest/include) -include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googlemock/include) +if (EXISTS "/usr/local/include/gtest" OR EXISTS "/opt/homebrew/include/gtest") + message(STATUS "System googletest detected, prioritizing bundled version") + include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googletest/include) + include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googlemock/include) +endif() function(disable_target_warnings NAME) target_compile_options(${NAME} PRIVATE "-w") diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index dfdfed075..5440e4668 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -2,9 +2,12 @@ add_subdirectory(murmur3) add_subdirectory(libpg_query) set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) # don't override our compiler/linker options when building gtest +if (EXISTS "/usr/local/include/gtest" OR EXISTS "/opt/homebrew/include/gtest") + message(STATUS "System googletest detected, prioritizing bundled version") # Prevent system googletest headers from being used - add our include paths with higher priority -include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googletest/include) -include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googlemock/include) + include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googletest/include) + include_directories(BEFORE ${PROJECT_SOURCE_DIR}/third_party/googletest/googlemock/include) +endif() add_subdirectory(googletest) add_subdirectory(fmt) diff --git a/third_party/googletest/googlemock/CMakeLists.txt b/third_party/googletest/googlemock/CMakeLists.txt index 45a094a1f..ebae68296 100644 --- a/third_party/googletest/googlemock/CMakeLists.txt +++ b/third_party/googletest/googlemock/CMakeLists.txt @@ -102,13 +102,23 @@ else() endif() string(REPLACE ";" "$" dirs "${gmock_build_include_dirs}") -# Don't add SYSTEM as CMake uses -isystem compiler flag which will override the bundled headers & use system specific headers -target_include_directories(gmock INTERFACE - "$" - "$/${CMAKE_INSTALL_INCLUDEDIR}>") -target_include_directories(gmock_main INTERFACE - "$" - "$/${CMAKE_INSTALL_INCLUDEDIR}>") +if (EXISTS "/usr/local/include/gtest" OR EXISTS "/opt/homebrew/include/gtest") + message(STATUS "System googletest detected, prioritizing bundled version") + # Don't add SYSTEM as CMake uses -isystem compiler flag which will override the bundled headers & use system specific headers + target_include_directories(gmock INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") + target_include_directories(gmock_main INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") +else() + target_include_directories(gmock SYSTEM INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") + target_include_directories(gmock_main SYSTEM INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") +endif() ######################################################################## # diff --git a/third_party/googletest/googletest/CMakeLists.txt b/third_party/googletest/googletest/CMakeLists.txt index 051a5806c..261569cf6 100644 --- a/third_party/googletest/googletest/CMakeLists.txt +++ b/third_party/googletest/googletest/CMakeLists.txt @@ -141,13 +141,23 @@ endif() cxx_library(gtest_main "${cxx_strict}" src/gtest_main.cc) set_target_properties(gtest_main PROPERTIES VERSION ${GOOGLETEST_VERSION}) string(REPLACE ";" "$" dirs "${gtest_build_include_dirs}") -# Don't add SYSTEM as CMake uses -isystem compiler flag which will override the bundled headers & use system specific headers -target_include_directories(gtest INTERFACE - "$" - "$/${CMAKE_INSTALL_INCLUDEDIR}>") -target_include_directories(gtest_main INTERFACE - "$" - "$/${CMAKE_INSTALL_INCLUDEDIR}>") +if (EXISTS "/usr/local/include/gtest" OR EXISTS "/opt/homebrew/include/gtest") + message(STATUS "System googletest detected, prioritizing bundled version") + # Don't add SYSTEM as CMake uses -isystem compiler flag which will override the bundled headers & use system specific headers + target_include_directories(gtest INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") + target_include_directories(gtest_main INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") +else() + target_include_directories(gtest SYSTEM INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") + target_include_directories(gtest_main SYSTEM INTERFACE + "$" + "$/${CMAKE_INSTALL_INCLUDEDIR}>") +endif() if(CMAKE_SYSTEM_NAME MATCHES "QNX") target_link_libraries(gtest PUBLIC regex) endif()