Skip to content

Commit 83e33fc

Browse files
jhumpjhump
authored
Support recently added source_file_descriptors field of CodeGeneratorRequest (#2792)
1 parent de6db03 commit 83e33fc

File tree

11 files changed

+143
-46
lines changed

11 files changed

+143
-46
lines changed

‎.golangci.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,8 @@ issues:
233233
- paralleltest
234234
text: "missing the call to method parallel"
235235
path: private/pkg/git/repository_test.go
236+
# This greatly simplifies creation of descriptors, and it's safe enough since
237+
# it's just test code.
238+
- linters:
239+
- forcetypeassert
240+
path: private/bufpkg/bufimage/source_retention_options_test\.go

‎CHANGELOG.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22

33
## [Unreleased]
44

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

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

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ require (
66
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240221180331-f05a6f4403ce.1
77
connectrpc.com/connect v1.15.0
88
connectrpc.com/otelconnect v0.7.0
9-
github.com/bufbuild/protocompile v0.8.0
9+
github.com/bufbuild/protocompile v0.9.0
1010
github.com/bufbuild/protovalidate-go v0.6.0
1111
github.com/bufbuild/protoyaml-go v0.1.8
1212
github.com/docker/docker v25.0.3+incompatible

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc
1010
github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
1111
github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
1212
github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
13-
github.com/bufbuild/protocompile v0.8.0 h1:9Kp1q6OkS9L4nM3FYbr8vlJnEwtbpDPQlQOVXfR+78s=
14-
github.com/bufbuild/protocompile v0.8.0/go.mod h1:+Etjg4guZoAqzVk2czwEQP12yaxLJ8DxuqCJ9qHdH94=
13+
github.com/bufbuild/protocompile v0.9.0 h1:DI8qLG5PEO0Mu1Oj51YFPqtx6I3qYXUAhJVJ/IzAVl0=
14+
github.com/bufbuild/protocompile v0.9.0/go.mod h1:s89m1O8CqSYpyE/YaSGtg1r1YFMF5nLTwh4vlj6O444=
1515
github.com/bufbuild/protovalidate-go v0.6.0 h1:Jgs1kFuZ2LHvvdj8SpCLA1W/+pXS8QSM3F/E2l3InPY=
1616
github.com/bufbuild/protovalidate-go v0.6.0/go.mod h1:1LamgoYHZ2NdIQH0XGczGTc6Z8YrTHjcJVmiBaar4t4=
1717
github.com/bufbuild/protoyaml-go v0.1.8 h1:X9QDLfl9uEllh4gsXUGqPanZYCOKzd92uniRtW2OnAQ=

‎private/buf/bufgen/generator.go‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -272,22 +272,23 @@ func (g *generator) execLocalPlugin(
272272
if err != nil {
273273
return nil, err
274274
}
275-
generateOptions := []bufpluginexec.GenerateOption{
276-
bufpluginexec.GenerateWithPluginPath(pluginConfig.Path...),
277-
bufpluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath),
275+
requests, err := bufimage.ImagesToCodeGeneratorRequests(
276+
pluginImages,
277+
pluginConfig.Opt,
278+
nil,
279+
includeImports,
280+
includeWellKnownTypes,
281+
)
282+
if err != nil {
283+
return nil, err
278284
}
279285
response, err := g.pluginexecGenerator.Generate(
280286
ctx,
281287
container,
282288
pluginConfig.PluginName(),
283-
bufimage.ImagesToCodeGeneratorRequests(
284-
pluginImages,
285-
pluginConfig.Opt,
286-
nil,
287-
includeImports,
288-
includeWellKnownTypes,
289-
),
290-
generateOptions...,
289+
requests,
290+
bufpluginexec.GenerateWithPluginPath(pluginConfig.Path...),
291+
bufpluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath),
291292
)
292293
if err != nil {
293294
return nil, fmt.Errorf("plugin %s: %v", pluginConfig.PluginName(), err)

‎private/buf/cmd/buf/command/alpha/protoc/plugin.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,16 @@ func executePlugin(
5656
storageosProvider,
5757
runner,
5858
)
59+
requests, err := bufimage.ImagesToCodeGeneratorRequests(
60+
images,
61+
strings.Join(pluginInfo.Opt, ","),
62+
bufpluginexec.DefaultVersion,
63+
false,
64+
false,
65+
)
66+
if err != nil {
67+
return nil, err
68+
}
5969
var options []bufpluginexec.GenerateOption
6070
if pluginInfo.Path != "" {
6171
options = append(options, bufpluginexec.GenerateWithPluginPath(pluginInfo.Path))
@@ -64,13 +74,7 @@ func executePlugin(
6474
ctx,
6575
container,
6676
pluginName,
67-
bufimage.ImagesToCodeGeneratorRequests(
68-
images,
69-
strings.Join(pluginInfo.Opt, ","),
70-
bufpluginexec.DefaultVersion,
71-
false,
72-
false,
73-
),
77+
requests,
7478
options...,
7579
)
7680
if err != nil {

‎private/buf/cmd/protoc-gen-buf-lint/lint_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,5 +342,7 @@ func testBuildCodeGeneratorRequest(
342342
}
343343
image, err := bufimage.NewImage(imageFiles)
344344
require.NoError(t, err)
345-
return bufimage.ImageToCodeGeneratorRequest(image, parameter, nil, false, false)
345+
codeGenReq, err := bufimage.ImageToCodeGeneratorRequest(image, parameter, nil, false, false)
346+
require.NoError(t, err)
347+
return codeGenReq
346348
}

‎private/bufpkg/bufimage/bufimage.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ func ImageToCodeGeneratorRequest(
440440
compilerVersion *pluginpb.Version,
441441
includeImports bool,
442442
includeWellKnownTypes bool,
443-
) *pluginpb.CodeGeneratorRequest {
443+
) (*pluginpb.CodeGeneratorRequest, error) {
444444
return imageToCodeGeneratorRequest(
445445
image,
446446
parameter,
@@ -465,7 +465,7 @@ func ImagesToCodeGeneratorRequests(
465465
compilerVersion *pluginpb.Version,
466466
includeImports bool,
467467
includeWellKnownTypes bool,
468-
) []*pluginpb.CodeGeneratorRequest {
468+
) ([]*pluginpb.CodeGeneratorRequest, error) {
469469
requests := make([]*pluginpb.CodeGeneratorRequest, len(images))
470470
// alreadyUsedPaths is a map of paths that have already been added to an image.
471471
//
@@ -499,7 +499,8 @@ func ImagesToCodeGeneratorRequests(
499499
}
500500
}
501501
for i, image := range images {
502-
requests[i] = imageToCodeGeneratorRequest(
502+
var err error
503+
requests[i], err = imageToCodeGeneratorRequest(
503504
image,
504505
parameter,
505506
compilerVersion,
@@ -508,8 +509,11 @@ func ImagesToCodeGeneratorRequests(
508509
alreadyUsedPaths,
509510
nonImportPaths,
510511
)
512+
if err != nil {
513+
return nil, err
514+
}
511515
}
512-
return requests
516+
return requests, nil
513517
}
514518

515519
// ImageModuleDependency is a dependency of an image.

‎private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go‎

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ func TestBasic(t *testing.T) {
454454
newImage.Files(),
455455
)
456456
diff := cmp.Diff(protoImage, bufimage.ImageToProtoImage(newImage), protocmp.Transform())
457-
require.Equal(t, "", diff)
457+
require.Empty(t, diff)
458458
fileDescriptorSet := &descriptorpb.FileDescriptorSet{
459459
File: []*descriptorpb.FileDescriptorProto{
460460
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
@@ -467,7 +467,7 @@ func TestBasic(t *testing.T) {
467467
},
468468
}
469469
diff = cmp.Diff(fileDescriptorSet, bufimage.ImageToFileDescriptorSet(image), protocmp.Transform())
470-
require.Equal(t, "", diff)
470+
require.Empty(t, diff)
471471
codeGeneratorRequest := &pluginpb.CodeGeneratorRequest{
472472
ProtoFile: []*descriptorpb.FileDescriptorProto{
473473
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
@@ -486,21 +486,32 @@ func TestBasic(t *testing.T) {
486486
"b/b.proto",
487487
"d/d.proto/d.proto",
488488
},
489+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
490+
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
491+
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
492+
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
493+
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
494+
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
495+
},
489496
}
497+
actualRequest, err := bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false)
498+
require.NoError(t, err)
490499
diff = cmp.Diff(
491500
codeGeneratorRequest,
492-
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false),
501+
actualRequest,
493502
protocmp.Transform(),
494503
)
495-
require.Equal(t, "", diff)
504+
require.Empty(t, diff)
496505

497506
// verify that includeWellKnownTypes is a no-op if includeImports is false
507+
actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true)
508+
require.NoError(t, err)
498509
diff = cmp.Diff(
499510
codeGeneratorRequest,
500-
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true),
511+
actualRequest,
501512
protocmp.Transform(),
502513
)
503-
require.Equal(t, "", diff)
514+
require.Empty(t, diff)
504515

505516
codeGeneratorRequestIncludeImports := &pluginpb.CodeGeneratorRequest{
506517
ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -522,13 +533,23 @@ func TestBasic(t *testing.T) {
522533
"b/b.proto",
523534
"d/d.proto/d.proto",
524535
},
536+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
537+
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
538+
testProtoImageFileToFileDescriptorProto(protoImageFileImport),
539+
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
540+
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
541+
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
542+
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
543+
},
525544
}
545+
actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false)
546+
require.NoError(t, err)
526547
diff = cmp.Diff(
527548
codeGeneratorRequestIncludeImports,
528-
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false),
549+
actualRequest,
529550
protocmp.Transform(),
530551
)
531-
require.Equal(t, "", diff)
552+
require.Empty(t, diff)
532553
newImage, err = bufimage.NewImageForCodeGeneratorRequest(codeGeneratorRequest)
533554
require.NoError(t, err)
534555
AssertImageFilesEqual(
@@ -564,13 +585,24 @@ func TestBasic(t *testing.T) {
564585
"b/b.proto",
565586
"d/d.proto/d.proto",
566587
},
588+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
589+
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
590+
testProtoImageFileToFileDescriptorProto(protoImageFileImport),
591+
testProtoImageFileToFileDescriptorProto(protoImageFileWellKnownTypeImport),
592+
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
593+
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
594+
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
595+
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
596+
},
567597
}
598+
actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true)
599+
require.NoError(t, err)
568600
diff = cmp.Diff(
569601
codeGeneratorRequestIncludeImportsAndWellKnownTypes,
570-
bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true),
602+
actualRequest,
571603
protocmp.Transform(),
572604
)
573-
require.Equal(t, "", diff)
605+
require.Empty(t, diff)
574606
// imagesByDir and multiple Image tests
575607
imagesByDir, err := bufimage.ImageByDir(image)
576608
require.NoError(t, err)
@@ -618,6 +650,10 @@ func TestBasic(t *testing.T) {
618650
"a/a.proto",
619651
"a/b.proto",
620652
},
653+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
654+
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
655+
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
656+
},
621657
},
622658
{
623659
ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -633,6 +669,10 @@ func TestBasic(t *testing.T) {
633669
"b/a.proto",
634670
"b/b.proto",
635671
},
672+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
673+
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
674+
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
675+
},
636676
},
637677
{
638678
ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -643,13 +683,17 @@ func TestBasic(t *testing.T) {
643683
FileToGenerate: []string{
644684
"d/d.proto/d.proto",
645685
},
686+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
687+
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
688+
},
646689
},
647690
}
648-
requestsFromImages := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false)
691+
requestsFromImages, err := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false)
692+
require.NoError(t, err)
649693
require.Equal(t, len(codeGeneratorRequests), len(requestsFromImages))
650694
for i := range codeGeneratorRequests {
651695
diff = cmp.Diff(codeGeneratorRequests[i], requestsFromImages[i], protocmp.Transform())
652-
require.Equal(t, "", diff)
696+
require.Empty(t, diff)
653697
}
654698
codeGeneratorRequestsIncludeImports := []*pluginpb.CodeGeneratorRequest{
655699
{
@@ -665,6 +709,11 @@ func TestBasic(t *testing.T) {
665709
"import.proto",
666710
"a/b.proto",
667711
},
712+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
713+
testProtoImageFileToFileDescriptorProto(protoImageFileAA),
714+
testProtoImageFileToFileDescriptorProto(protoImageFileImport),
715+
testProtoImageFileToFileDescriptorProto(protoImageFileAB),
716+
},
668717
},
669718
{
670719
ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -680,6 +729,10 @@ func TestBasic(t *testing.T) {
680729
"b/a.proto",
681730
"b/b.proto",
682731
},
732+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
733+
testProtoImageFileToFileDescriptorProto(protoImageFileBA),
734+
testProtoImageFileToFileDescriptorProto(protoImageFileBB),
735+
},
683736
},
684737
{
685738
ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -690,13 +743,17 @@ func TestBasic(t *testing.T) {
690743
FileToGenerate: []string{
691744
"d/d.proto/d.proto",
692745
},
746+
SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
747+
testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
748+
},
693749
},
694750
}
695-
requestsFromImages = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false)
751+
requestsFromImages, err = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false)
752+
require.NoError(t, err)
696753
require.Equal(t, len(codeGeneratorRequestsIncludeImports), len(requestsFromImages))
697754
for i := range codeGeneratorRequestsIncludeImports {
698755
diff = cmp.Diff(codeGeneratorRequestsIncludeImports[i], requestsFromImages[i], protocmp.Transform())
699-
require.Equal(t, "", diff)
756+
require.Empty(t, diff)
700757
}
701758
}
702759

