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

Add support for editions #1031

Open
ouillie opened this issue Apr 18, 2024 · 4 comments
Open

Add support for editions #1031

ouillie opened this issue Apr 18, 2024 · 4 comments

Comments

@ouillie
Copy link

ouillie commented Apr 18, 2024

Editions are the future of protobuf syntax.

Currently, when you set edition = "2023" at the top of a .proto file, and run it through prost-build using protoc_arg("--experimental_editions"), it will panic on this line because the value of FileDescriptorProto.syntax is "editions". That's pretty much the extent of my investigation. I just figured I'd file this bug since support for editions doesn't seem to be tracked elsewhere.

This may sound like it's adding complexity to Prost, but it looks like it should actually simplify Prost once Prototiller is released, since that will effectively normalize all the different syntaxes and editions into a canonical representation using features, and Prost will only have to support that canonical representation in the long term. It would be a somewhat heavy refactor up front, though.

@caspermeijn
Copy link
Collaborator

I agree, editions are important. However, due to backwards compatibility of protoc itself, we need to keep supporting proto2 and proto3 for some time.

Are you willing to do the work to add this support?

@ouillie
Copy link
Author

ouillie commented Apr 22, 2024

That makes sense. I won't have bandwidth for a couple months, but I'm down to contribute, especially after Prototiller is released.

@mzabaluev
Copy link
Contributor

Up to spec support for editions requires at least the following:

@esrauchg
Copy link

esrauchg commented Mar 6, 2025

Drive by chiming in from someone on the Protobuf team at Google here.

Unfortunately open sourcing the prototiller tool is held up so we still don't have a date for that (sorry!), but in nearly all cases manually upgrading a file from Proto3 to Editions 2023 files shouldn't be too hard so there shouldn't be a very strong reason to wait for it.

In general, the vision of Edition2023 is simply that it can reunify the syntaxes Proto2 and Proto3 and set the groundwork for safe syntax evolution (the name is actually inspired by Rust Editions, with the same idea of always supporting interop between editions and tooling to safely upgrade files to the next edition). There's not really anything extra in Edition 2023 that wasn't already there in Proto2 and Proto3. The ideal is that any Proto2 or Proto3 file should be able to move to Edition 2023 and have it be perfectly behavior preserving way (basically: start with Proto3, change it to Edition-2023, and the right options at the top of the file the the Prost codegen should be exactly the same as it was).

Ideally Editions 2023 support should only involve identifying any conditionals in the code generation which check exactly "is syntax == Proto2 or Proto3?" to decide things like "should this field be in an Option<>?" and decide in a more generalized way to also support Edition 2023 (which supports all proto2 and proto3 behavior under one syntax, and so a check on syntax can't decide those things anymore).

There's some argument against doing any extra conformance-to-spec work for things like open-enums specifically related to Prost advertising support of Edition2023 inputs, since ideally users should be able to move their Proto3 file to Edition2023 files and still get the same Prost gencode if they want. This does have oddities that if your implementation has preexisting non-conformant behavior on open-vs-closed enums, if you 'fix' that behavior only on Edition2023 it will make it a breaking change for users to upgrade their .proto files, so the strategy we recommend is more "make Edition2023 behavior whatever the corresponding preexisting proto2/proto3 behavior was, then separately consider fixing nonconformant behavior of things like open enums on both Proto3 and Edition2023 together a broader "conformance to intended spec" topic.

I glanced at the Prost generator code, I only immediately see 2 conditionals against .syntax in code_generator.rs that would need to be fixed for it to handle Edition2023 inputs:
https://github.com/tokio-rs/prost/blob/master/prost-build/src/code_generator.rs#L1037

The main work that would need to happen for Prost to support Edition 2023 is for those 2 conditionals to honor Feature Resolution: this relates to the behavior where under Editions the idea is that you can set if fields should be treated as optional or not by default at any of the file > message > nested message > individual field level scopes and it applies downward within the scope. For every feature, we also publish the syntax-dependent defaults including for Proto2/Proto3, so the intended design is that you can have your code generator operate only use this feature resolution notion and you'll have support for all possible syntaxes (including easily extend to future ones if the file-level default of a given feature might change in eg Protobuf Edition 2025).

Protoc plugins written in C++ get this feature-scoping/cascading behavior for free since we implemented it in the descriptor.h that we publish, but code generators written in other languages that have done their own bootstrap on descriptor.proto (like Prost has) directly need to implement that. It generally amounts to a DFS to propagate the feature values downward in scopes, but it could e.g. a decision on a field could on demand traverse upwards until you find the narrowest scope that decides what the optionality of that field should be. Once you have that "feature cascading" behavior in place, I believe Prost supporting Edition 2023 is only like a ~3 line delta.

There's more info about Feature Resolution here:
https://protobuf.dev/editions/implementation/#:~:text=specific%20test%20cases-,Feature%20Resolution,-Editions%20use%20lexical&text=specific%20test%20cases-,Feature%20Resolution,-Editions%20use%20lexical

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

No branches or pull requests

4 participants