build(deps): Align protobuf and related dependencies with the gRPC v1.81.1 bump - #466
Conversation
The cc-clients and python-clients external projects clone the client repo but never pass a third-party tag, so the client build always uses the third_party default (main) regardless of what the surrounding build was pinned to. Add a TRITON_THIRD_PARTY_REPO_TAG cache variable and forward it to both client sub-builds.
mc-nv
changed the title
mc-nv
changed the title
protobuf v33's protobuf-config.cmake (pulled in transitively through gRPCConfig's find_dependency) requires the utf8_range package, whose config is installed inside the protobuf tree at <third-party>/protobuf/<libdir>/cmake/utf8_range. The third-party root prefix alone cannot resolve it, so add the protobuf install tree as an additional CMAKE_PREFIX_PATH entry.
TRITON_MIN_CXX_STANDARD (default 20) governs the third-party build, and
the abseil install pins its options.h to the standard used at build
time — with C++20 it selects the std:: ordering types. Only the
perf_analyzer executable requested cxx_std_20 via
target_compile_features; the client-backend object libraries compiled
at the compiler default (gnu++17) and failed against the pinned abseil
headers ('partial_ordering' has not been declared in 'std'). Set
CMAKE_CXX_STANDARD project-wide so every target matches the third-party
stack.
JsonPrintOptions::always_print_primitive_fields was removed in protobuf v33 (bundled with gRPC v1.81.1); use its designated replacement always_print_fields_with_no_presence. Also updated in the (currently disabled) TensorFlow Serving backend for when it returns.
Greptile SummaryThis PR aligns the perf_analyzer build system with the gRPC v1.81.1 / protobuf v33 third-party bump by fixing missing tag forwarding, a missing
Confidence Score: 5/5The changes are tightly scoped to build-system wiring and a well-understood API rename; the nightly CI pipeline confirms the build succeeds end-to-end. All four changes are mechanical and narrowly targeted: cache variable forwarding, a CMake prefix path addition, a C++ standard propagation, and a protobuf API rename that compiles cleanly. No runtime logic was altered beyond the JSON serialization option swap, and the semantic implications of that swap are already documented in a prior review thread. src/CMakeLists.txt — the TRITON_MIN_CXX_STANDARD guard has no fallback default, so a standalone inner-build invocation would silently skip the C++ standard setting. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Root["CMakeLists.txt (root)\nTRITON_THIRD_PARTY_REPO_TAG\nTRITON_MIN_CXX_STANDARD"]
Root -->|ExternalProject| CC["cc-clients\n(triton/client.git)\n+ TRITON_THIRD_PARTY_REPO_TAG ✅"]
Root -->|ExternalProject| PA["perf-analyzer\n(local src/)\n+ TRITON_MIN_CXX_STANDARD ✅"]
Root -->|ExternalProject| PY["python-clients\n(triton/client.git)\n+ TRITON_THIRD_PARTY_REPO_TAG ✅"]
CC -->|"builds & installs\nthird-party libs"| TP["TRITON_THIRD_PARTY_INSTALL_PREFIX\n(cc_clients/third-party/)"]
TP --> PA
PA -->|"src/CMakeLists.txt\nCMAKE_PREFIX_PATH"| PB["${INSTALL_PREFIX}\n${INSTALL_PREFIX}/protobuf ✅\n(utf8_range resolution)"]
PB -->|"find_package"| Proto["protobuf v33\ngRPC v1.81.1\nabseil (C++20)"]
PA -->|"CMAKE_CXX_STANDARD\n= TRITON_MIN_CXX_STANDARD (20)"| ABIFix["ABI aligned with\nabseil headers ✅"]
Reviews (2): Last reviewed commit: "fix: Replace removed protobuf always_pri..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| options.preserve_proto_field_names = true; | ||
| options.always_print_primitive_fields = true; | ||
| options.always_print_fields_with_no_presence = true; | ||
| ::google::protobuf::util::MessageToJsonString( | ||
| model_metadata_proto, &metadata, options); |
There was a problem hiding this comment.
always_print_fields_with_no_presence is the documented protobuf v33 replacement for always_print_primitive_fields, but the two options diverge for proto3 optional scalar fields and oneof members: the old option printed those at their default value regardless of whether they were explicitly set; the new option will omit them when they are not set (because optional and oneof fields carry explicit presence).
If any field in ModelMetadataResponse or ModelConfigResponse is a proto3 optional scalar (or is a oneof member), callers that previously received a full JSON object with default-zero values will now get a sparse object and may hit missing-key errors. The same applies to the TFS GetModelMetadataResponse site. Worth confirming against the actual .proto definitions that no optional/oneof scalars are relied upon at their default values.
4 participants
What does the PR do?
Fixes the nightly build against the gRPC v1.81.1 / protobuf v33 third_party bump:
TRITON_THIRD_PARTY_REPO_TAGcache variable and forward it to the cc-clients and python-clients ExternalProject builds. Previously the client sub-builds fell back to their default third_party tag and picked up a revision incompatible with the rest of the build.CMAKE_PREFIX_PATHentry so theutf8_rangepackage config nested inside it (required by protobuf v33's package config, pulled in transitively viagRPCConfig) resolves during perf-analyzer configure.CMAKE_CXX_STANDARDproject-wide fromTRITON_MIN_CXX_STANDARD(default 20). The abseil install pins its options to the standard used at build time (C++20 selects thestd::ordering types); the client-backend object libraries previously compiled at the compiler default (gnu++17) and failed against the pinned abseil headers.JsonPrintOptions::always_print_primitive_fieldswas removed — use its replacementalways_print_fields_with_no_presence(triton and TFS client backends).Checklist
<commit_type>: <Title>Related PRs:
Where should the reviewer start?
CMakeLists.txt—TRITON_THIRD_PARTY_REPO_TAGcache var and its forwarding to both sub-buildsTest plan:
Nightly build pipeline on internal GitLab CI.
Caveats:
None.
Background
The gRPC v1.81.1 / protobuf v33 update in triton-inference-server/third_party#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)