feat(cmake): add find package modules - #7
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a set of alternative CMake Find*Alt.cmake modules (plus shared utilities) to locate common native dependencies via config packages first, then fall back to pkg-config / manual search, while exposing consistent imported targets/variables to the rest of the build.
Changes:
- Introduces
FindPackageUtils.cmakeand apaimon_find_target_headers()helper to derive header roots from imported targets. - Adds new
Find<Dep>Alt.cmakemodules for several third-party dependencies (zstd, glog, fmt, ZLIB, TBB, Snappy, RapidJSON, RE2, Protobuf, ORC, LZ4, GTest, Avro, Arrow). - Creates imported interface/unknown targets (e.g.,
zstd,fmt,glog,tbb, etc.) when upstream packages don’t provide the expected canonical target names.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set(options NO_DEFAULT_PATH) | ||
| set(one_value_args) | ||
| set(multi_value_args NAMES HINTS PATH_SUFFIXES) | ||
| cmake_parse_arguments(ARG | ||
| "${options}" | ||
| "${one_value_args}" | ||
| "${multi_value_args}" | ||
| ${ARGN}) |
| if(ARG_PATH_SUFFIXES) | ||
| list(APPEND _find_args PATH_SUFFIXES ${ARG_PATH_SUFFIXES}) | ||
| endif() | ||
| list(APPEND _find_args NO_DEFAULT_PATH) |
| if(_PAIMON_ORC_TARGET) | ||
| if(NOT TARGET orc::orc) | ||
| add_library(orc::orc INTERFACE IMPORTED) | ||
| target_link_libraries(orc::orc INTERFACE ${_PAIMON_ORC_TARGET}) | ||
| endif() |
| get_target_property(ORC_INCLUDE_DIR orc::orc INTERFACE_INCLUDE_DIRECTORIES) | ||
| set(ORCAlt_FOUND TRUE) | ||
| else() |
| include(FindPackageHandleStandardArgs) | ||
| find_package_handle_standard_args(glogAlt REQUIRED_VARS GLOG_INCLUDE_DIR) | ||
|
|
||
| if(glogAlt_FOUND AND NOT TARGET glog) | ||
| add_library(glog INTERFACE IMPORTED) | ||
| set_target_properties(glog PROPERTIES INTERFACE_INCLUDE_DIRECTORIES | ||
| "${GLOG_INCLUDE_DIR}") | ||
| target_link_libraries(glog INTERFACE ${_PAIMON_GLOG_TARGET}) | ||
| endif() |
| PROPERTIES IMPORTED_LOCATION "${RE2_LIBRARY}" | ||
| INTERFACE_INCLUDE_DIRECTORIES | ||
| "${RE2_INCLUDE_DIR}") | ||
| set(RE2_LIBRARIES "${RE2_LIBRARY}") |
| paimon_find_target_headers(ZLIB_INCLUDE_DIR | ||
| ZLIB::ZLIB | ||
| NAMES | ||
| zlib.h | ||
| HINTS | ||
| ${ZLIB_INCLUDE_DIRS} | ||
| ${_PAIMON_ZLIB_FIND_ARGS}) |
Co-authored-by: Socrates <suxiaogang223@icloud.com>
e8a748f to
b67820f
Compare
leaves12138
left a comment
leaves12138
left a comment
There was a problem hiding this comment.
Thanks for migrating the CMake find package modules. I compared all added modules with the source snapshot; they match the staged migration with ASF header normalization. The files have standard ASF headers, no line-ending/trailing-whitespace issues, parse cleanly via a lightweight cmake -P include smoke check, and the PR can merge cleanly into current main.
Non-blocking note: the existing Apache Arrow attribution in LICENSE already lists the earlier CMake build modules, but it does not list these new Find*Alt.cmake modules. I do not see an explicit third-party/adapted marker in these files, so I am not blocking on it, but we may want to keep the build-module attribution list in LICENSE consistent in a follow-up.
LGTM.
Purpose
Linked issue: N/A
Migrate CMake find-package modules from the Alibaba-origin C++ repository into the Apache repository. These modules provide alternate package discovery for Arrow, Avro, GTest, LZ4, ORC, Protobuf, RE2, RapidJSON, Snappy, TBB, ZLIB, fmt, glog, zstd, plus shared
FindPackageUtils.cmakehelpers.The migrated files keep their source paths under
cmake_modules/. Alibaba-owned headers were converted to the ASF CMake-style license header used in this repository. No files needed original third-party license preservation, and no requested files were skipped.Tests
python3 /home/jinli.zjw/.codex/skills/paimon-cpp-migrate/scripts/check_migration_batch.py --files ...git diff --cached --checkcmake -P cmake_modules/FindPackageUtils.cmakeAPI and Format
No public C++ API, storage format, or protocol changes.
Documentation
No user-facing documentation changes.
Generative AI tooling
Migrate-by: OpenAI Codex