Skip to content

Conversation

@Eyizoha
Copy link
Contributor

@Eyizoha Eyizoha commented Jan 15, 2026

Purpose

  • feat(third_party): Add checksum verification for downloaded dependencies
    • Avoid redundant downloads of dependency packages
  • fix(build): Set Protobuf_ROOT for ORC
    • Prevent ORC from potentially using system protoc during compilation, which could lead to mismatch between generated code and library
  • fix(build): Set the expected output path
    • Replace undefined path variables and unify artifact placement paths
  • feat(build): Add option to disable C++11 ABI
    • Facilitate integration for projects not using C++11 ABI
  • feat(build): Add build and package script
    • Provide a simple automated build and packaging script

Tests

None

API and Format

No impact on API or format

Documentation

No new features introduced

@lxy-9602 lxy-9602 requested a review from zjw1111 January 15, 2026 08:44
@zjw1111 zjw1111 requested a review from lucasfang January 16, 2026 02:22
Copy link

Copilot AI left a comment

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 improves the build system's reliability and flexibility by adding checksum verification for downloaded dependencies, fixing build configuration issues, and providing a convenient build/package script.

Changes:

  • Added checksum verification with automatic skip of already-downloaded dependencies to avoid redundant downloads
  • Fixed build configuration by setting Protobuf_ROOT for ORC, replacing undefined output path variables with proper CMAKE_* variables, and adding C++11 ABI control
  • Added an automated build and packaging script for simplified workflows

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
third_party/versions.txt Added package name variables for each dependency to support checksum verification and local file detection
third_party/download_dependencies.sh Implemented SHA256 checksum verification with multi-platform support and automatic skip logic for existing valid files
cmake_modules/ThirdpartyToolchain.cmake Added THIRDPARTY_DIR variable, local file fallback logic, Protobuf_ROOT for ORC, and C++11 ABI flag propagation to external projects
cmake_modules/BuildUtils.cmake Replaced undefined OUTPUT_PATH variables with proper CMAKE_*_OUTPUT_DIRECTORY variables for consistent artifact placement
build_and_package.sh New script providing automated build and packaging workflow with configurable options
CMakeLists.txt Added PAIMON_USE_CXX11_ABI option with validation preventing Lance/Lumina from being enabled when ABI is off, and added CMAKE_RUNTIME_OUTPUT_DIRECTORY definition
.gitignore Added output directory to ignore list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

fi
mkdir -p "$BUILD_DIR"
cd "$BUILD_DIR"
cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DCMAKE_INSTALL_PREFIX="$PACKAGE_DIR" $CMAKE_OPTIONS ..
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

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

The CMAKE_OPTIONS variable should be quoted when passed to cmake to properly handle options with spaces or special characters. Without quotes, word splitting could cause issues.

Copilot uses AI. Check for mistakes.
@zjw1111 zjw1111 merged commit 1df6b31 into alibaba:main Jan 23, 2026
8 checks passed
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.

4 participants