diff --git a/CHANGELOG.md b/CHANGELOG.md index ca527b8773..dc5491d2fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ - Fix issue where `buf generate` would succeed on missing insertion points and panic on empty insertion point files. +- Update `buf generate` to allow the use of Editions syntax when doing local code + generation by proxying to a `protoc` binary (for languages where code gen is + implemented inside of `protoc` instead of in a plugin: Java, C++, Python, etc). +- Allow use of an array of strings for the `protoc_path` property of for `buf.gen.yaml`, + where the first array element is the actual path and other array elements are extra + arguments that are passed to `protoc` each time it is invoked. ## [v1.33.0] - 2024-06-13 diff --git a/private/buf/bufgen/generator.go b/private/buf/bufgen/generator.go index c4a5775677..af4acc532d 100644 --- a/private/buf/bufgen/generator.go +++ b/private/buf/bufgen/generator.go @@ -308,7 +308,7 @@ func (g *generator) execLocalPlugin( pluginConfig.Name(), requests, bufprotopluginexec.GenerateWithPluginPath(pluginConfig.Path()...), - bufprotopluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath()), + bufprotopluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath()...), ) if err != nil { return nil, fmt.Errorf("plugin %s: %v", pluginConfig.Name(), err) diff --git a/private/buf/bufprotopluginexec/bufprotopluginexec.go b/private/buf/bufprotopluginexec/bufprotopluginexec.go index 23e9f70e53..9f18a1bae6 100644 --- a/private/buf/bufprotopluginexec/bufprotopluginexec.go +++ b/private/buf/bufprotopluginexec/bufprotopluginexec.go @@ -98,7 +98,7 @@ func GenerateWithPluginPath(pluginPath ...string) GenerateOption { // GenerateWithProtocPath returns a new GenerateOption that uses the given protoc // path to the plugin. -func GenerateWithProtocPath(protocPath string) GenerateOption { +func GenerateWithProtocPath(protocPath ...string) GenerateOption { return func(generateOptions *generateOptions) { generateOptions.protocPath = protocPath } @@ -139,14 +139,15 @@ func NewHandler( // Initialize builtin protoc plugin handler. We always look for protoc-gen-X first, // but if not, check the builtins. if _, ok := bufconfig.ProtocProxyPluginNames[pluginName]; ok { - if handlerOptions.protocPath == "" { - handlerOptions.protocPath = "protoc" + if len(handlerOptions.protocPath) == 0 { + handlerOptions.protocPath = []string{"protoc"} } - if protocPath, err := unsafeLookPath(handlerOptions.protocPath); err != nil { + protocPath, protocExtraArgs := handlerOptions.protocPath[0], handlerOptions.protocPath[1:] + protocPath, err := unsafeLookPath(protocPath) + if err != nil { return nil, err - } else { - return newProtocProxyHandler(storageosProvider, runner, tracer, protocPath, pluginName), nil } + return newProtocProxyHandler(storageosProvider, runner, tracer, protocPath, protocExtraArgs, pluginName), nil } return nil, fmt.Errorf( "could not find protoc plugin for name %s - please make sure protoc-gen-%s is installed and present on your $PATH", @@ -162,7 +163,7 @@ type HandlerOption func(*handlerOptions) // // The default is to do exec.LookPath on "protoc". // protocPath is expected to be unnormalized. -func HandlerWithProtocPath(protocPath string) HandlerOption { +func HandlerWithProtocPath(protocPath ...string) HandlerOption { return func(handlerOptions *handlerOptions) { handlerOptions.protocPath = protocPath } @@ -190,8 +191,8 @@ func NewBinaryHandler(runner command.Runner, tracer tracing.Tracer, pluginPath s } type handlerOptions struct { - protocPath string pluginPath []string + protocPath []string } func newHandlerOptions() *handlerOptions { diff --git a/private/buf/bufprotopluginexec/generator.go b/private/buf/bufprotopluginexec/generator.go index 73314dedbb..bb882833af 100644 --- a/private/buf/bufprotopluginexec/generator.go +++ b/private/buf/bufprotopluginexec/generator.go @@ -60,7 +60,7 @@ func (g *generator) Generate( } handlerOptions := []HandlerOption{ HandlerWithPluginPath(generateOptions.pluginPath...), - HandlerWithProtocPath(generateOptions.protocPath), + HandlerWithProtocPath(generateOptions.protocPath...), } handler, err := NewHandler( g.storageosProvider, @@ -84,7 +84,7 @@ func (g *generator) Generate( type generateOptions struct { pluginPath []string - protocPath string + protocPath []string } func newGenerateOptions() *generateOptions { diff --git a/private/buf/bufprotopluginexec/protoc_proxy_handler.go b/private/buf/bufprotopluginexec/protoc_proxy_handler.go index 3680929b54..0f4ae1dc8f 100644 --- a/private/buf/bufprotopluginexec/protoc_proxy_handler.go +++ b/private/buf/bufprotopluginexec/protoc_proxy_handler.go @@ -27,6 +27,7 @@ import ( "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/ioext" "github.com/bufbuild/buf/private/pkg/protoencoding" + "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tmp" @@ -43,6 +44,7 @@ type protocProxyHandler struct { runner command.Runner tracer tracing.Tracer protocPath string + protocExtraArgs []string pluginName string } @@ -51,6 +53,7 @@ func newProtocProxyHandler( runner command.Runner, tracer tracing.Tracer, protocPath string, + protocExtraArgs []string, pluginName string, ) *protocProxyHandler { return &protocProxyHandler{ @@ -58,6 +61,7 @@ func newProtocProxyHandler( runner: runner, tracer: tracer, protocPath: protocPath, + protocExtraArgs: protocExtraArgs, pluginName: pluginName, } } @@ -128,10 +132,10 @@ func (h *protocProxyHandler) Handle( defer func() { retErr = multierr.Append(retErr, tmpDir.Close()) }() - args := []string{ + args := slicesext.Concat(h.protocExtraArgs, []string{ fmt.Sprintf("--descriptor_set_in=%s", descriptorFilePath), fmt.Sprintf("--%s_out=%s", h.pluginName, tmpDir.AbsPath()), - } + }) if getSetExperimentalAllowProto3OptionalFlag(protocVersion) { args = append( args, @@ -167,6 +171,12 @@ func (h *protocProxyHandler) Handle( if getFeatureProto3OptionalSupported(protocVersion) { responseWriter.SetFeatureProto3Optional() } + // We always claim support for all Editions in the response because the invocation to + // "protoc" will fail if it can't handle the input editions. That way, we don't have to + // track which protoc versions support which editions and synthesize this information. + // And that also lets us support users passing "--experimental_editions" to protoc. + responseWriter.SetFeatureSupportsEditions(descriptorpb.Edition_EDITION_PROTO2, descriptorpb.Edition_EDITION_MAX) + // no need for symlinks here, and don't want to support readWriteBucket, err := h.storageosProvider.NewReadWriteBucket(tmpDir.AbsPath()) if err != nil { @@ -195,7 +205,7 @@ func (h *protocProxyHandler) getProtocVersion( if err := h.runner.Run( ctx, h.protocPath, - command.RunWithArgs("--version"), + command.RunWithArgs(slicesext.Concat(h.protocExtraArgs, []string{"--version"})...), command.RunWithEnviron(pluginEnv.Environ), command.RunWithStdout(stdoutBuffer), ); err != nil { diff --git a/private/buf/cmd/buf/command/alpha/protoc/protoc.go b/private/buf/cmd/buf/command/alpha/protoc/protoc.go index f0756ced04..7d959049d6 100644 --- a/private/buf/cmd/buf/command/alpha/protoc/protoc.go +++ b/private/buf/cmd/buf/command/alpha/protoc/protoc.go @@ -73,8 +73,10 @@ Additional flags: BindFlags: flagsBuilder.Bind, NormalizeFlag: flagsBuilder.Normalize, Version: fmt.Sprintf( - "%v.%v.%v-buf", - bufprotopluginexec.DefaultVersion.GetMajor(), + "%v.%v-buf", + // DefaultVersion has an extra major version that corresponds to + // backwards-compatibility level of C++ runtime. The actual version + // of the compiler is just the minor and patch versions. bufprotopluginexec.DefaultVersion.GetMinor(), bufprotopluginexec.DefaultVersion.GetPatch(), ), diff --git a/private/bufpkg/bufconfig/buf_gen_yaml_file.go b/private/bufpkg/bufconfig/buf_gen_yaml_file.go index 4bed47131f..a35511689e 100644 --- a/private/bufpkg/bufconfig/buf_gen_yaml_file.go +++ b/private/bufpkg/bufconfig/buf_gen_yaml_file.go @@ -316,9 +316,9 @@ type externalGeneratePluginConfigV1 struct { // Opt can be one string or multiple strings. Opt interface{} `json:"opt,omitempty" yaml:"opt,omitempty"` // Path can be one string or multiple strings. - Path interface{} `json:"path,omitempty" yaml:"path,omitempty"` - ProtocPath string `json:"protoc_path,omitempty" yaml:"protoc_path,omitempty"` - Strategy string `json:"strategy,omitempty" yaml:"strategy,omitempty"` + Path any `json:"path,omitempty" yaml:"path,omitempty"` + ProtocPath any `json:"protoc_path,omitempty" yaml:"protoc_path,omitempty"` + Strategy string `json:"strategy,omitempty" yaml:"strategy,omitempty"` } // externalGenerateManagedConfigV1 represents the managed mode config in a v1 buf.gen.yaml file. @@ -496,17 +496,18 @@ type externalGeneratePluginConfigV2 struct { // Local is the local path (either relative or absolute) to a binary or other runnable program which // implements the protoc plugin interface. This can be one string (the program) or multiple (remaining // strings are arguments to the program). - Local interface{} `json:"local,omitempty" yaml:"local,omitempty"` + Local any `json:"local,omitempty" yaml:"local,omitempty"` // ProtocBuiltin is the protoc built-in plugin name, in the form of 'java' instead of 'protoc-gen-java'. ProtocBuiltin *string `json:"protoc_builtin,omitempty" yaml:"protoc_builtin,omitempty"` - // ProtocPath is only valid with ProtocBuiltin - ProtocPath *string `json:"protoc_path,omitempty" yaml:"protoc_path,omitempty"` + // ProtocPath is only valid with ProtocBuiltin. This can be one string (the path to protoc) or multiple + // (remaining strings are extra args to pass to protoc). + ProtocPath any `json:"protoc_path,omitempty" yaml:"protoc_path,omitempty"` // Out is required. Out string `json:"out,omitempty" yaml:"out,omitempty"` // Opt can be one string or multiple strings. - Opt interface{} `json:"opt,omitempty" yaml:"opt,omitempty"` - IncludeImports bool `json:"include_imports,omitempty" yaml:"include_imports,omitempty"` - IncludeWKT bool `json:"include_wkt,omitempty" yaml:"include_wkt,omitempty"` + Opt any `json:"opt,omitempty" yaml:"opt,omitempty"` + IncludeImports bool `json:"include_imports,omitempty" yaml:"include_imports,omitempty"` + IncludeWKT bool `json:"include_wkt,omitempty" yaml:"include_wkt,omitempty"` // Strategy is only valid with ProtoBuiltin and Local. Strategy *string `json:"strategy,omitempty" yaml:"strategy,omitempty"` } diff --git a/private/bufpkg/bufconfig/generate_config_test.go b/private/bufpkg/bufconfig/generate_config_test.go index d4ec5c2382..299d62373c 100644 --- a/private/bufpkg/bufconfig/generate_config_test.go +++ b/private/bufpkg/bufconfig/generate_config_test.go @@ -206,7 +206,7 @@ func TestParseConfigFromExternalV1(t *testing.T) { pluginConfigType: PluginConfigTypeProtocBuiltin, name: "cpp", out: "cpp/out", - protocPath: "path/to/protoc", + protocPath: []string{"path/to/protoc"}, }, }, }, @@ -219,7 +219,7 @@ func TestParseConfigFromExternalV1(t *testing.T) { { Plugin: "cpp", Out: "cpp/out", - ProtocPath: "path/to/protoc", + ProtocPath: []any{"path/to/protoc", "--experimental_editions"}, }, }, }, @@ -230,7 +230,7 @@ func TestParseConfigFromExternalV1(t *testing.T) { pluginConfigType: PluginConfigTypeProtocBuiltin, name: "cpp", out: "cpp/out", - protocPath: "path/to/protoc", + protocPath: []string{"path/to/protoc", "--experimental_editions"}, }, }, }, diff --git a/private/bufpkg/bufconfig/generate_plugin_config.go b/private/bufpkg/bufconfig/generate_plugin_config.go index d450cfbc80..e2509438b1 100644 --- a/private/bufpkg/bufconfig/generate_plugin_config.go +++ b/private/bufpkg/bufconfig/generate_plugin_config.go @@ -100,10 +100,10 @@ type GeneratePluginConfig interface { // // This is not empty only when the plugin is local. Path() []string - // ProtocPath returns a path to protoc. + // ProtocPath returns a path to protoc, including any extra arguments. // - // This is not empty only when the plugin is protoc-builtin - ProtocPath() string + // This is not empty only when the plugin is protoc-builtin. + ProtocPath() []string // RemoteHost returns the remote host of the remote plugin. // // This is not empty only when the plugin is remote. @@ -184,7 +184,7 @@ func NewProtocBuiltinPluginConfig( includeImports bool, includeWKT bool, strategy *GenerateStrategy, - protocPath string, + protocPath []string, ) (GeneratePluginConfig, error) { return newProtocBuiltinPluginConfig( name, @@ -229,7 +229,7 @@ type pluginConfig struct { includeWKT bool strategy *GenerateStrategy path []string - protocPath string + protocPath []string remoteHost string revision int } @@ -307,6 +307,10 @@ func newPluginConfigFromExternalV1( if err != nil { return nil, err } + protocPath, err := encoding.InterfaceSliceOrStringToStringSlice(externalConfig.ProtocPath) + if err != nil { + return nil, err + } if externalConfig.Plugin != "" && bufremotepluginref.IsPluginReferenceOrIdentity(pluginIdentifier) { if externalConfig.Path != nil { return nil, fmt.Errorf("cannot specify path for remote plugin %s", externalConfig.Plugin) @@ -314,7 +318,7 @@ func newPluginConfigFromExternalV1( if externalConfig.Strategy != "" { return nil, fmt.Errorf("cannot specify strategy for remote plugin %s", externalConfig.Plugin) } - if externalConfig.ProtocPath != "" { + if externalConfig.ProtocPath != nil { return nil, fmt.Errorf("cannot specify protoc_path for remote plugin %s", externalConfig.Plugin) } return newRemotePluginConfig( @@ -339,7 +343,7 @@ func newPluginConfigFromExternalV1( path, ) } - if externalConfig.ProtocPath != "" { + if externalConfig.ProtocPath != nil { return newProtocBuiltinPluginConfig( pluginIdentifier, externalConfig.Out, @@ -347,7 +351,7 @@ func newPluginConfigFromExternalV1( false, false, strategy, - externalConfig.ProtocPath, + protocPath, ) } // It could be either local or protoc built-in. We defer to the plugin executor @@ -438,9 +442,9 @@ func newPluginConfigFromExternalV2( path, ) case externalConfig.ProtocBuiltin != nil: - var protocPath string - if externalConfig.ProtocPath != nil { - protocPath = *externalConfig.ProtocPath + protocPath, err := encoding.InterfaceSliceOrStringToStringSlice(externalConfig.ProtocPath) + if err != nil { + return nil, err } if externalConfig.Revision != nil { return nil, fmt.Errorf("cannot specify revision for protoc built-in plugin %s", *externalConfig.ProtocBuiltin) @@ -545,7 +549,7 @@ func newProtocBuiltinPluginConfig( includeImports bool, includeWKT bool, strategy *GenerateStrategy, - protocPath string, + protocPath []string, ) (*pluginConfig, error) { if includeWKT && !includeImports { return nil, errors.New("cannot include well-known types without including imports") @@ -597,7 +601,7 @@ func (p *pluginConfig) Path() []string { return p.path } -func (p *pluginConfig) ProtocPath() string { +func (p *pluginConfig) ProtocPath() []string { return p.protocPath } @@ -653,8 +657,12 @@ func newExternalGeneratePluginConfigV2FromPluginConfig( } case PluginConfigTypeProtocBuiltin: externalPluginConfigV2.ProtocBuiltin = toPointer(generatePluginConfig.Name()) - if protocPath := generatePluginConfig.ProtocPath(); protocPath != "" { - externalPluginConfigV2.ProtocPath = &protocPath + if protocPath := generatePluginConfig.ProtocPath(); len(protocPath) > 0 { + if len(protocPath) == 1 { + externalPluginConfigV2.ProtocPath = protocPath[0] + } else { + externalPluginConfigV2.ProtocPath = protocPath + } } case PluginConfigTypeLocalOrProtocBuiltin: binaryName := "protoc-gen-" + generatePluginConfig.Name() diff --git a/private/pkg/encoding/encoding.go b/private/pkg/encoding/encoding.go index e317c2280b..0fddc370bd 100644 --- a/private/pkg/encoding/encoding.go +++ b/private/pkg/encoding/encoding.go @@ -183,7 +183,7 @@ func InterfaceSliceOrStringToStringSlice(in interface{}) ([]string, error) { switch t := in.(type) { case string: return []string{t}, nil - case []interface{}: + case []any: if len(t) == 0 { return nil, nil }