Skip to content

build: keep CMake and Meson definitions in sync - #896

Open
Sakshamm-Goyal wants to merge 2 commits into
apache:mainfrom
Sakshamm-Goyal:fix/build-definition-parity
Open

build: keep CMake and Meson definitions in sync#896
Sakshamm-Goyal wants to merge 2 commits into
apache:mainfrom
Sakshamm-Goyal:fix/build-definition-parity

Conversation

@Sakshamm-Goyal

Copy link
Copy Markdown

Summary

  • install the missing public headers in Meson and the top-level catalog headers in CMake
  • include snapshot_summary_builder_test.cc in Meson's table_test
  • align the CRoaring, nlohmann/json, and GoogleTest fallback versions across CMake and Meson
  • add a dependency-free parity regression and run it in CI

Closes #894.

Validation

  • python3 dev/test_build_definition_parity.py
  • pre-commit run --files .github/workflows/test.yml cmake_modules/IcebergThirdpartyToolchain.cmake src/iceberg/catalog/CMakeLists.txt src/iceberg/catalog/rest/auth/meson.build src/iceberg/meson.build src/iceberg/test/meson.build dev/test_build_definition_parity.py
  • CMake configure with bundle/REST disabled, followed by cmake --build build-cmake-parity --target table_test --parallel 2
  • ctest --test-dir build-cmake-parity -R '^table_test$' --output-on-failure
  • Meson configure with REST/tests enabled, followed by meson compile -C build-meson-parity table_test -j 2
  • meson test -C build-meson-parity table_test --print-errorlogs
  • inspected both generated install manifests for the corrected public headers

AI assistance disclosure

AI-assisted tooling was used for initial regression-test scaffolding and mechanical build-definition edits. I reviewed the change end-to-end, verified the dependency choices and generated manifests, and ran the validations listed above. I am not aware of remaining uncertainty in the affected build paths.

@Sakshamm-Goyal

Copy link
Copy Markdown
Author

The failing Ubuntu Debug and macOS Build Example steps were caused by CRoaring 4.3.11 exposing roaring-headers and roaring-headers-cpp in the vendored target install interface. Iceberg exports roaring without exporting those build-only helper targets, so downstream find_package(iceberg) marked the package not found.

I pushed 09fabab to keep both helper targets behind BUILD_INTERFACE and added parity coverage for the export invariant. Validation completed locally:

  • build-definition parity: 4/4 passed
  • repository pre-commit on the changed files: passed
  • real Debug CMake build + install: passed
  • installed targets contain no roaring-headers references
  • downstream example/ configure and build against the installed package: passed

Replacement CI is now running on the updated head.

@manuzhang

Copy link
Copy Markdown
Member

@Sakshamm-Goyal Thanks for the PR! I opened #894 to track current status and initiated a discussion on how we can maintain and sync the build tools in the long term. Before taking actions, we'd better gather feedbacks from the community, especially from meson maintainers like @WillAyd. Meanwhile, there are prerequisite issues like #256 we need to resolve first.

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.

Keep CMake and Meson build definitions in sync

2 participants