Skip to content

build: move GTest and Google Benchmark setup into IcebergThirdpartyToolchain#836

Merged
wgtmac merged 1 commit into
apache:mainfrom
LuciferYang:move-gtest-benchmark-to-toolchain
Jul 20, 2026
Merged

build: move GTest and Google Benchmark setup into IcebergThirdpartyToolchain#836
wgtmac merged 1 commit into
apache:mainfrom
LuciferYang:move-gtest-benchmark-to-toolchain

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

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.

…olchain

Move the GoogleTest and Google Benchmark FetchContent setup from the test and
benchmark subdirectories into cmake_modules/IcebergThirdpartyToolchain.cmake, so
all third-party dependency resolution stays in one place. Add
resolve_gtest_dependency() and resolve_benchmark_dependency(), gated by
ICEBERG_BUILD_TESTS and ICEBERG_BUILD_BENCHMARKS. Version pins are unchanged and
only CMake is affected.

Fixes apache#829.
Copilot AI review requested due to automatic review settings July 20, 2026 04:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes GoogleTest and Google Benchmark dependency resolution in cmake_modules/IcebergThirdpartyToolchain.cmake, aligning test/benchmark third-party setup with the rest of the project’s CMake dependency management (per #829 / discussion from #825).

Changes:

  • Moved GoogleTest FetchContent configuration out of src/iceberg/test/CMakeLists.txt into IcebergThirdpartyToolchain.cmake via resolve_gtest_dependency().
  • Moved Google Benchmark FetchContent configuration out of src/iceberg/benchmark/CMakeLists.txt into IcebergThirdpartyToolchain.cmake via resolve_benchmark_dependency().
  • Added gating so these build-only dependencies are only resolved when ICEBERG_BUILD_TESTS / ICEBERG_BUILD_BENCHMARKS are enabled.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/iceberg/test/CMakeLists.txt Removes local GTest FetchContent setup; relies on toolchain resolver.
src/iceberg/benchmark/CMakeLists.txt Removes local benchmark FetchContent setup; relies on toolchain resolver.
cmake_modules/IcebergThirdpartyToolchain.cmake Adds resolve_gtest_dependency() / resolve_benchmark_dependency() and conditionally invokes them based on build options.

@wgtmac
wgtmac merged commit 9221381 into apache:main Jul 20, 2026
21 checks passed
@LuciferYang

Copy link
Copy Markdown
Contributor Author

Thank you @wgtmac

@LuciferYang
LuciferYang deleted the move-gtest-benchmark-to-toolchain branch July 21, 2026 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move GTest and Google Benchmark setup into IcebergThirdpartyToolchain

3 participants