[fix](paimon) install paimon-cpp arrow static deps into isolated dir#60730
Open
xylaaaaa wants to merge 5 commits intoapache:masterfrom
Open
[fix](paimon) install paimon-cpp arrow static deps into isolated dir#60730xylaaaaa wants to merge 5 commits intoapache:masterfrom
xylaaaaa wants to merge 5 commits intoapache:masterfrom
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
There was a problem hiding this comment.
Pull request overview
Installs paimon-cpp’s Arrow static library dependencies into an isolated directory to avoid conflicting with Doris’ Arrow build.
Changes:
- Adds an isolated
paimon_depsdirectory under the paimon-cpp install prefix. - Copies a specific set of Arrow/Parquet static libraries into that isolated directory when present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| libarrow_acero.a \ | ||
| libparquet.a; do | ||
| if [ -f "arrow_ep-install/lib/${paimon_arrow_dep}" ]; then | ||
| cp -v "arrow_ep-install/lib/${paimon_arrow_dep}" "${paimon_deps_dir}/${paimon_arrow_dep}" |
There was a problem hiding this comment.
This loop silently skips missing static libs, which can turn into harder-to-debug link failures later (especially if Arrow’s built targets differ by version/options). If these archives are required, fail fast when any expected file is missing (e.g., emit an error and return non-zero). If they’re optional, at least log an explicit warning per missing library so the build output explains what happened.
Suggested change
| cp -v "arrow_ep-install/lib/${paimon_arrow_dep}" "${paimon_deps_dir}/${paimon_arrow_dep}" | |
| cp -v "arrow_ep-install/lib/${paimon_arrow_dep}" "${paimon_deps_dir}/${paimon_arrow_dep}" | |
| else | |
| echo "Warning: expected Arrow static library 'arrow_ep-install/lib/${paimon_arrow_dep}' not found; paimon-cpp static linking may fail if this archive is required." >&2 |
Comment on lines
+2033
to
2048
| local paimon_deps_dir="${TP_INSTALL_DIR}/paimon-cpp/lib64/paimon_deps" | ||
| mkdir -p "${paimon_deps_dir}" | ||
| for paimon_arrow_dep in \ | ||
| libarrow.a \ | ||
| libarrow_filesystem.a \ | ||
| libarrow_dataset.a \ | ||
| libarrow_acero.a \ | ||
| libparquet.a; do | ||
| if [ -f "arrow_ep-install/lib/${paimon_arrow_dep}" ]; then | ||
| cp -v "arrow_ep-install/lib/${paimon_arrow_dep}" "${paimon_deps_dir}/${paimon_arrow_dep}" | ||
| fi | ||
| done | ||
|
|
||
| # Install roaring_bitmap, renamed to avoid conflict with Doris's croaringbitmap | ||
| if [ -f "release/libroaring_bitmap.a" ]; then | ||
| cp -v "release/libroaring_bitmap.a" "${TP_INSTALL_DIR}/lib64/libroaring_bitmap_paimon.a" |
There was a problem hiding this comment.
The paths hardcode both the destination libdir (lib64) and the Arrow EP libdir (arrow_ep-install/lib). This makes the logic brittle if the build outputs to lib64 (or a different layout) in some environments/configurations. Consider deriving the lib directory from existing build variables (or probing arrow_ep-install/lib64 vs .../lib) and using a single computed arrow_lib_dir/paimon_lib_dir to avoid repeated assumptions.
Suggested change
| local paimon_deps_dir="${TP_INSTALL_DIR}/paimon-cpp/lib64/paimon_deps" | |
| mkdir -p "${paimon_deps_dir}" | |
| for paimon_arrow_dep in \ | |
| libarrow.a \ | |
| libarrow_filesystem.a \ | |
| libarrow_dataset.a \ | |
| libarrow_acero.a \ | |
| libparquet.a; do | |
| if [ -f "arrow_ep-install/lib/${paimon_arrow_dep}" ]; then | |
| cp -v "arrow_ep-install/lib/${paimon_arrow_dep}" "${paimon_deps_dir}/${paimon_arrow_dep}" | |
| fi | |
| done | |
| # Install roaring_bitmap, renamed to avoid conflict with Doris's croaringbitmap | |
| if [ -f "release/libroaring_bitmap.a" ]; then | |
| cp -v "release/libroaring_bitmap.a" "${TP_INSTALL_DIR}/lib64/libroaring_bitmap_paimon.a" | |
| # Determine paimon-cpp library directory (lib vs lib64) | |
| local paimon_lib_root="${TP_INSTALL_DIR}/paimon-cpp" | |
| local paimon_lib_dir="${paimon_lib_root}/lib" | |
| if [ ! -d "${paimon_lib_dir}" ]; then | |
| paimon_lib_dir="${paimon_lib_root}/lib64" | |
| fi | |
| local paimon_deps_dir="${paimon_lib_dir}/paimon_deps" | |
| mkdir -p "${paimon_deps_dir}" | |
| # Determine Arrow EP library directory (lib vs lib64) | |
| local arrow_lib_dir="arrow_ep-install/lib" | |
| if [ ! -d "${arrow_lib_dir}" ]; then | |
| arrow_lib_dir="arrow_ep-install/lib64" | |
| fi | |
| for paimon_arrow_dep in \ | |
| libarrow.a \ | |
| libarrow_filesystem.a \ | |
| libarrow_dataset.a \ | |
| libarrow_acero.a \ | |
| libparquet.a; do | |
| if [ -f "${arrow_lib_dir}/${paimon_arrow_dep}" ]; then | |
| cp -v "${arrow_lib_dir}/${paimon_arrow_dep}" "${paimon_deps_dir}/${paimon_arrow_dep}" | |
| fi | |
| done | |
| # Install roaring_bitmap, renamed to avoid conflict with Doris's croaringbitmap | |
| # Determine top-level TP lib directory (lib vs lib64) | |
| local tp_lib_dir="${TP_INSTALL_DIR}/lib" | |
| if [ ! -d "${tp_lib_dir}" ]; then | |
| tp_lib_dir="${TP_INSTALL_DIR}/lib64" | |
| fi | |
| if [ -f "release/libroaring_bitmap.a" ]; then | |
| cp -v "release/libroaring_bitmap.a" "${tp_lib_dir}/libroaring_bitmap_paimon.a" |
| libarrow_dataset.a \ | ||
| libarrow_acero.a \ | ||
| libparquet.a; do | ||
| if [ -f "arrow_ep-install/lib/${paimon_arrow_dep}" ]; then |
There was a problem hiding this comment.
The paths hardcode both the destination libdir (lib64) and the Arrow EP libdir (arrow_ep-install/lib). This makes the logic brittle if the build outputs to lib64 (or a different layout) in some environments/configurations. Consider deriving the lib directory from existing build variables (or probing arrow_ep-install/lib64 vs .../lib) and using a single computed arrow_lib_dir/paimon_lib_dir to avoid repeated assumptions.
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 28870 ms |
TPC-DS: Total hot run time: 185466 ms |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
morningman
previously approved these changes
Feb 13, 2026
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 28801 ms |
TPC-DS: Total hot run time: 184445 ms |
Contributor
|
run buildall |
github-actions
bot
removed
the
approved
Contributor
|
run buildall |
TPC-H: Total hot run time: 28782 ms |
TPC-DS: Total hot run time: 183686 ms |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
followup #60296
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)
Related thirdparty PR: #60296