Support feature resolution for Protobuf Editions. #473
Conversation
This file contains 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
|
Are they any concerns that need to be addressed? I can also have a pull request with all the changes for Protobuf Editions support at once if that is the preference. |
|
I was hoping @agrue would take the lead on this one :) I'm a bit distanced from protobufs these days (I know!) so it's harder for me to find the time to invest into a proper review. |
|
Sure, I will take a look. Just need to find the time. |
agrue
reviewed
Sep 30, 2024
agrue
reviewed
Sep 30, 2024
agrue
approved these changes
Sep 30, 2024
This should successfully generate code for these files. However, they do not have the correct semantics yet. `protoc` does not translate equivalent features supported in proto2 and proto3 into previous `FieldOptions` fields, and we must adjust the semantics according to what are specified in `FeatureSet` messages.
It will not accept actual Protobuf Editions files by restricting the supported edition to `LEGACY`. This restriction will be lifted once support for edition 2023 is complete.
chungyc
force-pushed
the
edition-features-incremental
branch
from
d50e7da to
0298168
Compare
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.
This implements Protobuf Editions feature resolution for the Haskell
protocplugin.This embeds the binary protobuf message that
protocoutputs for feature set defaults, which is deserialized at runtime. It is done this way since it does not seem sustainable to manually translate the protobuf message to Haskell code every time there is a new edition. It also seems to be the same approach that protocolbuffers/protobuf intends for use with custom code generators written in C++. This is not stored in a separate file with the binary protobuf message to avoid adding additional dependencies such as file-embed.Tests are not included in this pull request, but this code is tested in a subsequent change (https://github.com/chungyc/proto-lens/pull/8), which confirm that the feature resolution works by maintaining the same behavior for proto2 and proto3. This is due to the fact that tests cannot directly import these modules. Perhaps the modules in
app/should be turned into an internal library for unit tests, but that change is not for this one.This does not complete Protobuf Editions support, which requires two more changes: https://github.com/chungyc/proto-lens/pull/7 (control proto2 and proto3 behavior based on features, not syntax type), https://github.com/chungyc/proto-lens/pull/8 (override features in enclosed scopes)
For #468.