diff --git a/private/buf/bufgen/generator.go b/private/buf/bufgen/generator.go index 86273d7921..930f88e565 100644 --- a/private/buf/bufgen/generator.go +++ b/private/buf/bufgen/generator.go @@ -24,6 +24,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufprotopluginexec" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufimage/bufimagegenerate" "github.com/bufbuild/buf/private/bufpkg/bufimage/bufimagemodify" "github.com/bufbuild/buf/private/bufpkg/bufprotoplugin" "github.com/bufbuild/buf/private/bufpkg/bufprotoplugin/bufprotopluginos" @@ -324,7 +325,7 @@ func (g *generator) execLocalPlugin( if err != nil { return nil, err } - requests, err := bufimage.ImagesToCodeGeneratorRequests( + requests, err := bufimagegenerate.ImagesToCodeGeneratorRequests( pluginImages, pluginConfig.Opt(), nil, diff --git a/private/buf/bufgen/image_provider.go b/private/buf/bufgen/image_provider.go index b57e3bca6b..bddba9f4ef 100644 --- a/private/buf/bufgen/image_provider.go +++ b/private/buf/bufgen/image_provider.go @@ -19,6 +19,7 @@ import ( "sync" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufimage/bufimagegenerate" ) // imageProvider is used to provide the images used @@ -29,7 +30,7 @@ import ( // strategy. type imageProvider struct { image bufimage.Image - imagesByDir []bufimage.Image + imagesByDir []bufimagegenerate.ImageForGeneration lock sync.Mutex } @@ -39,16 +40,16 @@ func newImageProvider(image bufimage.Image) *imageProvider { } } -func (p *imageProvider) GetImages(strategy Strategy) ([]bufimage.Image, error) { +func (p *imageProvider) GetImages(strategy Strategy) ([]bufimagegenerate.ImageForGeneration, error) { switch strategy { case StrategyAll: - return []bufimage.Image{p.image}, nil + return []bufimagegenerate.ImageForGeneration{bufimagegenerate.NewImageForGenerationFromImage(p.image)}, nil case StrategyDirectory: p.lock.Lock() defer p.lock.Unlock() if p.imagesByDir == nil { var err error - p.imagesByDir, err = bufimage.ImageByDir(p.image) + p.imagesByDir, err = bufimagegenerate.ImageByDirSplitImports(p.image) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/command/alpha/protoc/plugin.go b/private/buf/cmd/buf/command/alpha/protoc/plugin.go index 7c09dade60..3ad1eeb274 100644 --- a/private/buf/cmd/buf/command/alpha/protoc/plugin.go +++ b/private/buf/cmd/buf/command/alpha/protoc/plugin.go @@ -21,8 +21,10 @@ import ( "github.com/bufbuild/buf/private/buf/bufprotopluginexec" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufimage/bufimagegenerate" "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" "go.uber.org/zap" @@ -59,8 +61,8 @@ func executePlugin( storageosProvider, runner, ) - requests, err := bufimage.ImagesToCodeGeneratorRequests( - images, + requests, err := bufimagegenerate.ImagesToCodeGeneratorRequests( + slicesext.Map(images, bufimagegenerate.NewImageForGenerationFromImage), strings.Join(pluginInfo.Opt, ","), bufprotopluginexec.DefaultVersion, false, diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint_test.go b/private/buf/cmd/protoc-gen-buf-lint/lint_test.go index 531c21d563..37da560765 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint_test.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufimage/bufimagegenerate" "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" @@ -365,7 +366,7 @@ func testBuildRequest( } image, err := bufimage.NewImage(imageFiles) require.NoError(t, err) - codeGenReq, err := bufimage.ImageToCodeGeneratorRequest(image, parameter, nil, false, false) + codeGenReq, err := bufimagegenerate.ImageToCodeGeneratorRequest(bufimagegenerate.NewImageForGenerationFromImage(image), parameter, nil, false, false) require.NoError(t, err) request, err := protoplugin.NewRequest(codeGenReq) require.NoError(t, err) diff --git a/private/bufpkg/bufimage/bufimage.go b/private/bufpkg/bufimage/bufimage.go index ec5a3d1d51..ecd40e15de 100644 --- a/private/bufpkg/bufimage/bufimage.go +++ b/private/bufpkg/bufimage/bufimage.go @@ -577,94 +577,6 @@ func ImageToFileDescriptorProtos(image Image) []*descriptorpb.FileDescriptorProt return imageFilesToFileDescriptorProtos(image.Files()) } -// ImageToCodeGeneratorRequest returns a new CodeGeneratorRequest for the Image. -// -// All non-imports are added as files to generate. -// If includeImports is set, all non-well-known-type imports are also added as files to generate. -// If includeWellKnownTypes is set, well-known-type imports are also added as files to generate. -// includeWellKnownTypes has no effect if includeImports is not set. -func ImageToCodeGeneratorRequest( - image Image, - parameter string, - compilerVersion *pluginpb.Version, - includeImports bool, - includeWellKnownTypes bool, -) (*pluginpb.CodeGeneratorRequest, error) { - return imageToCodeGeneratorRequest( - image, - parameter, - compilerVersion, - includeImports, - includeWellKnownTypes, - nil, - nil, - ) -} - -// ImagesToCodeGeneratorRequests converts the Images to CodeGeneratorRequests. -// -// All non-imports are added as files to generate. -// If includeImports is set, all non-well-known-type imports are also added as files to generate. -// If includeImports is set, only one CodeGeneratorRequest will contain any given file as a FileToGenerate. -// If includeWellKnownTypes is set, well-known-type imports are also added as files to generate. -// includeWellKnownTypes has no effect if includeImports is not set. -func ImagesToCodeGeneratorRequests( - images []Image, - parameter string, - compilerVersion *pluginpb.Version, - includeImports bool, - includeWellKnownTypes bool, -) ([]*pluginpb.CodeGeneratorRequest, error) { - requests := make([]*pluginpb.CodeGeneratorRequest, len(images)) - // alreadyUsedPaths is a map of paths that have already been added to an image. - // - // We track this if includeImports is set, so that when we find an import, we can - // see if the import was already added to a CodeGeneratorRequest via another Image - // in the Image slice. If the import was already added, we do not add duplicates - // across CodeGeneratorRequests. - var alreadyUsedPaths map[string]struct{} - // nonImportPaths is a map of non-import paths. - // - // We track this if includeImports is set. If we find a non-import file in Image A - // and this file is an import in Image B, the file will have already been added to - // a CodeGeneratorRequest via Image A, so do not add the duplicate to any other - // CodeGeneratorRequest. - var nonImportPaths map[string]struct{} - if includeImports { - // We don't need to track these if includeImports is false, so we only populate - // the maps if includeImports is true. If includeImports is false, only non-imports - // will be added to each CodeGeneratorRequest, so figuring out whether or not - // we should add a given import to a given CodeGeneratorRequest is unnecessary. - // - // imageToCodeGeneratorRequest checks if these maps are nil before every access. - alreadyUsedPaths = make(map[string]struct{}) - nonImportPaths = make(map[string]struct{}) - for _, image := range images { - for _, imageFile := range image.Files() { - if !imageFile.IsImport() { - nonImportPaths[imageFile.Path()] = struct{}{} - } - } - } - } - for i, image := range images { - var err error - requests[i], err = imageToCodeGeneratorRequest( - image, - parameter, - compilerVersion, - includeImports, - includeWellKnownTypes, - alreadyUsedPaths, - nonImportPaths, - ) - if err != nil { - return nil, err - } - } - return requests, nil -} - type newImageForProtoOptions struct { noReparse bool computeUnusedImports bool diff --git a/private/bufpkg/bufimage/bufimagegenerate/bufimagegenerate.go b/private/bufpkg/bufimage/bufimagegenerate/bufimagegenerate.go new file mode 100644 index 0000000000..08dc2ebd68 --- /dev/null +++ b/private/bufpkg/bufimage/bufimagegenerate/bufimagegenerate.go @@ -0,0 +1,71 @@ +// Copyright 2020-2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bufimagegenerate + +import ( + "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/pkg/slicesext" +) + +// ImageForGeneration is a buf image to be generated. +type ImageForGeneration interface { + // files are the files that comprise the image, but not all files should be generated. + files() []imageFileForGeneration +} + +// NewImageForGenerationFromImage creates an ImageForGeneration that works the exact same way as before +// this type is added, and this is the one of the two exported functions that construct an ImageForGeneration. +// The other one is ImageByDirSplitImports. +// +// TODO: update this doc +func NewImageForGenerationFromImage(image bufimage.Image) ImageForGeneration { + imageFilesForGeneration := slicesext.Map(image.Files(), func(imageFile bufimage.ImageFile) imageFileForGeneration { + return imageFileForGeneration{ + ImageFile: imageFile, + toGenerate: true, + } + }) + return imageForGeneration(imageFilesForGeneration) +} + +// *** PRIVATE *** + +type imageFileForGeneration struct { + bufimage.ImageFile + + // toGenerate returns whether the file may be generated. This is not necessarily the same as IsImport(), + // especially when strategy is set to directory. It also does not guarantee the file's inclusion in file_to_generate. + // + // - If it returns true, the file will be generated unless includeImports or includeWKT says otherwise. + // - If it returns false, it will not be generated regardless of includeImports and includeWKT. + toGenerate bool +} + +type imageForGeneration []imageFileForGeneration + +func (i imageForGeneration) files() []imageFileForGeneration { + return []imageFileForGeneration(i) +} + +func newImageForGeneration(image bufimage.Image, filesToGenerate map[string]struct{}) ImageForGeneration { + imageFilesForGeneration := imageForGeneration(slicesext.Map(image.Files(), func(imageFile bufimage.ImageFile) imageFileForGeneration { + _, ok := filesToGenerate[imageFile.Path()] + return imageFileForGeneration{ + ImageFile: imageFile, + toGenerate: ok, + } + })) + return imageForGeneration(imageFilesForGeneration) +} diff --git a/private/bufpkg/bufimage/bufimagegenerate/codegenrequest.go b/private/bufpkg/bufimage/bufimagegenerate/codegenrequest.go new file mode 100644 index 0000000000..d56ac2c848 --- /dev/null +++ b/private/bufpkg/bufimage/bufimagegenerate/codegenrequest.go @@ -0,0 +1,210 @@ +// Copyright 2020-2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bufimagegenerate + +import ( + "fmt" + + "github.com/bufbuild/buf/private/gen/data/datawkt" + "github.com/bufbuild/protoplugin/protopluginutil" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" + "google.golang.org/protobuf/types/pluginpb" +) + +// ImagesToCodeGeneratorRequests converts the Images to CodeGeneratorRequests. +// +// All non-imports are added as files to generate. +// If includeImports is set, all non-well-known-type imports are also added as files to generate. +// If includeImports is set, only one CodeGeneratorRequest will contain any given file as a FileToGenerate. +// If includeWellKnownTypes is set, well-known-type imports are also added as files to generate. +// includeWellKnownTypes has no effect if includeImports is not set. +func ImagesToCodeGeneratorRequests( + images []ImageForGeneration, + parameter string, + compilerVersion *pluginpb.Version, + includeImports bool, + includeWellKnownTypes bool, +) ([]*pluginpb.CodeGeneratorRequest, error) { + requests := make([]*pluginpb.CodeGeneratorRequest, 0, len(images)) + // alreadyUsedPaths is a map of paths that have already been added to an image. + // + // We track this if includeImports is set, so that when we find an import, we can + // see if the import was already added to a CodeGeneratorRequest via another Image + // in the Image slice. If the import was already added, we do not add duplicates + // across CodeGeneratorRequests. + var alreadyUsedPaths map[string]struct{} + // nonImportPaths is a map of non-import paths. + // + // We track this if includeImports is set. If we find a non-import file in Image A + // and this file is an import in Image B, the file will have already been added to + // a CodeGeneratorRequest via Image A, so do not add the duplicate to any other + // CodeGeneratorRequest. + var nonImportPaths map[string]struct{} + if includeImports { + // We don't need to track these if includeImports is false, so we only populate + // the maps if includeImports is true. If includeImports is false, only non-imports + // will be added to each CodeGeneratorRequest, so figuring out whether or not + // we should add a given import to a given CodeGeneratorRequest is unnecessary. + // + // imageToCodeGeneratorRequest checks if these maps are nil before every access. + alreadyUsedPaths = make(map[string]struct{}) + nonImportPaths = make(map[string]struct{}) + for _, image := range images { + for _, imageFile := range image.files() { + if !imageFile.IsImport() { + nonImportPaths[imageFile.Path()] = struct{}{} + } + } + } + } + for _, image := range images { + var err error + request, err := imageToCodeGeneratorRequest( + image, + parameter, + compilerVersion, + includeImports, + includeWellKnownTypes, + alreadyUsedPaths, + nonImportPaths, + ) + if err != nil { + return nil, err + } + if len(request.FileToGenerate) == 0 { + continue + } + requests = append(requests, request) + } + return requests, nil +} + +// ImageToCodeGeneratorRequest returns a new CodeGeneratorRequest for the Image. +// +// All non-imports are added as files to generate. +// If includeImports is set, all non-well-known-type imports are also added as files to generate. +// If includeWellKnownTypes is set, well-known-type imports are also added as files to generate. +// includeWellKnownTypes has no effect if includeImports is not set. +func ImageToCodeGeneratorRequest( + image ImageForGeneration, + parameter string, + compilerVersion *pluginpb.Version, + includeImports bool, + includeWellKnownTypes bool, +) (*pluginpb.CodeGeneratorRequest, error) { + return imageToCodeGeneratorRequest( + image, + parameter, + compilerVersion, + includeImports, + includeWellKnownTypes, + nil, + nil, + ) +} + +// *** PRIVATE *** + +func imageToCodeGeneratorRequest( + image ImageForGeneration, + parameter string, + compilerVersion *pluginpb.Version, + includeImports bool, + includeWellKnownTypes bool, + alreadyUsedPaths map[string]struct{}, + nonImportPaths map[string]struct{}, +) (*pluginpb.CodeGeneratorRequest, error) { + imageFiles := image.files() + request := &pluginpb.CodeGeneratorRequest{ + ProtoFile: make([]*descriptorpb.FileDescriptorProto, len(imageFiles)), + CompilerVersion: compilerVersion, + } + if parameter != "" { + request.Parameter = proto.String(parameter) + } + for i, imageFile := range imageFiles { + fileDescriptorProto := imageFile.FileDescriptorProto() + // ProtoFile should include only runtime-retained options for files to generate. + if isFileToGenerate( + imageFile, + alreadyUsedPaths, + nonImportPaths, + includeImports, + 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 = protopluginutil.StripSourceRetentionOptions(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, nil +} + +func isFileToGenerate( + imageFile imageFileForGeneration, + alreadyUsedPaths map[string]struct{}, + nonImportPaths map[string]struct{}, + includeImports bool, + includeWellKnownTypes bool, +) bool { + if !imageFile.toGenerate { + return false + } + path := imageFile.Path() + if !imageFile.IsImport() { + if alreadyUsedPaths != nil { + // set as already used + alreadyUsedPaths[path] = struct{}{} + } + // this is a non-import in this image, we always want to generate + return true + } + if !includeImports { + // we don't want to include imports + return false + } + if !includeWellKnownTypes && datawkt.Exists(path) { + // we don't want to generate wkt even if includeImports is set unless + // includeWellKnownTypes is set + return false + } + if alreadyUsedPaths != nil { + if _, ok := alreadyUsedPaths[path]; ok { + // this was already added for generate to another image + return false + } + } + if nonImportPaths != nil { + if _, ok := nonImportPaths[path]; ok { + // this is a non-import in another image so it will be generated + // from another image + return false + } + } + // includeImports is set, this isn't a wkt, and it won't be generated in another image + if alreadyUsedPaths != nil { + // set as already used + alreadyUsedPaths[path] = struct{}{} + } + return true +} diff --git a/private/bufpkg/bufimage/bufimagegenerate/split.go b/private/bufpkg/bufimage/bufimagegenerate/split.go new file mode 100644 index 0000000000..e503f3ca45 --- /dev/null +++ b/private/bufpkg/bufimage/bufimagegenerate/split.go @@ -0,0 +1,147 @@ +// Copyright 2020-2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bufimagegenerate + +import ( + "sort" + + "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/pkg/normalpath" + "github.com/bufbuild/buf/private/pkg/slicesext" + "github.com/bufbuild/buf/private/pkg/syserror" +) + +// ImageByDirSplitImports returns multiple images split by directory. +// +// This function does not treat import files differently from non-import files. +// +// TODO: rename this and maybe it should be moved to the bufgen package. +// +// If strategy is dir, we want the files in file_to_generate inside each +// CodeGeneratorRequest to be in the same directory. +// +// For example, if we have an image with the following files: +// +// - a/a.proto -> b/b.proto +// - b/b.proto -> c/c.proto +// - b/b.proto -> d/d.proto +// +// where a/a.proto and b/b.proto are the non-imports, and c/c.proto and and d/d.proto are imports. +// +// Now, if we have includeImports set to true, we should send 4 CodeGeneratorRequests to each plugin. +// Each request should have file_to_generate equal to ["x/x.proto"] of length 1, with x being one of a, b, c or d, +// and its proto_file should include "all files in files_to_generate and everything they import", by contract. +// +// If includeImports is set to false, the request with c/c.proto to generate and the one with d/d.proto should not exist, +// and we should only send 2 requests to the plugin. +func ImageByDirSplitImports(image bufimage.Image) ([]ImageForGeneration, error) { + dirToImageFilePaths := normalpath.ByDir( + slicesext.Map(image.Files(), func(imageFile bufimage.ImageFile) string { + return imageFile.Path() + })..., + ) + // we need this to produce a deterministic order of the returned Images + dirs := make([]string, 0, len(dirToImageFilePaths)) + for dir := range dirToImageFilePaths { + dirs = append(dirs, dir) + } + sort.Strings(dirs) + newImages := make([]ImageForGeneration, 0, len(dirToImageFilePaths)) + for _, dir := range dirs { + imageFilePaths, ok := dirToImageFilePaths[dir] + if !ok { + // this should never happen + return nil, syserror.Newf("no dir for %q in dirToImageFilePaths", dir) + } + imageFilesToGenerate, err := slicesext.MapError(imageFilePaths, func(filePath string) (bufimage.ImageFile, error) { + imageFile := image.GetFile(filePath) + if imageFile == nil { + return nil, syserror.Newf("expected image file to exist at %q", filePath) + } + return imageFile, nil + }) + if err != nil { + return nil, err + } + newImage, err := getImageForGenerationForFilePaths(image, slicesext.ToStructMap(imageFilePaths), imageFilesToGenerate) + if err != nil { + return nil, err + } + newImages = append(newImages, newImage) + } + return newImages, nil +} + +// *** PRIVATE *** + +func getImageForGenerationForFilePaths( + image bufimage.Image, + generationPaths map[string]struct{}, + generationImageFiles []bufimage.ImageFile, +) (ImageForGeneration, error) { + var imageFiles []bufimage.ImageFile + seenPaths := make(map[string]struct{}) + for _, nonImportImageFile := range generationImageFiles { + imageFiles = addFileForGenerationWithImports( + imageFiles, + image, + generationPaths, + seenPaths, + nonImportImageFile, + ) + } + imageWithPaths, err := bufimage.NewImage(imageFiles) + if err != nil { + return nil, err + } + return newImageForGeneration(imageWithPaths, generationPaths), nil +} + +// largely copied from addFileWithImports +// +// returns accumulated files in correct order +func addFileForGenerationWithImports( + accumulator []bufimage.ImageFile, + image bufimage.Image, + nonImportPaths map[string]struct{}, + seenPaths map[string]struct{}, + imageFile bufimage.ImageFile, +) []bufimage.ImageFile { + path := imageFile.Path() + // if seen already, skip + if _, ok := seenPaths[path]; ok { + return accumulator + } + seenPaths[path] = struct{}{} + + // then, add imports first, for proper ordering + for _, importPath := range imageFile.FileDescriptorProto().GetDependency() { + if importFile := image.GetFile(importPath); importFile != nil { + accumulator = addFileForGenerationWithImports( + accumulator, + image, + nonImportPaths, + seenPaths, + importFile, + ) + } + } + + accumulator = append( + accumulator, + imageFile, + ) + return accumulator +} diff --git a/private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go b/private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go index fca33cf987..ca8a32c950 100644 --- a/private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go +++ b/private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufimage/bufimagegenerate" "github.com/bufbuild/buf/private/gen/data/datawkt" imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -510,7 +511,7 @@ func TestBasic(t *testing.T) { testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName), }, } - actualRequest, err := bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false) + actualRequest, err := bufimagegenerate.ImageToCodeGeneratorRequest(bufimagegenerate.NewImageForGenerationFromImage(image), "foo", nil, false, false) require.NoError(t, err) diff = cmp.Diff( codeGeneratorRequest, @@ -520,7 +521,7 @@ func TestBasic(t *testing.T) { require.Empty(t, diff) // verify that includeWellKnownTypes is a no-op if includeImports is false - actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true) + actualRequest, err = bufimagegenerate.ImageToCodeGeneratorRequest(bufimagegenerate.NewImageForGenerationFromImage(image), "foo", nil, false, true) require.NoError(t, err) diff = cmp.Diff( codeGeneratorRequest, @@ -558,7 +559,7 @@ func TestBasic(t *testing.T) { testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName), }, } - actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false) + actualRequest, err = bufimagegenerate.ImageToCodeGeneratorRequest(bufimagegenerate.NewImageForGenerationFromImage(image), "foo", nil, true, false) require.NoError(t, err) diff = cmp.Diff( codeGeneratorRequestIncludeImports, @@ -611,7 +612,7 @@ func TestBasic(t *testing.T) { testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName), }, } - actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true) + actualRequest, err = bufimagegenerate.ImageToCodeGeneratorRequest(bufimagegenerate.NewImageForGenerationFromImage(image), "foo", nil, true, true) require.NoError(t, err) diff = cmp.Diff( codeGeneratorRequestIncludeImportsAndWellKnownTypes, @@ -704,7 +705,7 @@ func TestBasic(t *testing.T) { }, }, } - requestsFromImages, err := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false) + requestsFromImages, err := bufimagegenerate.ImagesToCodeGeneratorRequests(slicesext.Map(imagesByDir, bufimagegenerate.NewImageForGenerationFromImage), "foo", nil, false, false) require.NoError(t, err) require.Equal(t, len(codeGeneratorRequests), len(requestsFromImages)) for i := range codeGeneratorRequests { @@ -764,7 +765,7 @@ func TestBasic(t *testing.T) { }, }, } - requestsFromImages, err = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false) + requestsFromImages, err = bufimagegenerate.ImagesToCodeGeneratorRequests(slicesext.Map(imagesByDir, bufimagegenerate.NewImageForGenerationFromImage), "foo", nil, true, false) require.NoError(t, err) require.Equal(t, len(codeGeneratorRequestsIncludeImports), len(requestsFromImages)) for i := range codeGeneratorRequestsIncludeImports { diff --git a/private/bufpkg/bufimage/util.go b/private/bufpkg/bufimage/util.go index 1880f3297a..a52780206c 100644 --- a/private/bufpkg/bufimage/util.go +++ b/private/bufpkg/bufimage/util.go @@ -19,17 +19,14 @@ import ( "fmt" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/gen/data/datawkt" 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/buf/private/pkg/uuidutil" - "github.com/bufbuild/protoplugin/protopluginutil" "google.golang.org/protobuf/encoding/protowire" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/descriptorpb" - "google.golang.org/protobuf/types/pluginpb" ) // Must match the tag number for ImageFile.buf_extensions defined in proto/buf/alpha/image/v1/image.proto. @@ -411,91 +408,3 @@ func stripBufExtensionField(unknownFields protoreflect.RawFields) protoreflect.R } return result } - -func imageToCodeGeneratorRequest( - image Image, - parameter string, - compilerVersion *pluginpb.Version, - includeImports bool, - includeWellKnownTypes bool, - alreadyUsedPaths map[string]struct{}, - nonImportPaths map[string]struct{}, -) (*pluginpb.CodeGeneratorRequest, error) { - imageFiles := image.Files() - request := &pluginpb.CodeGeneratorRequest{ - ProtoFile: make([]*descriptorpb.FileDescriptorProto, len(imageFiles)), - CompilerVersion: compilerVersion, - } - if parameter != "" { - request.Parameter = proto.String(parameter) - } - for i, imageFile := range imageFiles { - fileDescriptorProto := imageFile.FileDescriptorProto() - // ProtoFile should include only runtime-retained options for files to generate. - if isFileToGenerate( - imageFile, - alreadyUsedPaths, - nonImportPaths, - includeImports, - 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 = protopluginutil.StripSourceRetentionOptions(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, nil -} - -func isFileToGenerate( - imageFile ImageFile, - alreadyUsedPaths map[string]struct{}, - nonImportPaths map[string]struct{}, - includeImports bool, - includeWellKnownTypes bool, -) bool { - path := imageFile.Path() - if !imageFile.IsImport() { - if alreadyUsedPaths != nil { - // set as already used - alreadyUsedPaths[path] = struct{}{} - } - // this is a non-import in this image, we always want to generate - return true - } - if !includeImports { - // we don't want to include imports - return false - } - if !includeWellKnownTypes && datawkt.Exists(path) { - // we don't want to generate wkt even if includeImports is set unless - // includeWellKnownTypes is set - return false - } - if alreadyUsedPaths != nil { - if _, ok := alreadyUsedPaths[path]; ok { - // this was already added for generate to another image - return false - } - } - if nonImportPaths != nil { - if _, ok := nonImportPaths[path]; ok { - // this is a non-import in another image so it will be generated - // from another image - return false - } - } - // includeImports is set, this isn't a wkt, and it won't be generated in another image - if alreadyUsedPaths != nil { - // set as already used - alreadyUsedPaths[path] = struct{}{} - } - return true -}