-
Notifications
You must be signed in to change notification settings - Fork 346
Add type exclusion filters for buf generate #3624
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
Changes from 46 commits
7aab46c
e2608aa
d05d210
5940c75
f763966
4935430
400f958
2a66cf2
2c4378d
a0b373e
11a88a9
99902e6
b5b7daa
e2c5f8b
8e286f4
cb5a8ee
00ebe13
20b720b
3cec9b8
5997223
764626a
dc449ef
90cc4a0
cffd97c
b460387
e531a7a
2a2017a
49cf91c
07ea2e0
c43b74e
347120a
d66e197
883527c
5decf0b
e07f2b7
a3f2055
b8ed6d7
cbfb29f
13c3882
3124c00
eaf49d2
59f5d78
07152c5
bf07344
cd43600
f291c2a
e78e983
e39dddd
56c5fda
9a80424
60dbf41
a57873e
986a33f
e7de3e8
d7849e1
62dc5d4
4c75c24
cd9d8a1
219ff4b
c20a21c
9e8f745
bc52487
7b12e98
5035d61
12ce214
f5cfcd5
01f7b09
f5398e5
62b4402
1696cca
584bde7
1585379
304f0af
698e350
d57a7d9
7b1f6bf
d327c23
b4c1f56
b6a92ee
412b940
c6b0e6a
3c13e37
1c78fc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -20,12 +20,14 @@ import ( | |||||
| "fmt" | ||||||
| "log/slog" | ||||||
| "path/filepath" | ||||||
| "sort" | ||||||
|
|
||||||
| connect "connectrpc.com/connect" | ||||||
| "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/bufimagemodify" | ||||||
| "github.com/bufbuild/buf/private/bufpkg/bufimage/bufimageutil" | ||||||
| "github.com/bufbuild/buf/private/bufpkg/bufprotoplugin" | ||||||
| "github.com/bufbuild/buf/private/bufpkg/bufprotoplugin/bufprotopluginos" | ||||||
| "github.com/bufbuild/buf/private/bufpkg/bufremoteplugin" | ||||||
|
|
@@ -204,72 +206,91 @@ func (g *generator) execPlugins( | |||||
| includeImportsOverride *bool, | ||||||
| includeWellKnownTypesOverride *bool, | ||||||
| ) ([]*pluginpb.CodeGeneratorResponse, error) { | ||||||
| imageProvider := newImageProvider(image) | ||||||
| // Collect all of the plugin jobs so that they can be executed in parallel. | ||||||
| jobs := make([]func(context.Context) error, 0, len(pluginConfigs)) | ||||||
| responses := make([]*pluginpb.CodeGeneratorResponse, len(pluginConfigs)) | ||||||
| requiredFeatures := computeRequiredFeatures(image) | ||||||
| remotePluginConfigTable := make(map[string][]*remotePluginExecArgs, len(pluginConfigs)) | ||||||
| for i, pluginConfig := range pluginConfigs { | ||||||
| index := i | ||||||
| currentPluginConfig := pluginConfig | ||||||
| // We're using this as a proxy for Type() == PluginConfigTypeRemote. | ||||||
| // | ||||||
| // We should be using the enum here. | ||||||
| remote := currentPluginConfig.RemoteHost() | ||||||
| if remote != "" { | ||||||
| remotePluginConfigTable[remote] = append( | ||||||
| remotePluginConfigTable[remote], | ||||||
| &remotePluginExecArgs{ | ||||||
| Index: index, | ||||||
| PluginConfig: currentPluginConfig, | ||||||
| }, | ||||||
|
|
||||||
| // Group the pluginConfigs by similar properties to batch image processing. | ||||||
| pluginConfigsForImage := slicesext.ToIndexedValuesMap(pluginConfigs, createPluginConfigKeyForImage) | ||||||
| for _, indexedPluginConfigs := range pluginConfigsForImage { | ||||||
| image := image | ||||||
| hashPluginConfig := indexedPluginConfigs[0].Value | ||||||
|
|
||||||
| // Apply per-plugin filters. | ||||||
| includeTypes := hashPluginConfig.Types() | ||||||
| excludeTypes := hashPluginConfig.ExcludeTypes() | ||||||
| if len(includeTypes) > 0 || len(excludeTypes) > 0 { | ||||||
| var err error | ||||||
| image, err = bufimageutil.FilterImage( | ||||||
| image, | ||||||
| bufimageutil.WithIncludeTypes(includeTypes...), | ||||||
| bufimageutil.WithExcludeTypes(excludeTypes...), | ||||||
| ) | ||||||
| } else { | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Batch for each remote. | ||||||
| if remote := hashPluginConfig.RemoteHost(); remote != "" { | ||||||
| jobs = append(jobs, func(ctx context.Context) error { | ||||||
| includeImports := currentPluginConfig.IncludeImports() | ||||||
| if includeImportsOverride != nil { | ||||||
| includeImports = *includeImportsOverride | ||||||
| } | ||||||
| includeWellKnownTypes := currentPluginConfig.IncludeWKT() | ||||||
| if includeWellKnownTypesOverride != nil { | ||||||
| includeWellKnownTypes = *includeWellKnownTypesOverride | ||||||
| } | ||||||
| response, err := g.execLocalPlugin( | ||||||
| results, err := g.execRemotePluginsV2( | ||||||
| ctx, | ||||||
| container, | ||||||
| imageProvider, | ||||||
| currentPluginConfig, | ||||||
| includeImports, | ||||||
| includeWellKnownTypes, | ||||||
| image, | ||||||
| remote, | ||||||
| indexedPluginConfigs, | ||||||
| includeImportsOverride, | ||||||
| includeWellKnownTypesOverride, | ||||||
| ) | ||||||
| if err != nil { | ||||||
| return err | ||||||
| } | ||||||
| responses[index] = response | ||||||
| for _, result := range results { | ||||||
| responses[result.Index] = result.Value | ||||||
| } | ||||||
| return nil | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
| } | ||||||
| // Batch for each remote. | ||||||
| for remote, indexedPluginConfigs := range remotePluginConfigTable { | ||||||
| if len(indexedPluginConfigs) > 0 { | ||||||
|
|
||||||
| // Local plugins. | ||||||
| var images []bufimage.Image | ||||||
| switch Strategy(hashPluginConfig.Strategy()) { | ||||||
| case StrategyAll: | ||||||
| images = []bufimage.Image{image} | ||||||
| case StrategyDirectory: | ||||||
| var err error | ||||||
| images, err = bufimage.ImageByDir(image) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| default: | ||||||
| return nil, fmt.Errorf("unknown strategy: %v", hashPluginConfig.Strategy()) | ||||||
| } | ||||||
| for _, indexedPluginConfig := range indexedPluginConfigs { | ||||||
| jobs = append(jobs, func(ctx context.Context) error { | ||||||
| results, err := g.execRemotePluginsV2( | ||||||
| includeImports := hashPluginConfig.IncludeImports() | ||||||
|
||||||
| includeImports := hashPluginConfig.IncludeImports() | |
| includeImports := indexedPluginConfig.Value.IncludeImports() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done e39dddd
emcfarlane marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above, the loop variable was named indexedPluginConfig. Maybe use the same name here, for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name is a little odd. I think you're just trying to recover the values in the key, except the key had to mangle the slices to make things comparable. So maybe naming it
pluginConfigForKeywould be more clear?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e78e983