generate native Connect services and check declarations with buf - #18
Open
hugowetterberg wants to merge 2 commits into
Open
generate native Connect services and check declarations with buf#18hugowetterberg wants to merge 2 commits into
hugowetterberg wants to merge 2 commits into
Conversation
A service is now one of two shapes, and the layout says which. A declaration in the flat layout, <proto root>/<application>/service.proto, is dual stack and generates exactly what it generated before. A declaration whose own directory is a version is native: protoc-gen-go and protoc-gen-connect-go only, no adapters, no plain interface, no Twirp — and streaming methods allowed, which the other shape cannot have because protoc-gen-elephant-rpc and protoc-gen-twirp both fail generation on a stream. rpc.DualStack names the service directories that generate dual stack whatever their layout, for a legacy service that moves layout before its Twirp callers are gone. Discovery walks the proto root at any depth rather than matching two fixed globs, so a declaration nested under a package prefix is found: elephant.collab.v1 in rpc/elephant/collab/v1. vendor, node_modules, testdata and dot directories are never descended into, testdata for the reason the go command treats it as opaque — a plugin that keeps a fixture declaration would otherwise have Go generated into it. The buf module root moves from the repository root to the proto root, which is what PACKAGE_DIRECTORY_MATCH is checked against and what an import resolves against. For a repository whose sources live under rpc that renames the protobuf file, and the name reaches the generated Go, so the bump is breaking for those repositories and the CHANGELOG says what has to happen in the same commit. A repository whose proto root is the repository root, which is elephant-api, is unaffected and still needs no buf.yaml. rpc:stub scaffolds elephant.<app>.v1 into <proto root>/elephant/<app>/v1 and gives the service the Service suffix, so a fresh stub passes buf lint with the STANDARD rules and no exemptions. It did not before. rpc:lint, rpc:breaking, rpc:format and rpc:formatCheck run the pinned buf over the discovered services, so a vendored declaration is compiled as an import and never checked. rpc:breaking resolves the branch its git input names against the checkout it runs in and falls back to origin/<branch>, since a CI checkout has no local main; a shallow clone has neither, and the target says so rather than blaming a missing first commit.
The layout deciding the shape outright meant an existing service could only reach the native shape by moving to the versioned layout — which changes its proto package, and the proto package is in its procedure path. Adopting connect-go's own interface, and so streaming, was welded to breaking every caller's path. Retiring Twirp had the smaller version of the same problem: rpc.Twirp is repository-wide, so a repository holding six services could not take one of them off the /twirp/ paths without taking all six. A service now has one of three shapes. ShapeDualStack serves Connect and /twirp/ on the plain protobuf service interface, ShapeConnect serves Connect only on that same interface, and ShapeNative serves Connect only on connect-go's own handler interface and is the one shape that may declare a streaming method. The layout picks the default — flat is dual stack while Twirp is on and connect when it is off, versioned is native — and rpc.Shapes overrides it by service directory, in both directions. Naming a service there changes what it generates and nothing else: no directory moves, no package changes, no path changes. That replaces rpc.DualStack, which only went one way and is not in a release. The plugin list and the plugin options both follow the shape, so generation is one buf run per shape rather than two: which generator declares the plain service interface is protoc-gen-twirp for dual stack and protoc-gen-elephant-rpc for connect, and neither runs for native. Stale file removal follows the same rule per service, so changing a shape cleans up whichever declaration of the interface is no longer generated. rpc.ElephantRPCOptions may no longer set the plugin's interface option, since it applies to every service at once and can now only contradict a per-service shape. Generation refuses it and names rpc.Shapes instead. Nothing in the fleet set it.
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.
Summary
A service now has one of three shapes:
rpc.ShapeDualStack/twirp/rpc.ShapeConnectrpc.ShapeNativeOnly
ShapeNativemay declare a streaming method:protoc-gen-elephant-rpcandprotoc-gen-twirpboth fail generation on a stream, since the plain interface returns one response and has no room for one.The layout picks the default and
rpc.Shapesoverrides it per service. Flat layout defaults toShapeDualStackwhilerpc.Twirpis set andShapeConnectwhen it is not; versioned layout defaults toShapeNative.The override is not a corner case — it is how an existing service moves, and an earlier revision of this branch got it wrong by deriving the shape from the layout alone. A service's proto package is in its procedure path, and the versioned layout is what puts a version in the package, so under that rule an existing service could reach
ShapeNativeonly by moving directory: adopting connect-go's interface, and therefore streaming, was welded to breaking every caller's path. Retiring Twirp had the smaller version of the same problem, sincerpc.Twirpis repository-wide and elephant-api holds six services.Naming a service there changes what it generates and nothing else — no directory moves, no package changes, no path changes. Going native still changes the generated Go a caller compiles against, which is a library break a consumer adopts on its own schedule; the wire contract does not shift.
Generation is one buf run per shape, since the plugin list and the plugin options both follow the shape: which generator declares the plain service interface is
protoc-gen-twirpfor dual stack andprotoc-gen-elephant-rpcfor connect, and neither runs for native. Stale-file removal follows the same rule per service.rpc.ElephantRPCOptionsmay no longer set the plugin'sinterfaceoption — it applies to every service at once and can only contradict a per-service shape. Nothing in the fleet set it.Discovery now walks the proto root at any depth instead of matching two fixed globs, so a declaration nested under a package prefix is found —
elephant.collab.v1inrpc/elephant/collab/v1.vendor,node_modules,testdataand dot directories are never descended into;testdatafor the reason the go command treats it as opaque, since a plugin that keeps a fixture declaration would otherwise have Go generated into it.rpc:stubscaffoldselephant.<app>.v1into<proto root>/elephant/<app>/v1with theServicesuffix, so a fresh stub passesbuf lintwith the STANDARD rules and no exemptions. It did not before.rpc:lint,rpc:breaking,rpc:formatandrpc:formatCheckrun the pinned buf over the discovered services, so a vendored declaration is compiled as an import and never checked against rules belonging to the repository it came from.Breaking, for repositories whose protobuf sources live under
rpcThe buf module root moves from the repository root to the proto root, which is what
PACKAGE_DIRECTORY_MATCHis checked against and what animportresolves against. Two things follow, both to be dealt with in the commit that bumps this module:importinside a.protois now written relative to the proto root. Generation fails withimported file does not existuntil it is. No repository has an intra-repo import today.greeter/service.protorather thanrpc/greeter/service.proto, and that name is part of whatprotoc-gen-gowrites, so the descriptor variable is renamed and the embedded descriptor bytes inservice.pb.goandservice.twirp.gochange with it.That rename does not reach the service's callers. A file's name is independent of its
package, so message and service full names, the RPC paths and the encoding are all untouched, and the only exported symbol that moves is theFile_*descriptor variable — which nothing in the fleet references. The cost is a regeneration diff, not a release coordinated with anybody calling the service.Affected: newswire and elephant-collab, both of which carry
File_rpc_<app>_service_prototoday. idapo already generates the new naming and is unaffected, as is elephant-api, whose proto root is the repository root.rpc:breakingcannot pass on the bump commit of such a repository — the state it compares against has nobuf.yamland names its files from the repository root, so buf reads every declaration as deleted. The target says so rather than reporting the deletions, and the check has to be skipped once on that PR.Notes for reviewers
A CI job running
rpc:breakingneedsactions/checkoutwithfetch-depth: 0. The default shallow checkout has no localmain; the target falls back toorigin/<branch>where that exists, but a--depth 1clone has neither and fails with a message saying so.The CHANGELOG entry carries no PR references — there was no number when it was written.