Allow protoc_path to be a slice so user can specify extra args (#3098)

With this, the user can specify, for example `--experimental_editions`.
When proxying to `protoc`, we can synthesize support for all editions
since `protoc` will actually handle the validation and fail if it can't
handle an edition.

Author: jhump

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
 

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)

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 {

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 {

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,13 +53,15 @@ func newProtocProxyHandler(
 	runner command.Runner,
 	tracer tracing.Tracer,
 	protocPath string,
+	protocExtraArgs []string,
 	pluginName string,
 ) *protocProxyHandler {
 	return &protocProxyHandler{
 		storageosProvider: storageosProvider,
 		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 {

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(),
 		),

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"`
 }

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"},
 					},
 				},
 			},

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,14 +307,18 @@ 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)
 		}
 		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,15 +343,15 @@ func newPluginConfigFromExternalV1(
 			path,
 		)
 	}
-	if externalConfig.ProtocPath != "" {
+	if externalConfig.ProtocPath != nil {
 		return newProtocBuiltinPluginConfig(
 			pluginIdentifier,
 			externalConfig.Out,
 			opt,
 			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()

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
 		}