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 features files to the set of standard imports #295

Merged
merged 4 commits into from Apr 29, 2024

Conversation

jhump
Copy link
Member

@jhump jhump commented Apr 26, 2024

The "standard imports" (made available to compile operations using protocompile.WithStandardImports) are files that would be included with protoc, if a user were instead compiling with protoc.

As of v26.1, protoc now includes two new files: "goole/protobuf/cpp_features.proto" and "google/protobuf/java_features.proto". But these are not generated to Go code into packages in the Protobuf runtime, unlike all of the other well-known types. So, for these, we embed binary-encoded file descriptors.

Notably protoc does not include "google/protobuf/go_features.proto". However, that file is part of the Go Protobuf runtime, with its generated code being available via the google.golang.org/protobuf/types/gofeaturespb package. So we also make that file available via protocompile.WithStandardImports (even though protoc doesn't include it).

@jhump jhump requested a review from pkwarren April 26, 2024 01:27
@jhump jhump force-pushed the jh/features-as-standard-imports branch from 5a18cfd to 4014fb1 Compare April 26, 2024 01:42
@jhump jhump requested review from emcfarlane and removed request for pkwarren April 26, 2024 12:28
std_imports.go Outdated Show resolved Hide resolved
internal/featuresext/cpp_features.protoset Show resolved Hide resolved
@emcfarlane
Copy link
Contributor

How would we go about updating the protoset files and would other languages be included as they are added?

@jhump
Copy link
Member Author

jhump commented Apr 26, 2024

How would we go about updating the protoset files and would other languages be included as they are added?

To the first part: that's part of the make generate target. So updating to a new protoc (by editing the protoc version file) will automatically pull everything in and update everything (and check that it's updated in CI).

To the second: if they add new files to the core Protobuf SDK (that now includes cpp_features.proto and java_features.proto, then we'd want to add them here, too, just to mimic protoc's ability to provide these files without users having to explicitly provide them elsewhere. The Protobuf team says they would only include such files with the protoc download for languages whose code generation is built directly into protoc. (I chose to add go_features.proto here, even though it does not fall into that camp just because it is trivially available via the Protobuf Go runtime, and people using a Go library to process protos might be more likely to be using the Go features.)

Co-authored-by: Edward McFarlane <3036610+emcfarlane@users.noreply.github.com>
@jhump jhump merged commit 83dc971 into main Apr 29, 2024
8 checks passed
@jhump jhump deleted the jh/features-as-standard-imports branch April 29, 2024 14:41
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.

None yet

2 participants