github-actions[bot] commented on code in PR #65304:
URL: https://github.com/apache/doris/pull/65304#discussion_r3594865882
##########
be/cmake/thirdparty.cmake:
##########
@@ -110,6 +110,10 @@ add_thirdparty(arrow_flight_sql LIB64)
add_thirdparty(arrow_dataset LIB64)
add_thirdparty(arrow_acero LIB64)
add_thirdparty(parquet LIB64)
+# liblance_c.a contains compiler_builtins cbrt symbols. Place libm before it
+# so the final linker resolves C math symbols from the system library first.
+add_thirdparty(lance_c LIB64 NOTADD)
+list(APPEND COMMON_THIRDPARTY m lance_c)
Review Comment:
`COMMON_THIRDPARTY` is linked before `glibc-compatibility`, so adding bare
`m` here puts system `-lm` ahead of the compatibility archive on every default
Linux link. That contradicts the explicit ordering invariant in
`be/CMakeLists.txt` requiring `glibc-compatibility` before `-lc -lm`: existing
Doris `pow`/`log`/`exp` references can now be satisfied by the build host's
libm before the corresponding compatibility objects are extracted,
reintroducing newer libm symbol versions into binaries intended for older
glibc. Please keep every `-lm` after the compatibility targets and solve
Lance's compiler-builtins collision without changing the global system-library
order.
##########
thirdparty/build-thirdparty.sh:
##########
@@ -2126,6 +2126,79 @@ build_paimon_cpp() {
echo "Paimon-cpp internal dependencies installed successfully"
}
+# lance-c
+build_lance_c() {
+ check_if_source_exist "${LANCE_C_SOURCE}"
+ cd "${TP_SOURCE_DIR}/${LANCE_C_SOURCE}"
+
+ rm -rf "${BUILD_DIR}"
+ mkdir -p "${BUILD_DIR}"
+
+ local cargo_bin="${LANCE_C_CARGO:-${CARGO:-cargo}}"
+ if ! command -v "${cargo_bin}" >/dev/null 2>&1; then
+ echo "cargo is required to build lance-c. Install Rust 1.91.0 or set
LANCE_C_CARGO."
+ exit 1
+ fi
+ if [[ ! -x "${TP_INSTALL_DIR}/bin/protoc" ]]; then
+ echo "protoc is required to build lance-c. Build protobuf first."
+ exit 1
+ fi
+
+ local cargo_env=(
+ "CARGO_BUILD_JOBS=${PARALLEL}"
+ "CARGO_TARGET_DIR=${PWD}/${BUILD_DIR}"
+ "PROTOC=${TP_INSTALL_DIR}/bin/protoc"
+ )
+ if command -v rustup >/dev/null 2>&1 && [[ -z "${RUSTUP_TOOLCHAIN}" ]];
then
+ if rustup toolchain list | grep -q '^1.91.0'; then
+ cargo_env+=("RUSTUP_TOOLCHAIN=1.91.0")
+ fi
+ fi
+
+ local required_rust_version="1.91.0"
+ local cargo_version
+ if ! cargo_version="$(env "${cargo_env[@]}" "${cargo_bin}" --version | awk
'{print $2}')"; then
+ echo "failed to get cargo version for lance-c. Install Rust
${required_rust_version} or set LANCE_C_CARGO/RUSTUP_TOOLCHAIN."
+ exit 1
+ fi
+ if ! awk -v required="${required_rust_version}" -v
actual="${cargo_version}" 'BEGIN {
+ split(required, r, ".");
+ split(actual, a, ".");
+ for (i = 1; i <= 3; i++) {
+ if ((a[i] + 0) > (r[i] + 0)) {
+ exit 0;
+ }
+ if ((a[i] + 0) < (r[i] + 0)) {
+ exit 1;
+ }
+ }
+ exit 0;
+ }'; then
+ echo "lance-c requires Rust/Cargo ${required_rust_version} or newer,
but found ${cargo_version}."
+ echo "Install Rust ${required_rust_version} or set
LANCE_C_CARGO/RUSTUP_TOOLCHAIN."
+ exit 1
+ fi
+
+ if [[ "${KERNEL}" != 'Darwin' ]]; then
+ cargo_env+=("CFLAGS=${CFLAGS:-} -std=gnu17")
Review Comment:
This `CFLAGS` value is consumed by cc-rs native dependencies; the locked
`lz4-sys` crate, for example, compiles four C sources through `cc::Build`. The
GCC 7 builder installs GCC 7.3 and now default-builds Lance-C, but GCC
documents `-std=gnu17` as a new option in GCC 8, so that path still fails after
Rust/Cargo is provisioned (the separate issue already raised in discussion
r3578107999). Please gate this flag by compiler support, use a compatible C
mode, or select a newer C compiler for Lance-C, and cover the intended
legacy-toolchain build.
##########
dist/LICENSE-dist.txt:
##########
@@ -1476,6 +1476,9 @@ The Apache Software License, Version 2.0
* opentelemetry-proto: 0.18.0
* opentelemetry-cpp: 1.4.0
* clucene: 2.4.6
+ * lance-c: 0.1.2
Review Comment:
The new static archive includes Apache DataFusion 52.4.0 and Apache Arrow
Object Store 0.12.5 (both are present in the locked inventory), and each
published crate carries a `NOTICE.txt` attribution. Apache-2.0 section 4(d)
requires those relevant attributions to accompany redistributed derivatives,
but neither `NOTICE.txt` nor `dist/NOTICE-dist.txt` mentions either project;
`build.sh` packages the former with Doris binaries. Please add these NOTICE
entries and audit the remaining locked crates for any additional NOTICE files
before distributing `liblance_c.a`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
