github-actions[bot] commented on code in PR #68268:
URL: https://github.com/apache/doris/pull/68268#discussion_r4059111247
##########
thirdparty/build-thirdparty.sh:
##########
@@ -1902,6 +1902,27 @@ build_nlohmann_json() {
"${BUILD_SYSTEM}" install
}
+build_google_cloud_cpp() {
+ check_if_source_exist "${GOOGLE_CLOUD_CPP_SOURCE}"
+ cd "${TP_SOURCE_DIR}/${GOOGLE_CLOUD_CPP_SOURCE}"
+
+ rm -rf "${BUILD_DIR}"
+ "${CMAKE_CMD}" -G "${GENERATOR}" -B "${BUILD_DIR}" \
+ -DCMAKE_BUILD_TYPE=Release \
+ -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
+ -DCMAKE_PREFIX_PATH="${TP_INSTALL_DIR}" \
+ -DCMAKE_POSITION_INDEPENDENT_CODE=ON \
+ -DBUILD_SHARED_LIBS=OFF \
Review Comment:
[P1] Preserve static libcurl's transitive link dependencies
With `BUILD_SHARED_LIBS=OFF`, these archive-only commands cannot detect an
incomplete final link interface. Doris builds curl 8.2.1 static-only with zlib
enabled, but supported CMake `FindCURL` versions use pkg-config only as a
search hint and create `CURL::libcurl` without its private libraries. Upstream
then exports `google-cloud-cpp::rest_internal -> CURL::libcurl`, while its
installed config never finds ZLIB. An eventual
`find_package(google_cloud_cpp_oauth2)` consumer can therefore fail with
unresolved zlib symbols even though this build succeeds. Please provide and
force a prefix-local CURL config target that carries all static transitive
dependencies (or an equivalent shim/patch), and add a minimal installed-target
link smoke check.
##########
thirdparty/build-thirdparty.sh:
##########
@@ -1902,6 +1902,27 @@ build_nlohmann_json() {
"${BUILD_SYSTEM}" install
}
+build_google_cloud_cpp() {
+ check_if_source_exist "${GOOGLE_CLOUD_CPP_SOURCE}"
+ cd "${TP_SOURCE_DIR}/${GOOGLE_CLOUD_CPP_SOURCE}"
+
+ rm -rf "${BUILD_DIR}"
+ "${CMAKE_CMD}" -G "${GENERATOR}" -B "${BUILD_DIR}" \
+ -DCMAKE_BUILD_TYPE=Release \
+ -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
+ -DCMAKE_PREFIX_PATH="${TP_INSTALL_DIR}" \
Review Comment:
[P1] Make the install prefix an actual lookup boundary
`CMAKE_PREFIX_PATH` only adds a preferred prefix; upstream v2.45.0 still
performs unconstrained Abseil, CURL, OpenSSL, and nlohmann-json lookups. CMake
checks package roots, environment package directories, registries, and system
prefixes too, so a selective or incomplete build can silently use host packages
instead of failing. That can produce ABI-incompatible artifacts and violates
`thirdparty/AGENTS.md`, which requires Doris-managed dependencies to resolve
only below the Doris third-party prefix. Please root package/library/include
lookup at `${TP_INSTALL_DIR}` (while leaving program lookup for host build
tools), or pin/patch each managed lookup with prefix-local paths and
`NO_DEFAULT_PATH` semantics.
--
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]