‎private/bufpkg/bufimage/util.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1"
2525
"github.com/bufbuild/buf/private/pkg/normalpath"
2626
"github.com/bufbuild/buf/private/pkg/slicesext"
27+
"github.com/bufbuild/protocompile/options"
2728
"google.golang.org/protobuf/encoding/protowire"
2829
"google.golang.org/protobuf/proto"
2930
"google.golang.org/protobuf/reflect/protoreflect"
@@ -414,7 +415,7 @@ func imageToCodeGeneratorRequest(
414415
includeWellKnownTypes bool,
415416
alreadyUsedPaths map[string]struct{},
416417
nonImportPaths map[string]struct{},
417-
) *pluginpb.CodeGeneratorRequest {
418+
) (*pluginpb.CodeGeneratorRequest, error) {
418419
imageFiles := image.Files()
419420
request := &pluginpb.CodeGeneratorRequest{
420421
ProtoFile: make([]*descriptorpb.FileDescriptorProto, len(imageFiles)),
@@ -424,7 +425,8 @@ func imageToCodeGeneratorRequest(
424425
request.Parameter = proto.String(parameter)
425426
}
426427
for i, imageFile := range imageFiles {
427-
request.ProtoFile[i] = imageFile.FileDescriptorProto()
428+
fileDescriptorProto := imageFile.FileDescriptorProto()
429+
// ProtoFile should include only runtime-retained options for files to generate.
428430
if isFileToGenerate(
429431
imageFile,
430432
alreadyUsedPaths,
@@ -433,9 +435,18 @@ func imageToCodeGeneratorRequest(
433435
includeWellKnownTypes,
434436
) {
435437
request.FileToGenerate = append(request.FileToGenerate, imageFile.Path())
438+
// Source-retention options for items in FileToGenerate are provided in SourceFileDescriptors.
439+
request.SourceFileDescriptors = append(request.SourceFileDescriptors, fileDescriptorProto)
440+
// And the corresponding descriptor in ProtoFile will have source-retention options stripped.
441+
var err error
442+
fileDescriptorProto, err = options.StripSourceRetentionOptionsFromFile(fileDescriptorProto)
443+
if err != nil {
444+
return nil, fmt.Errorf("failed to strip source-retention options for file %q when constructing a CodeGeneratorRequest: %w", imageFile.Path(), err)
445+
}
436446
}
447+
request.ProtoFile[i] = fileDescriptorProto
437448
}
438-
return request
449+
return request, nil
439450
}
440451

441452
func isFileToGenerate(

0 commit comments

Comments
 (0)