connectrpc-build: add Config::emit_descriptor_set for gRPC server reflection#141
Open
christopherwxyz wants to merge 2 commits into
Open
connectrpc-build: add Config::emit_descriptor_set for gRPC server reflection#141christopherwxyz wants to merge 2 commits into
christopherwxyz wants to merge 2 commits into
Conversation
…er reflection compile() already parses a full FileDescriptorSet (protoc --include_imports, buf --as-file-descriptor-set, or a precompiled set), but it can only be read in via descriptor_set() — never written out. Build scripts that need those bytes to serve grpc.reflection.v1.ServerReflection (e.g. for grpcurl) must run protoc a second time with --descriptor_set_out --include_imports. Add the symmetric Config::emit_descriptor_set(name): write the descriptor set already computed during compile() to <out_dir>/<name> as wire-format bytes, ready to back server reflection. Works uniformly across all three descriptor sources (import closure already guaranteed); reuses write_if_changed; purely additive. Follow-up to anthropics/buffa#125 (buffa-build chose the embedded buffa-reflect pool and left the standalone-.bin case open). Tests: builder-state toggle + end-to-end emit/decode/round-trip against the echo.fds.bin fixture. cargo test -p connectrpc-build: 20 passed.
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
Author
|
recheck |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
connectrpc-build: add
Config::emit_descriptor_setfor gRPC Server ReflectionMotivation
Config::compile()already parses the.protoinputs into a fullFileDescriptorSet—run_protocis invoked with--include_imports,run_bufwith--as-file-descriptor-set, and the precompiled path is read as-is, so thedescriptor_bytesit decodes for codegen always carry the complete transitive import closure.Today that set can be read in (
Config::descriptor_set(path)) but never written out. A build script that needs those bytes — to servegrpc.reflection.v1.ServerReflectionto clients likegrpcurlviainclude_bytes!(concat!(env!("OUT_DIR"), "…"))— has to invokeprotoca second time with--descriptor_set_out --include_imports, duplicating work connectrpc-build already did and re-requiring aprotocinstall even when generation usedbufor a precompiled set.This adds the symmetric
Config::emit_descriptor_set(name): write the descriptor set connectrpc-build already computed to<out_dir>/<name>.This is the connectrpc-build counterpart of the buffa#125 follow-up the maintainers invited — buffa-build went with the embedded
buffa-reflectpool for in-process reflection, explicitly leaving open the "standalone.binfor tooling outside the Rust build" case. Emitting at the connectrpc-build layer is the natural home for that: the bytes are already in hand, with--include_importsguaranteed, for every descriptor source.API
Implementation (connectrpc-build/src/lib.rs)
Purely additive — no change to existing behavior; the field defaults to
None.Configstruct — new field:Config::new()— default:descriptor_set):compile(), right aftercreate_dir_all(&out_dir)(withdescriptor_bytesalready in scope from the descriptor-source match):It reuses the existing
write_if_changed(so it won't churn mtimes / trigger needless rebuilds), and works uniformly for theProtoc,Buf, andPrecompiledsources because each already produces an import-completedescriptor_bytes.Tests (included)
Two tests added to the existing
mod tests, mirroringcompile_precompiled_descriptor_set:config_emit_descriptor_set_toggle— builder-state check.emit_descriptor_set_writes_reflection_bin— end-to-end against theecho.fds.binfixture (hermetic, Precompiled source, no protoc): asserts the file is written,FileDescriptorSet::decode_from_slicesucceeds with a non-empty file list, and the bytes round-trip the source set exactly.Verified locally:
cargo build/clippy -p connectrpc-buildclean;cargo test -p connectrpc-build→ 20 passed, 0 failed.Notes
buffa-reflect's in-process pool — this is the wire-bytes path for serving reflection v1 to external clients.Follow-up to anthropics/buffa#125.