fix(build): Improve build system reliability and flexibility #55

Eyizoha · 2026-01-15T07:49:13Z

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

zjw1111 · 2026-01-15T08:58:53Z

build_and_package.sh

+    mkdir -p "$OUTPUT_DIR"
+    PACKAGE_DIR="$OUTPUT_DIR/$BUILD_NAME"
+    rm -rf "$PACKAGE_DIR"
+    mkdir -p "$PACKAGE_DIR/include" "$PACKAGE_DIR/lib"
+
+    cp -r include/* "$PACKAGE_DIR/include/"
+    cp -r "$BUILD_DIR"/$BUILD_TYPE/*.a "$PACKAGE_DIR/lib/"
+    cp -r "$BUILD_DIR"/$BUILD_TYPE/*.so "$PACKAGE_DIR/lib/"


you can add -DCMAKE_INSTALL_PREFIX=$OUTPUT_DIR in CMAKE_OPTIONS, and run make install after make -j to collect all include files and all .a/.so to $OUTPUT_DIR

zjw1111 · 2026-01-15T10:47:29Z

cmake_modules/ThirdpartyToolchain.cmake

        "-DZSTD_HOME=${ORC_ZSTD_ROOT}"
        "-DZLIB_HOME=${ORC_ZLIB_ROOT}"
        "-DPROTOBUF_HOME=${ORC_PROTOBUF_ROOT}"
+        "-DProtobuf_ROOT=${ORC_PROTOBUF_ROOT}"


Maybe it's already right with -DPROTOBUF_HOME. See orc's code, all PROTOBUF_HOME、PROTOBUF_ROOT、Protobuf_ROOT are linked to PROTOBUF_HOME.

Is there any bad case in your development environment, which set -DPROTOBUF_HOME is not ok, but set -DProtobuf_ROOT is ok?

Yes, in my development environment, the system has a higher incompatible version of protobuf installed (3.9.1). During the build process, ORC unexpectedly uses the system's protoc to generate code, but at the linking stage, it uses the project-specified version of libprotobuf, which leads to undefined reference errors.

Based on my analysis, the issue is that find_package(Protobuf CONFIG) in ORC's FindProtobuf.cmake will prioritize finding the system's protobuf and use the system's protoc. PROTOBUF_HOME is not among CMake's standard search variables, so if only PROTOBUF_HOME is set, find_package() will ignore it and search in system paths instead.

After I set Protobuf_ROOT, the problem was resolved. This issue should be reproducible if you install a higher version of protobuf on the system (e.g., 3.9.1).

Copilot

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.

Copilot · 2026-01-19T06:55:34Z

build_and_package.sh

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


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 · 2026-01-19T06:55:34Z

build_and_package.sh

+mkdir -p "$BUILD_DIR"
+cd "$BUILD_DIR"
+cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DCMAKE_INSTALL_PREFIX="$PACKAGE_DIR" $CMAKE_OPTIONS ..
+make -j"$(nproc 2>/dev/null || echo 4)"


The script uses 'nproc' without quoting in the make command. While the fallback to 4 is provided, it would be more robust to quote the entire command substitution to avoid potential issues.

Suggested change

make -j"$(nproc 2>/dev/null || echo 4)"

JOBS=$(nproc 2>/dev/null || echo 4)

make -j"$JOBS"

Copilot · 2026-01-19T06:55:35Z

third_party/download_dependencies.sh

+
+  echo "Downloading ${url} to ${out}..."
+  wget --continue --output-document="${out}" "${url}" || \
+    (echo "Failed to download ${url}" 1>&2; exit 1)


The error message says "Failed to download" but should be "Failed downloading" to match the original error message style and maintain consistency with the rest of the codebase.

Suggested change

(echo "Failed to download ${url}" 1>&2; exit 1)

(echo "Failed downloading ${url}" 1>&2; exit 1)

Copilot · 2026-01-19T06:55:35Z

third_party/download_dependencies.sh

+  fi
+
+  echo "Downloading ${url} to ${out}..."
+  wget --continue --output-document="${out}" "${url}" || \


The --quiet flag was removed from wget. This will make the download output more verbose. Consider whether this was intentional or if the --quiet flag should be retained for cleaner build logs.

Suggested change

wget --continue --output-document="${out}" "${url}" || \

wget --quiet --continue --output-document="${out}" "${url}" || \

Copilot · 2026-01-19T06:55:35Z

build_and_package.sh

+            BUILD_TYPE="release"
+            shift
+            ;;
+        -d|--debug)
+            BUILD_TYPE="debug"


The BUILD_TYPE variable should use proper case ('Release' or 'Debug') instead of lowercase ('release' or 'debug') to follow CMake conventions and ensure consistent behavior across all platforms and generators.

Copilot · 2026-01-19T06:55:35Z

third_party/download_dependencies.sh

+      openssl dgst -sha256 "${file}" | cut -d' ' -f2
+    else
+      # sha256sum and shasum have similar output format
+      ${checksum_cmd} "${file}" | cut -d' ' -f1


The unquoted expansion of checksum_cmd relies on word splitting to work correctly. While this works in this specific context, it's fragile and could break if the command path contains spaces. Consider using an array or restructuring the code to call commands directly for better robustness.

lucasfang · 2026-01-21T06:17:39Z

build_and_package.sh

@@ -0,0 +1,129 @@
+#!/usr/bin/env bash
+#
+# Copyright 2024-present Alibaba Inc.


2026-present

lucasfang · 2026-01-21T06:26:56Z

third_party/versions.txt


 PAIMON_ZLIB_BUILD_VERSION=1.3.1
 PAIMON_ZLIB_BUILD_SHA256_CHECKSUM=9a93b2b7dfdac77ceba5a558a580e74667dd6fede4585b91eefb60f03b72df23
+PAIMON_ZLIB_PKG_NAME=zlib-1.3.1.tar.gz


To reduce redundancy, consider deriving *_PKG_NAME from *_BUILD_VERSION (e.g., zlib- $ {PAIMON_ZLIB_BUILD_VERSION}.tar.gz) instead of hardcoding the version twice？

Eyizoha added 5 commits January 14, 2026 19:55

feat(third_party): Add checksum verification for downloaded dependencies

f1c42e9

fix(build): Set Protobuf_ROOT for ORC

c5d9dab

fix(build): Set the expected output path

921b80f

feat(build): Add option for disable C++11 ABI

cbe5ced

feat(build): Add build and package script

fc3ba9a

lxy-9602 requested a review from zjw1111 January 15, 2026 08:44

zjw1111 requested a review from lucasfang January 16, 2026 02:22

zjw1111 reviewed Jan 16, 2026

View reviewed changes

fix(build): package use make install

47fb7e3

lucasfang requested a review from Copilot January 19, 2026 06:50

Copilot started reviewing on behalf of lucasfang January 19, 2026 06:50 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

Eyizoha added 2 commits January 19, 2026 17:05

fix(build): make copilot happy

e5526f7

fix: license & format

df47a0e

lucasfang reviewed Jan 21, 2026

View reviewed changes

Eyizoha and others added 2 commits January 21, 2026 14:55

fix review

d50fecd

Merge branch 'main' into dev

26542d8

lucasfang reviewed Jan 21, 2026

View reviewed changes

Eyizoha and others added 2 commits January 21, 2026 19:32

fix review

888840f

Merge branch 'main' into dev

b9b4976

	make -j"$(nproc 2>/dev/null \|\| echo 4)"
	JOBS=$(nproc 2>/dev/null \|\| echo 4)
	make -j"$JOBS"

	(echo "Failed to download ${url}" 1>&2; exit 1)
	(echo "Failed downloading ${url}" 1>&2; exit 1)

	wget --continue --output-document="${out}" "${url}" \|\| \
	wget --quiet --continue --output-document="${out}" "${url}" \|\| \

fix(build): Improve build system reliability and flexibility #55

Are you sure you want to change the base?

fix(build): Improve build system reliability and flexibility #55

Uh oh!

Conversation

Eyizoha commented Jan 15, 2026

Purpose

Tests

API and Format

Documentation

Uh oh!

zjw1111 Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Eyizoha Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

zjw1111 Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eyizoha Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

lucasfang Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Eyizoha Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

lucasfang Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Eyizoha Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zjw1111 Jan 15, 2026 •

edited

Loading