Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support protobuf editions, allow edition 2023 #403

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lstoll
Copy link

@lstoll lstoll commented Dec 25, 2024

Protobuf is moving to editions as a replacement for the syntax definition, and has defined a 2023 edition that is the successor to proto2/proto3 syntax. Go protobuf got a new Opaque API as well, that is tied to edition 2023.

Tooling needs to declare support for both editions, and what editions are supported. Currently protoc-gen-twirp does not, so using a 2023 edition file results in:

xxxx.proto: is an editions file, but code generator protoc-gen-twirp hasn't been updated to support editions yet. Please ask the owner of this code generator to add support or switch back to proto2/proto3.

This resolves that by declaring that the plugin supports editions. The max supported edition is set to 2023, the latest released. This can be incremented as needed, once it's ensured that the new edition doesn't introduce any issues or breaking changes.

From a Twirp perspective, edition 2023 appears to introduce nothing that needs to be handled in its generated code. The service definitions are untouched, just message fields. Twirp doesn't interact with these directly.

I waffled a bit on testing this. Like in #332, nothing that Twirp interacts with directly changes. We could maybe add a second 2023 edition proto definition to the clientcompat tests and switch between them with build tags or something, but I think that would be testing protobuf's ability to serialize/deserialize more than Twirp itself. Happy to try that / react to feedback though.

The example was also left as proto3, as editions support is fairly new and I didn't want to force anyone using that to have to update immediately.

Note: This PR requires the updates in #402 to function.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Flag the generator that editions are supported, and set the max edition to 2023
(the current version). This allows us to verify if newer editions have changes
that affect this tool.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant