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 recently added "source_file_descriptors" field of CodeGeneratorRequest #2792

Merged
merged 11 commits into from
Mar 7, 2024
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,8 @@ issues:
- paralleltest
text: "missing the call to method parallel"
path: private/pkg/git/repository_test.go
# This greatly simplifies creation of descriptors, and it's safe enough since
# it's just test code.
- linters:
- forcetypeassert
path: private/bufpkg/bufimage/source_retention_options_test\.go
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

## [Unreleased]

- No changes yet.
- Update `buf generate` so it populates the recently-added
[`source_file_descriptors`](https://github.com/protocolbuffers/protobuf/blob/v24.0/src/google/protobuf/compiler/plugin.proto#L96-L99)
field of the `CodeGeneratorRequest` message. This provides the plugin with access to options
that are configured to only be retained in source and not at runtime (via
[field option](https://github.com/protocolbuffers/protobuf/blob/v24.0/src/google/protobuf/descriptor.proto#L693-L702)).
Descriptors in the `proto_file` field will not include any options configured this way.

## [v1.29.0] - 2024-01-24

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240221180331-f05a6f4403ce.1
connectrpc.com/connect v1.15.0
connectrpc.com/otelconnect v0.7.0
github.com/bufbuild/protocompile v0.8.0
github.com/bufbuild/protocompile v0.9.0
github.com/bufbuild/protovalidate-go v0.6.0
github.com/bufbuild/protoyaml-go v0.1.8
github.com/docker/docker v25.0.3+incompatible
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc
github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
github.com/bufbuild/protocompile v0.8.0 h1:9Kp1q6OkS9L4nM3FYbr8vlJnEwtbpDPQlQOVXfR+78s=
github.com/bufbuild/protocompile v0.8.0/go.mod h1:+Etjg4guZoAqzVk2czwEQP12yaxLJ8DxuqCJ9qHdH94=
github.com/bufbuild/protocompile v0.9.0 h1:DI8qLG5PEO0Mu1Oj51YFPqtx6I3qYXUAhJVJ/IzAVl0=
github.com/bufbuild/protocompile v0.9.0/go.mod h1:s89m1O8CqSYpyE/YaSGtg1r1YFMF5nLTwh4vlj6O444=
github.com/bufbuild/protovalidate-go v0.6.0 h1:Jgs1kFuZ2LHvvdj8SpCLA1W/+pXS8QSM3F/E2l3InPY=
github.com/bufbuild/protovalidate-go v0.6.0/go.mod h1:1LamgoYHZ2NdIQH0XGczGTc6Z8YrTHjcJVmiBaar4t4=
github.com/bufbuild/protoyaml-go v0.1.8 h1:X9QDLfl9uEllh4gsXUGqPanZYCOKzd92uniRtW2OnAQ=
Expand Down
23 changes: 12 additions & 11 deletions private/buf/bufgen/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,22 +272,23 @@ func (g *generator) execLocalPlugin(
if err != nil {
return nil, err
}
generateOptions := []bufpluginexec.GenerateOption{
bufpluginexec.GenerateWithPluginPath(pluginConfig.Path...),
bufpluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath),
requests, err := bufimage.ImagesToCodeGeneratorRequests(
pluginImages,
pluginConfig.Opt,
nil,
includeImports,
includeWellKnownTypes,
)
if err != nil {
return nil, err
}
response, err := g.pluginexecGenerator.Generate(
ctx,
container,
pluginConfig.PluginName(),
bufimage.ImagesToCodeGeneratorRequests(
pluginImages,
pluginConfig.Opt,
nil,
includeImports,
includeWellKnownTypes,
),
generateOptions...,
requests,
bufpluginexec.GenerateWithPluginPath(pluginConfig.Path...),
bufpluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath),
)
if err != nil {
return nil, fmt.Errorf("plugin %s: %v", pluginConfig.PluginName(), err)
Expand Down
18 changes: 11 additions & 7 deletions private/buf/cmd/buf/command/alpha/protoc/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ func executePlugin(
storageosProvider,
runner,
)
requests, err := bufimage.ImagesToCodeGeneratorRequests(
images,
strings.Join(pluginInfo.Opt, ","),
bufpluginexec.DefaultVersion,
false,
false,
)
if err != nil {
return nil, err
}
var options []bufpluginexec.GenerateOption
if pluginInfo.Path != "" {
options = append(options, bufpluginexec.GenerateWithPluginPath(pluginInfo.Path))
Expand All @@ -64,13 +74,7 @@ func executePlugin(
ctx,
container,
pluginName,
bufimage.ImagesToCodeGeneratorRequests(
images,
strings.Join(pluginInfo.Opt, ","),
bufpluginexec.DefaultVersion,
false,
false,
),
requests,
options...,
)
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion private/buf/cmd/protoc-gen-buf-lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,5 +342,7 @@ func testBuildCodeGeneratorRequest(
}
image, err := bufimage.NewImage(imageFiles)
require.NoError(t, err)
return bufimage.ImageToCodeGeneratorRequest(image, parameter, nil, false, false)
codeGenReq, err := bufimage.ImageToCodeGeneratorRequest(image, parameter, nil, false, false)
require.NoError(t, err)
return codeGenReq
}
12 changes: 8 additions & 4 deletions private/bufpkg/bufimage/bufimage.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ func ImageToCodeGeneratorRequest(
compilerVersion *pluginpb.Version,
includeImports bool,
includeWellKnownTypes bool,
) *pluginpb.CodeGeneratorRequest {
) (*pluginpb.CodeGeneratorRequest, error) {
return imageToCodeGeneratorRequest(
image,
parameter,
Expand All @@ -465,7 +465,7 @@ func ImagesToCodeGeneratorRequests(
compilerVersion *pluginpb.Version,
includeImports bool,
includeWellKnownTypes bool,
) []*pluginpb.CodeGeneratorRequest {
) ([]*pluginpb.CodeGeneratorRequest, error) {
requests := make([]*pluginpb.CodeGeneratorRequest, len(images))
// alreadyUsedPaths is a map of paths that have already been added to an image.
//
Expand Down Expand Up @@ -499,7 +499,8 @@ func ImagesToCodeGeneratorRequests(
}
}
for i, image := range images {
requests[i] = imageToCodeGeneratorRequest(
var err error
requests[i], err = imageToCodeGeneratorRequest(
image,
parameter,
compilerVersion,
Expand All @@ -508,8 +509,11 @@ func ImagesToCodeGeneratorRequests(
alreadyUsedPaths,
nonImportPaths,
)
if err != nil {
return nil, err
}
}
return requests
return requests, nil
}

// ImageModuleDependency is a dependency of an image.
Expand Down
85 changes: 71 additions & 14 deletions private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ func TestBasic(t *testing.T) {
newImage.Files(),
)
diff := cmp.Diff(protoImage, bufimage.ImageToProtoImage(newImage), protocmp.Transform())
require.Equal(t, "", diff)
require.Empty(t, diff)
fileDescriptorSet := &descriptorpb.FileDescriptorSet{
File: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
Expand All @@ -467,7 +467,7 @@ func TestBasic(t *testing.T) {
},
}
diff = cmp.Diff(fileDescriptorSet, bufimage.ImageToFileDescriptorSet(image), protocmp.Transform())
require.Equal(t, "", diff)
require.Empty(t, diff)
codeGeneratorRequest := &pluginpb.CodeGeneratorRequest{
ProtoFile: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
Expand All @@ -486,21 +486,32 @@ func TestBasic(t *testing.T) {
"b/b.proto",
"d/d.proto/d.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
},
}
actualRequest, err := bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false)
require.NoError(t, err)
diff = cmp.Diff(
codeGeneratorRequest,
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false),
actualRequest,
protocmp.Transform(),
)
require.Equal(t, "", diff)
require.Empty(t, diff)

// verify that includeWellKnownTypes is a no-op if includeImports is false
actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true)
require.NoError(t, err)
diff = cmp.Diff(
codeGeneratorRequest,
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true),
actualRequest,
protocmp.Transform(),
)
require.Equal(t, "", diff)
require.Empty(t, diff)

codeGeneratorRequestIncludeImports := &pluginpb.CodeGeneratorRequest{
ProtoFile: []*descriptorpb.FileDescriptorProto{
Expand All @@ -522,13 +533,23 @@ func TestBasic(t *testing.T) {
"b/b.proto",
"d/d.proto/d.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
testProtoImageFileToFileDescriptorProto(protoImageFileImport),
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
},
}
actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false)
require.NoError(t, err)
diff = cmp.Diff(
codeGeneratorRequestIncludeImports,
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false),
actualRequest,
protocmp.Transform(),
)
require.Equal(t, "", diff)
require.Empty(t, diff)
newImage, err = bufimage.NewImageForCodeGeneratorRequest(codeGeneratorRequest)
require.NoError(t, err)
AssertImageFilesEqual(
Expand Down Expand Up @@ -564,13 +585,24 @@ func TestBasic(t *testing.T) {
"b/b.proto",
"d/d.proto/d.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
testProtoImageFileToFileDescriptorProto(protoImageFileImport),
testProtoImageFileToFileDescriptorProto(protoImageFileWellKnownTypeImport),
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
},
}
actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true)
require.NoError(t, err)
diff = cmp.Diff(
codeGeneratorRequestIncludeImportsAndWellKnownTypes,
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true),
actualRequest,
protocmp.Transform(),
)
require.Equal(t, "", diff)
require.Empty(t, diff)
// imagesByDir and multiple Image tests
imagesByDir, err := bufimage.ImageByDir(image)
require.NoError(t, err)
Expand Down Expand Up @@ -618,6 +650,10 @@ func TestBasic(t *testing.T) {
"a/a.proto",
"a/b.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
},
},
{
ProtoFile: []*descriptorpb.FileDescriptorProto{
Expand All @@ -633,6 +669,10 @@ func TestBasic(t *testing.T) {
"b/a.proto",
"b/b.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
},
},
{
ProtoFile: []*descriptorpb.FileDescriptorProto{
Expand All @@ -643,13 +683,17 @@ func TestBasic(t *testing.T) {
FileToGenerate: []string{
"d/d.proto/d.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
},
},
}
requestsFromImages := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false)
requestsFromImages, err := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false)
require.NoError(t, err)
require.Equal(t, len(codeGeneratorRequests), len(requestsFromImages))
for i := range codeGeneratorRequests {
diff = cmp.Diff(codeGeneratorRequests[i], requestsFromImages[i], protocmp.Transform())
require.Equal(t, "", diff)
require.Empty(t, diff)
}
codeGeneratorRequestsIncludeImports := []*pluginpb.CodeGeneratorRequest{
{
Expand All @@ -665,6 +709,11 @@ func TestBasic(t *testing.T) {
"import.proto",
"a/b.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
testProtoImageFileToFileDescriptorProto(protoImageFileImport),
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
},
},
{
ProtoFile: []*descriptorpb.FileDescriptorProto{
Expand All @@ -680,6 +729,10 @@ func TestBasic(t *testing.T) {
"b/a.proto",
"b/b.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
},
},
{
ProtoFile: []*descriptorpb.FileDescriptorProto{
Expand All @@ -690,13 +743,17 @@ func TestBasic(t *testing.T) {
FileToGenerate: []string{
"d/d.proto/d.proto",
},
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
},
},
}
requestsFromImages = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false)
requestsFromImages, err = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false)
require.NoError(t, err)
require.Equal(t, len(codeGeneratorRequestsIncludeImports), len(requestsFromImages))
for i := range codeGeneratorRequestsIncludeImports {
diff = cmp.Diff(codeGeneratorRequestsIncludeImports[i], requestsFromImages[i], protocmp.Transform())
require.Equal(t, "", diff)
require.Empty(t, diff)
}
}

