github-actions[bot] commented on code in PR #68268:
URL: https://github.com/apache/doris/pull/68268#discussion_r4063253479
##########
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 \
+ -DBUILD_TESTING=OFF \
+ -DGOOGLE_CLOUD_CPP_ENABLE=oauth2 \
+ -DGOOGLE_CLOUD_CPP_ENABLE_EXAMPLES=OFF \
+ -DGOOGLE_CLOUD_CPP_ENABLE_WERROR=OFF \
+ -DGOOGLE_CLOUD_CPP_WITH_MOCKS=OFF
+
+ "${CMAKE_CMD}" --build "${BUILD_DIR}" -j "${PARALLEL}"
+ "${CMAKE_CMD}" --install "${BUILD_DIR}" --prefix "${TP_INSTALL_DIR}"
Review Comment:
[P1] Make the standalone common package discover OpenSSL
This install publishes `google_cloud_cpp_common` as its own CMake package,
but upstream v2.45.0 exports `google-cloud-cpp::common` with public
`OpenSSL::Crypto` while its installed config finds only Threads and Abseil. A
fresh consumer that only calls `find_package(google_cloud_cpp_common CONFIG
REQUIRED)` and links `google-cloud-cpp::common` therefore fails at CMake
generation because `OpenSSL::Crypto` is undefined. Upstream's aggregate
verifier masks this by loading `rest_internal` later, which discovers OpenSSL.
This is distinct from the static libcurl closure: please patch the common
config to discover OpenSSL on non-Windows through the same prefix-confined
mechanism and add an isolated installed-common consumer smoke test.
##########
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 CoreFoundation in the macOS pkg-config closure
With this static install, common's pkg-config chain reaches pinned
`absl_time_zone`, but that module drops its Darwin CoreFoundation dependency.
Abseil places `-framework CoreFoundation` in the CMake target's `DEPS`; its
`.pc` generator only serializes `absl::...` dependencies and explicit
`LINKOPTS`, so `absl_time_zone.pc` omits the framework. This is observable
through the shipped common archive: `FormatRfc3339()` calls
`absl::UTCTimeZone()`, whose archive object also contains the Apple
CFTimeZone/CFString/CFRelease calls, leaving a macOS pkg-config consumer with
unresolved CoreFoundation symbols. The macOS jobs only install archives and the
version-only verifier never selects this member. Please preserve CoreFoundation
in the installed pkg-config metadata and smoke-link this path on macOS.
##########
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 \
+ -DBUILD_TESTING=OFF \
+ -DGOOGLE_CLOUD_CPP_ENABLE=oauth2 \
+ -DGOOGLE_CLOUD_CPP_ENABLE_EXAMPLES=OFF \
+ -DGOOGLE_CLOUD_CPP_ENABLE_WERROR=OFF \
+ -DGOOGLE_CLOUD_CPP_WITH_MOCKS=OFF
+
+ "${CMAKE_CMD}" --build "${BUILD_DIR}" -j "${PARALLEL}"
Review Comment:
[P1] Export StrFormat in common's pkg-config requirements
The installed `google_cloud_cpp_common.pc` omits a direct dependency needed
by this static archive. Upstream v2.45.0 links `google_cloud_cpp_common`
publicly to `absl::str_format`, and `InvocationIdGenerator::MakeInvocationId()`
calls `absl::StrFormat`, but the generated `.pc` requires `absl_strings` rather
than the separate `absl_str_format` module. In the pinned Abseil version,
`absl_strings` does not pull in StrFormat's compiled implementation, so a
pkg-config consumer that selects this archive member can fail at final link.
The upstream verifier misses this because it only calls `version_string()`.
Please add `absl_str_format` to common's pkg-config requirements and smoke-link
an API that exercises this archive member.
--
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]
