feat(cmake): migrate build utility modules - #4
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 CMake helper modules to standardize Paimon build options, compiler/linker flags, and sanitizer integration.
Changes:
- Introduces configurable build options (including sanitizers) and a build configuration summary.
- Adds a compiler/linker flags setup module including platform-specific warning levels and linker selection.
- Adds build utilities for creating shared/static libraries and registering unit tests, and wires in sanitizer flags via an interface target.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 17 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function(check_description_length name description) | ||
| foreach(description_line ${description}) | ||
| string(LENGTH ${description_line} line_length) | ||
| if(${line_length} GREATER 80) | ||
| message(FATAL_ERROR "description for ${name} contained a\n\ | ||
| line ${line_length} characters long!\n\ | ||
| (max is 80). Split it into more lines with semicolons") | ||
| endif() | ||
| endforeach() | ||
| endfunction() |
| macro(define_option name description default) | ||
| check_description_length(${name} ${description}) | ||
| list_join(description "\n" multiline_description) | ||
|
|
||
| option(${name} "${multiline_description}" ${default}) |
| macro(set_option_category name) | ||
| set(PAIMON_OPTION_CATEGORY ${name}) | ||
| list(APPEND "PAIMON_OPTION_CATEGORIES" ${name}) | ||
| endmacro() |
|
|
||
| option(${name} "${multiline_description}" ${default}) | ||
|
|
||
| list(APPEND "PAIMON_${PAIMON_OPTION_CATEGORY}_OPTION_NAMES" ${name}) |
| # Top level cmake dir | ||
| if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}") | ||
| #---------------------------------------------------------------------- | ||
| set_option_category("Compile and link") |
| if(LIB_INCLUDES) | ||
| target_include_directories(${LIB_NAME}_shared SYSTEM | ||
| PUBLIC ${ARG_EXTRA_INCLUDES}) | ||
| endif() |
| if(LIB_INCLUDES) | ||
| target_include_directories(${LIB_NAME}_static SYSTEM | ||
| PUBLIC ${ARG_EXTRA_INCLUDES}) | ||
| endif() |
| target_link_options(${LIB_NAME}_shared | ||
| PRIVATE | ||
| -Wl,--exclude-libs,ALL | ||
| -Wl,-Bsymbolic | ||
| -Wl,-z,defs | ||
| -Wl,--gc-sections) |
|
|
||
| add_library(paimon_sanitizer_flags INTERFACE) | ||
|
|
||
| if(PAIMON_USE_ASAN) |
leaves12138
left a comment
leaves12138
left a comment
There was a problem hiding this comment.
Thanks for migrating these CMake utility modules. I compared the files with the existing source snapshot and they are effectively the same migrated content.
One small blocker before merge: cmake_modules/san-config.cmake still has a non-standard license header, while the other newly added CMake modules use the standard ASF header. Please update san-config.cmake to the same ASF header style used by the other files in this PR.
Non-blocking note: some helpers reference build files that are expected to arrive in later migration PRs, such as build_support/run-test.sh and top-level CMake variables. That looks fine for a staged migration.
leaves12138
left a comment
leaves12138
left a comment
There was a problem hiding this comment.
Thanks for the update. The sanitizer config now uses the standard ASF header, and the migrated CMake utility modules match the source snapshot aside from the expected header normalization. LGTM.
Purpose
Linked issue: N/A
Migrate CMake build utility modules from Alibaba Paimon C++ repository into the Apache repository:
cmake_modules/BuildUtils.cmakecmake_modules/DefineOptions.cmakecmake_modules/SetupCxxFlags.cmakecmake_modules/san-config.cmakeNo extra dependency files were migrated. These files are listed in the source repository
LICENSEas Apache Arrow build system modules, so their existing Apache/Arrow attribution was preserved.Tests
python3 /home/jinli.zjw/.codex/skills/paimon-cpp-migrate/scripts/check_migration_batch.py --files cmake_modules/BuildUtils.cmake cmake_modules/DefineOptions.cmake cmake_modules/SetupCxxFlags.cmake cmake_modules/san-config.cmakegit diff --check --cachedcmake -P cmake_modules/BuildUtils.cmakecmake -P cmake_modules/DefineOptions.cmakeSetupCxxFlags.cmakeandsan-config.cmakerequire normal CMake project/configure context and are not script-mode standalone modules.API and Format
No API, storage format, or protocol changes.
Documentation
No user-facing documentation changes.
Generative AI tooling
Migrate-by: OpenAI Codex