Expand Down
17 changes: 14 additions & 3 deletions private/bufpkg/bufimage/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1"
"github.com/bufbuild/buf/private/pkg/normalpath"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/protocompile/options"
"google.golang.org/protobuf/encoding/protowire"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
Expand Down Expand Up @@ -414,7 +415,7 @@ func imageToCodeGeneratorRequest(
includeWellKnownTypes bool,
alreadyUsedPaths map[string]struct{},
nonImportPaths map[string]struct{},
) *pluginpb.CodeGeneratorRequest {
) (*pluginpb.CodeGeneratorRequest, error) {
imageFiles := image.Files()
request := &pluginpb.CodeGeneratorRequest{
ProtoFile: make([]*descriptorpb.FileDescriptorProto, len(imageFiles)),
Expand All @@ -424,7 +425,8 @@ func imageToCodeGeneratorRequest(
request.Parameter = proto.String(parameter)
}
for i, imageFile := range imageFiles {
request.ProtoFile[i] = imageFile.FileDescriptorProto()
fileDescriptorProto := imageFile.FileDescriptorProto()
// ProtoFile should include only runtime-retained options for files to generate.
if isFileToGenerate(
imageFile,
alreadyUsedPaths,
Expand All @@ -433,9 +435,18 @@ func imageToCodeGeneratorRequest(
includeWellKnownTypes,
) {
request.FileToGenerate = append(request.FileToGenerate, imageFile.Path())
// Source-retention options for items in FileToGenerate are provided in SourceFileDescriptors.
request.SourceFileDescriptors = append(request.SourceFileDescriptors, fileDescriptorProto)
// And the corresponding descriptor in ProtoFile will have source-retention options stripped.
var err error
fileDescriptorProto, err = options.StripSourceRetentionOptionsFromFile(fileDescriptorProto)
if err != nil {
return nil, fmt.Errorf("failed to strip source-retention options for file %q when constructing a CodeGeneratorRequest: %w", imageFile.Path(), err)
}
}
request.ProtoFile[i] = fileDescriptorProto
}
return request
return request, nil
}

func isFileToGenerate(
Expand Down
Loading
Loading