deps: update protobuf to 32.0 #356
Conversation
WalkthroughThe PR regenerates protobuf code to 6.32.0, adding conditional has-bit consistency checks, normalizing unsigned literal suffixes, and annotating internal constructors with [[maybe_unused]]. Headers switch operator delete to ::std::destroying_delete_t; some map key/value types are fully qualified (::std::string). Protobuf build files are substantially reorganized. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Msg as Protobuf Message
participant Guard as DebugHarden flag
participant Ver as VerifyHasBitConsistency
participant Ser as Serializer
App->>Msg: _InternalSerialize()
Msg->>Guard: if constexpr(DebugHardenVerifyHasBitConsistency)
alt Harden enabled
Guard->>Ver: VerifyHasBitConsistency()
Ver-->>Msg: return
end
Msg->>Ser: Serialize fields (has_bits, tables)
Ser-->>App: bytes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
agents/grpc/src/proto/exit.pb.cc (1)
209-216: Generated code requires C++20, not just C++17The
operator delete(..., ::std::destroying_delete_t)in the generated*.pb.hheaders is a C++20-only feature wheneverPROTOBUF_CUSTOM_VTABLEis enabled. You must either bump the project’s minimum standard to C++20 or disable custom vtables to retain C++17 compatibility.Attention required in your build configurations:
- Update your GYP/Bazel/Makefile flags (e.g.
common_node.gypi,deps/grpc/.bcr/presubmit.yml,Makefile, etc.) to include-std=c++20.- Or explicitly disable
PROTOBUF_CUSTOM_VTABLEduring protobuf code generation to avoid pulling instd::destroying_delete_t.deps/protobuf/src/file_lists.cmake (1)
1189-1189: Remove misspelled entry infile_lists.cmakeThe typoged line is redundant—the correct
def_to_proto_weak_import_test.protois already listed just above. Remove the bad entry to prevent build failures.• File:
deps/protobuf/src/file_lists.cmake(around line 1189)Proposed diff:
--- a/deps/protobuf/src/file_lists.cmake +++ b/deps/protobuf/src/file_lists.cmake @@ -1189 - ${protobuf_SOURCE_DIR}/upb/util/def_to_proto_wweak_import_test.protoVerified that
upb/util/def_to_proto_weak_import_test.protoexists under the vendored subtree (deps/grpc/third_party/upb/upb/util).deps/protobuf/src/google/protobuf/BUILD.bazel (1)
1061-1122: Add missingprotobuf_test_proto_librarymacro
The BUILD rule atdeps/protobuf/src/google/protobuf/BUILD.bazelinvokesprotobuf_test_proto_library, but I couldn’t find anyprotobuf_test_proto_library.bzlunderbazel/tests(or elsewhere) defining that symbol. Without it, Bazel loads will fail.• Create
bazel/tests/protobuf_test_proto_library.bzl(or update the vendored macros) and export:def protobuf_test_proto_library(name, srcs, option_deps=None, strip_import_prefix=None, visibility=None, deps=None): # …implementation including option_deps handling…• Ensure your BUILD files load it correctly:
load("//bazel/tests:protobuf_test_proto_library.bzl", "protobuf_test_proto_library")
🧹 Nitpick comments (2)
deps/protobuf/src/file_lists.cmake (1)
1128-1158: Conformance test wiring: verify opt-inConformance targets are now listed. Ensure your build doesn’t unintentionally pull these into default build pipelines unless desired.
If these are not used in your build, consider guarding them in higher-level CMake to avoid needless build graph expansion.
agents/grpc/src/proto/info.pb.cc (1)
141-164: Descriptor tables updated — include only.pb.cc/.pb.h(exclude gRPC files) in the version‐banner check and run a quick smoke buildThe gRPC‐plugin outputs (
*.grpc.pb.cc/.grpc.pb.h) don’t include the “Protobuf C++ Version” banner, so narrow the grep to true protobuf-only files. Then verify all.pb.cc/.pb.hadvertise 6.32.0 and kick off a smoke build:# Check banner in .pb.cc/.pb.h (excluding *.grpc.pb.*) rg -nP --include='*.pb.cc' --include='*.pb.h' \ --glob='agents/grpc/src/proto/**/*.pb.*' \ 'Protobuf C\+\+ Version:\s*6\.32\.0' -S # List any .pb.cc/.pb.h missing the banner fd -e cc -e h '.*\.pb\.(cc|h)$' agents/grpc/src/proto \ | grep -v '\.grpc\.pb\.' \ | xargs -I{} sh -c \ "rg -q 'Protobuf C\\+\\+ Version:\\s*6\\.32\\.0' '{}' || echo '{}'" \ && echo "All true protobuf files advertise 6.32.0"• Ensure no
.pb.cc/.pb.h(outside of*.grpc.pb.*) are missing the banner
• Run a targeted smoke build on Linux/macOS/Windows to catch any ODR or header/runtime mismatches early
| ${protobuf_SOURCE_DIR}/upb/wire/decode_fast/select.c | ||
| ${protobuf_SOURCE_DIR}/upb/wire/encode.c | ||
| ${protobuf_SOURCE_DIR}/upb/wire/eps_copy_input_stream.c | ||
| ${protobuf_SOURCE_DIR}/upb/wire/internal/decode_fast.c | ||
| ${protobuf_SOURCE_DIR}/upb/wire/internal/decoder.c | ||
| ${protobuf_SOURCE_DIR}/upb/wire/reader.c | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Upb wire decoder/reader migration: verify files exist and no stale references remain
Swapping in:
- upb/wire/decode_fast/select.c
- upb/wire/internal/decoder.c
- upb/wire/reader.c
and removing the older upb/wire/internal/decode_fast.c aligns with upstream refactors. Please verify the new files exist in the vendored tree and that the removed path isn’t referenced elsewhere.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Verify new upb sources exist"
for f in \
'upb/wire/decode_fast/select.c' \
'upb/wire/internal/decoder.c' \
'upb/wire/reader.c' \
'upb/wire/internal/decoder.h' \
'upb/wire/internal/reader.h' \
'upb/wire/reader.h' \
'upb/wire/writer.h' \
'upb/wire/decode_fast/combinations.h' \
'upb/wire/decode_fast/data.h' \
'upb/wire/decode_fast/select.h'
do
if ! fd -a "$(basename "$f")" | rg -n "$f" -n -C0; then
echo "MISSING: $f"
fi
done
echo
echo "2) Ensure the removed file is not referenced"
if rg -n 'upb/wire/internal/decode_fast\.c' -g '!**/node_modules/**'; then
echo "Stale reference to decode_fast.c found."
exit 1
else
echo "OK: no stale references to decode_fast.c"
fiLength of output: 5197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Verify new upb sources/headers in protobuf vendored tree"
for f in \
"deps/protobuf/src/upb/wire/decode_fast/select.c" \
"deps/protobuf/src/upb/wire/internal/decoder.c" \
"deps/protobuf/src/upb/wire/reader.c" \
"deps/protobuf/src/upb/wire/internal/decoder.h" \
"deps/protobuf/src/upb/wire/internal/reader.h" \
"deps/protobuf/src/upb/wire/reader.h" \
"deps/protobuf/src/upb/wire/writer.h" \
"deps/protobuf/src/upb/wire/decode_fast/combinations.h" \
"deps/protobuf/src/upb/wire/decode_fast/data.h" \
"deps/protobuf/src/upb/wire/decode_fast/select.h"
do
if [ -f "$f" ]; then
echo "FOUND: $f"
else
echo "MISSING: $f"
fi
done
echo
echo "2) Stale references to decode_fast.c in protobuf vendored tree"
if rg -n 'upb/wire/internal/decode_fast\.c' -g 'deps/protobuf/**/*'; then
echo "Stale reference(s) found in protobuf vendored tree."
else
echo "No stale references in protobuf vendored tree."
fiLength of output: 6904
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate any vendored upb directories under deps/protobuf"
fd -t d upb deps/protobuf
echo
echo "2) Search for decode_fast files in the protobuf tree"
rg -n 'decode_fast' -g 'deps/protobuf/**/*' -C1
echo
echo "3) Search for reader.c in the protobuf tree"
rg -n 'reader\.c' -g 'deps/protobuf/**/*' -C1
echo
echo "4) Search for writer.h in the protobuf tree"
rg -n 'writer\.h' -g 'deps/protobuf/**/*' -C1Length of output: 7349
Add missing uPB source/header files & remove stale decode_fast.c references
The CMake list in deps/protobuf/src/file_lists.cmake now references the new uPB paths but those files aren’t present in the vendored tree, and the old upb/wire/internal/decode_fast.c is still referenced in the gRPC build files. You’ll need to:
• Vend the new uPB files under deps/protobuf/src/upb/wire/... so that CMake can find:
– decode_fast/select.c
– decode_fast/combinations.h
– decode_fast/data.h
– decode_fast/select.h
– internal/decoder.c & internal/decoder.h
– internal/reader.h
– reader.c & reader.h
– writer.h
• Remove any remaining references to the deleted upb/wire/internal/decode_fast.c in:
– deps/grpc/Makefile (line 1758)
– deps/grpc/grpc.gyp (line 1203)
This will align the vendored tree with the updated file_lists.cmake and eliminate build errors.
🤖 Prompt for AI Agents
In deps/protobuf/src/file_lists.cmake around lines 697-702 the CMake list
references new uPB source/header files that are not present in the vendored tree
and there are still stale references to upb/wire/internal/decode_fast.c in the
gRPC build files; add the missing files under deps/protobuf/src/upb/wire/
(decode_fast/select.c, decode_fast/combinations.h, decode_fast/data.h,
decode_fast/select.h, internal/decoder.c, internal/decoder.h, internal/reader.h,
reader.c, reader.h, writer.h) so CMake can locate them, and remove the remaining
reference to upb/wire/internal/decode_fast.c from deps/grpc/Makefile (line 1758)
and deps/grpc/grpc.gyp (line 1203).
| # @//pkg:protoc-gen-upb_minitable | ||
| set(protoc-gen-upb_minitable_srcs | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/code_generator.cc | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/code_generator_lite.cc | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/plugin.cc | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/plugin.pb.cc | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/retention.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/common.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/common/names.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/file_layout.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/minitable/generator.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/minitable/main.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/minitable/names.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/minitable/names_internal.cc | ||
| ${protobuf_SOURCE_DIR}/upb_generator/plugin.cc | ||
| ) | ||
|
|
||
| # @//pkg:protoc-gen-upb_minitable | ||
| set(protoc-gen-upb_minitable_hdrs | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/code_generator.h | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/code_generator_lite.h | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/plugin.h | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/plugin.pb.h | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/retention.h | ||
| ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/scc.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/common.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/common/names.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/file_layout.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/minitable/generator.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/minitable/names.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/minitable/names_internal.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/plugin.h | ||
| ${protobuf_SOURCE_DIR}/upb_generator/plugin_bootstrap.h | ||
| ) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New protoc-gen-upb_minitable source/header lists: sanity check
The new minitable generator groups look correct. Please confirm these sets map to actual source files in the vendor tree to avoid CMake file-not-found errors.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Verifying protoc-gen-upb_minitable files exist"
files=(
'upb_generator/minitable/generator.cc'
'upb_generator/minitable/main.cc'
'upb_generator/minitable/generator.h'
'upb_generator/minitable/names.cc'
'upb_generator/minitable/names_internal.cc'
)
for f in "${files[@]}"; do
if ! fd -a "$(basename "$f")" | rg -n "$f" -n -C0; then
echo "MISSING: $f"
fi
doneLength of output: 1537
Invalid file references in protoc-gen-upb_minitable lists
The following entries in deps/protobuf/src/file_lists.cmake (lines 904–938) do not exist in the protobuf vendor tree and will cause CMake “file not found” errors:
- upb_generator/minitable/generator.cc
- upb_generator/minitable/main.cc
- upb_generator/minitable/generator.h
- upb_generator/minitable/names.cc
- upb_generator/minitable/names_internal.cc
Please update or remove these paths so they exactly match the actual files under ${protobuf_SOURCE_DIR}/upb_generator/minitable.
🤖 Prompt for AI Agents
In deps/protobuf/src/file_lists.cmake around lines 904–938, the
protoc-gen-upb_minitable source/header lists reference several non-existent
files under ${protobuf_SOURCE_DIR}/upb_generator/minitable (generator.cc,
main.cc, generator.h, names.cc, names_internal.cc); update these entries to
exactly match the real filenames in that vendor directory or remove them if they
don’t exist: inspect ${protobuf_SOURCE_DIR}/upb_generator/minitable, replace
each incorrect path with the correct filename (and path) or delete the entry
from both protoc-gen-upb_minitable_srcs and protoc-gen-upb_minitable_hdrs so
CMake no longer references missing files.
PR-URL: #356 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
|
Landed in ef5a0be. |
PR-URL: #356 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Summary by CodeRabbit
Chores
Refactor
Tests