build(deps): Align protobuf and related dependencies with the gRPC v1.81.1 bump#79
Conversation
mc-nv
changed the title
The patch worked around abseil/abseil-cpp#1769 and carried a TODO to remove it once the fix landed upstream. The abseil-cpp bundled with gRPC v1.81.1 (76bb243) includes that fix natively, and the patch no longer applies (FileNotMatchingError at CMakeLists.txt:214), breaking every tritonserver nightly build job. Also update the install_src dest-basename left at grpc_1.54.3 by the version bump.
The protobuf bundled with gRPC v1.81.1 has its CMakeLists.txt at the repository root; the cmake/ subdirectory now holds only helper modules. Configuring from cmake/ fails with 'does not appear to contain CMakeLists.txt'.
The protobuf bundled with gRPC v1.81.1 (v33.5) installs utf8_range as a separate CMake package under <prefix>/protobuf/lib/cmake/utf8_range, and protobuf-config.cmake now references the utf8_range::utf8_validity and utf8_range::utf8_range imported targets. Consumers that locate protobuf via Protobuf_DIR alone fail to configure with 'Could NOT find utf8_range (missing: utf8_range_DIR)'. Provide utf8_range_DIR to the grpc and google-cloud-cpp builds and add the directory to GCS_CMAKE_PREFIX_PATH.
The protobuf bundled with gRPC v1.81.1 is v33.5 (>= 3.22), and
opentelemetry-cpp hard-fails its configure when OTLP is enabled with a
protobuf >= 3.22 unless WITH_ABSEIL is ON ('Protobuf 3.22 or upper
require abseil-cpp'). Turn WITH_ABSEIL on, point absl_DIR at the absl
built from grpc-repo, and add absl to the project's dependencies.
The spanner library in google-cloud-cpp v2.28.0 does not compile against protobuf v33 (bundled with gRPC v1.81.1): protobuf's enum-name APIs now return absl::string_view, and spanner's proto_enum.h binds the result to 'const std::string&'. Triton only consumes google_cloud_cpp_storage (GCS filesystem support in core), so scope the build to the storage feature instead of the full GA library set — skipping the incompatible spanner code and shrinking the build.
WITH_FUNC_TESTS defaults to ON in opentelemetry-cpp v1.13.0 (independently of BUILD_TESTING) and the func_otlp_http test binary fails to link against protobuf v33: libprotobuf.a now requires the absl log/str_format libraries, which the functional-test link line does not carry. The otel libraries themselves build fine; skip the unneeded test binaries.
6e6353c to
2792fb2
Compare
| ${_CMAKE_ARGS_VCPKG_TARGET_TRIPLET} | ||
| -DBUILD_SHARED_LIBS:STRING=OFF | ||
| -DBUILD_TESTING:BOOL=OFF | ||
| -DWITH_FUNC_TESTS:BOOL=OFF |
There was a problem hiding this comment.
why change these flags? in the PR description i see:
opentelemetry-cpp: switch WITH_ABSEIL to ON against our absl install (adds absl_DIR and the absl dependency), and disable functional tests (WITH_FUNC_TESTS=OFF).
but why do we actually need to do this?
There was a problem hiding this comment.
It dictated by upstream configuration.
https://github.com/open-telemetry/opentelemetry-cpp/blob/v1.13.0/CMakeLists.txt#L382-L391
Greptile SummaryThis PR repairs the nightly build breakage caused by the gRPC v1.81.1 / protobuf v33 bump in #76 by updating dependency configuration throughout
Confidence Score: 4/5Safe to merge; all changes are targeted dependency configuration fixes validated by an internal CI pipeline, with two low-risk observations worth confirming before broad adoption. The changes are well-scoped: removing the obsolete patch, fixing the protobuf source dir, and plumbing utf8_range_DIR to the right consumers are straightforward mechanical fixes. Two things are worth a second look: opentelemetry-cpp does not receive utf8_range_DIR while grpc and google-cloud-cpp do, and restricting google-cloud-cpp to storage only would silently break any downstream that consumed other library features. Both are low-risk given CI validation. CMakeLists.txt — the opentelemetry-cpp block (utf8_range_DIR omission) and the google-cloud-cpp ENABLE=storage restriction. Important Files Changed
Reviews (1): Last reviewed commit: "fix: Disable opentelemetry-cpp functiona..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| -DWITH_FUNC_TESTS:BOOL=OFF | ||
| -DWITH_EXAMPLES:BOOL=OFF | ||
| -DWITH_BENCHMARK:BOOL=OFF | ||
| -DWITH_ABSEIL:BOOL=OFF | ||
| -DWITH_ABSEIL:BOOL=ON | ||
| -Dabsl_DIR:PATH=${TRITON_THIRD_PARTY_INSTALL_PREFIX}/absl/${LIB_DIR}/cmake/absl |
There was a problem hiding this comment.
utf8_range_DIR
With WITH_ABSEIL=ON, opentelemetry-cpp v1.13.0's CMake will call find_package(absl CONFIG) using the newly provided absl_DIR. If any of its internal cmake modules (e.g. for WITH_OTLP_HTTP) also invoke find_package(Protobuf CONFIG) — rather than relying solely on the raw Protobuf_LIBRARIES / Protobuf_INCLUDE_DIR variables — protobuf v33's package config will fail to locate utf8_range targets without a corresponding -Dutf8_range_DIR. The grpc and google-cloud-cpp targets both received this argument; opentelemetry-cpp did not. If CI passes in its current form this is benign, but adding -Dutf8_range_DIR:PATH=${_FINDPACKAGE_UTF8_RANGE_CONFIG_DIR} here would make the three consumers consistent.
| ${_CMAKE_ARGS_VCPKG_TARGET_TRIPLET} | ||
| -DCMAKE_CXX_STANDARD:STRING=${TRITON_MIN_CXX_STANDARD} | ||
| -DGOOGLE_CLOUD_CPP_DEPENDENCY_PROVIDER:STRING=package | ||
| -DGOOGLE_CLOUD_CPP_ENABLE:STRING=storage |
There was a problem hiding this comment.
GOOGLE_CLOUD_CPP_ENABLE=storage narrows previously built features
Previously this target built all default google-cloud-cpp components; restricting to storage means any Triton backend or downstream that links against google-cloud-cpp for BigTable, Pub/Sub, Spanner, or IAM will silently get a missing-target error at their own build time. Could you confirm that no Triton backends consume google-cloud-cpp features beyond storage?
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Are there any Triton backends or downstream consumers that use google-cloud-cpp features other than storage (e.g. bigtable, pubsub, spanner)?
3 participants
What does the PR do?
Fixes the nightly build fallout from the gRPC v1.81.1 / protobuf v33 bump (#76):
tools/abseil_no_rebuild.patch— the upstream abseil fix is integrated in gRPC v1.81.1, so the patch no longer applies; installed grpc source basename updated 1.54.3 → 1.81.1.cmake/subdirectory (protobuf v33 layout)._FINDPACKAGE_UTF8_RANGE_CONFIG_DIRand passutf8_range_DIRto protobuf consumers (grpc, google-cloud-cpp) plus the GCS CMake prefix path — protobuf v33's package config requires it.WITH_ABSEILto ON against our absl install (addsabsl_DIRand theabsldependency), and disable functional tests (WITH_FUNC_TESTS=OFF).GOOGLE_CLOUD_CPP_ENABLE=storage).Checklist
<commit_type>: <Title>Related PRs:
Where should the reviewer start?
CMakeLists.txt— grpc-repo patch removal, protobuf source dir,utf8_range_DIR, opentelemetry-cpp and google-cloud-cpp optionsTest plan:
Nightly build pipeline on internal GitLab CI.
Caveats:
None.
Background
The gRPC v1.81.1 / protobuf v33 update in #76 broke the nightly build across the Triton repos; this PR chain repairs it.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
- Resolves: TRI-1608
CI (internal): [#58489960](http://tritonserver.local/ci/pipelines/58489960)