Skip to content

Commit 9221381

Browse files
authored
build: move GTest and Google Benchmark setup into IcebergThirdpartyToolchain (#836)
## What Fixes #829. The GoogleTest and Google Benchmark FetchContent setup lived in `src/iceberg/test/CMakeLists.txt` and `src/iceberg/benchmark/CMakeLists.txt`, apart from the other third-party dependencies. This follows the review discussion on #825, which suggested keeping dependency resolution in one place. ## How - Add `resolve_gtest_dependency()` and `resolve_benchmark_dependency()` to `cmake_modules/IcebergThirdpartyToolchain.cmake`, next to the existing `resolve_*` functions. - Gate them on `ICEBERG_BUILD_TESTS` and `ICEBERG_BUILD_BENCHMARKS`, the same conditions that already guard the test and benchmark subdirectories. - Both functions call `prepare_fetchcontent()` like the other resolvers, so a command-line `-DCMAKE_COMPILE_WARNING_AS_ERROR=ON` does not build the vendored GTest/Benchmark sources with `-Werror`. - Version pins are unchanged. Only CMake is affected; Meson resolves these through `subprojects/*.wrap`, so it needs no change. ## Verified - `-DICEBERG_BUILD_BENCHMARKS=ON`: both dependencies fetch; `benchmark_smoke` and `schema_test` build; `ctest -R schema_test` passes. - Tests and benchmarks both off: neither dependency is fetched. - `-DCMAKE_COMPILE_WARNING_AS_ERROR=ON` with benchmarks on: configure and build succeed. - `pre-commit run -a` clean. --- This contribution was authored with assistance from Claude Opus 4.8, in line with the Iceberg guidelines for AI-assisted contributions (https://iceberg.apache.org/contribute/#guidelines-for-ai-assisted-contributions). All changes were reviewed and tested by the author.
1 parent bfc0040 commit 9221381

3 files changed

Lines changed: 67 additions & 30 deletions

File tree

cmake_modules/IcebergThirdpartyToolchain.cmake

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,57 @@ function(resolve_zstd_dependency)
801801
endif()
802802
endfunction()
803803

804+
# ----------------------------------------------------------------------
805+
# GoogleTest (tests only)
806+
#
807+
# GTest is only consumed by the unit tests; it is neither installed nor exported
808+
# as a system dependency, so it does not touch ICEBERG_SYSTEM_DEPENDENCIES.
809+
810+
function(resolve_gtest_dependency)
811+
prepare_fetchcontent()
812+
813+
set(INSTALL_GTEST
814+
OFF
815+
CACHE BOOL "" FORCE)
816+
817+
fetchcontent_declare(googletest
818+
GIT_REPOSITORY https://github.com/google/googletest.git
819+
GIT_TAG b514bdc898e2951020cbdca1304b75f5950d1f59 # release-1.15.2
820+
FIND_PACKAGE_ARGS
821+
NAMES
822+
GTest)
823+
fetchcontent_makeavailable(googletest)
824+
endfunction()
825+
826+
# ----------------------------------------------------------------------
827+
# Google Benchmark (benchmarks only)
828+
#
829+
# Like GTest, benchmark is only consumed by the benchmark targets and is neither
830+
# installed nor exported as a system dependency.
831+
832+
function(resolve_benchmark_dependency)
833+
prepare_fetchcontent()
834+
835+
set(BENCHMARK_ENABLE_GTEST_TESTS
836+
OFF
837+
CACHE BOOL "" FORCE)
838+
set(BENCHMARK_ENABLE_INSTALL
839+
OFF
840+
CACHE BOOL "" FORCE)
841+
set(BENCHMARK_ENABLE_TESTING
842+
OFF
843+
CACHE BOOL "" FORCE)
844+
845+
fetchcontent_declare(google_benchmark
846+
GIT_REPOSITORY https://github.com/google/benchmark.git
847+
GIT_TAG a4cf155615c63e019ae549e31703bf367df5b471 # v1.8.4
848+
FIND_PACKAGE_ARGS
849+
NAMES
850+
benchmark
851+
CONFIG)
852+
fetchcontent_makeavailable(google_benchmark)
853+
endfunction()
854+
804855
resolve_zlib_dependency()
805856
resolve_nanoarrow_dependency()
806857
resolve_croaring_dependency()
@@ -864,3 +915,17 @@ endfunction()
864915
if(ICEBERG_BUILD_HIVE)
865916
resolve_thrift_dependency()
866917
endif()
918+
919+
# ----------------------------------------------------------------------
920+
# Test and benchmark dependencies
921+
#
922+
# These are build-only tools, pulled in after the library dependencies and only
923+
# when the corresponding build option is enabled.
924+
925+
if(ICEBERG_BUILD_TESTS)
926+
resolve_gtest_dependency()
927+
endif()
928+
929+
if(ICEBERG_BUILD_BENCHMARKS)
930+
resolve_benchmark_dependency()
931+
endif()

src/iceberg/benchmark/CMakeLists.txt

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,7 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18-
set(BENCHMARK_ENABLE_GTEST_TESTS
19-
OFF
20-
CACHE BOOL "" FORCE)
21-
set(BENCHMARK_ENABLE_INSTALL
22-
OFF
23-
CACHE BOOL "" FORCE)
24-
set(BENCHMARK_ENABLE_TESTING
25-
OFF
26-
CACHE BOOL "" FORCE)
27-
28-
fetchcontent_declare(google_benchmark
29-
GIT_REPOSITORY https://github.com/google/benchmark.git
30-
GIT_TAG a4cf155615c63e019ae549e31703bf367df5b471 # v1.8.4
31-
FIND_PACKAGE_ARGS
32-
NAMES
33-
benchmark
34-
CONFIG)
35-
fetchcontent_makeavailable(google_benchmark)
18+
# Google Benchmark is resolved in cmake_modules/IcebergThirdpartyToolchain.cmake.
3619

3720
add_executable(benchmark_smoke benchmark_smoke.cc)
3821
target_link_libraries(benchmark_smoke

src/iceberg/test/CMakeLists.txt

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,7 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18-
set(INSTALL_GTEST
19-
OFF
20-
CACHE BOOL "" FORCE)
21-
22-
fetchcontent_declare(googletest
23-
GIT_REPOSITORY https://github.com/google/googletest.git
24-
GIT_TAG b514bdc898e2951020cbdca1304b75f5950d1f59 # release-1.15.2
25-
FIND_PACKAGE_ARGS
26-
NAMES
27-
GTest)
28-
29-
fetchcontent_makeavailable(googletest)
18+
# GoogleTest is resolved in cmake_modules/IcebergThirdpartyToolchain.cmake.
3019

3120
set(ICEBERG_TEST_RESOURCES "${CMAKE_SOURCE_DIR}/src/iceberg/test/resources")
3221

0 commit comments

Comments
 (0)