Skip to content

Commit

Permalink
Update bufcheck.RunnerProvider to use the plugin config
Browse files Browse the repository at this point in the history
Changes bufcheck.RunnerProvider to take the bufconfig.PluginConfig as
the argurment for creating the runner. This allows for setting the
runner based on configuration other than the plugin path. A helper has
been provided to create local runners for a command.Runner.
  • Loading branch information
emcfarlane committed Sep 18, 2024
1 parent 26ce1b0 commit f214164
Show file tree
Hide file tree
Showing 15 changed files with 43 additions and 64 deletions.
3 changes: 1 addition & 2 deletions private/buf/bufmigrate/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/bufbuild/buf/private/bufpkg/bufmodule"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/normalpath"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/storage"
"github.com/bufbuild/buf/private/pkg/storage/storagemem"
Expand Down Expand Up @@ -713,7 +712,7 @@ func equivalentCheckConfigInV2(
) (bufconfig.CheckConfig, error) {
// No need for custom lint/breaking plugins since there's no plugins to migrate from <=v1.
// TODO: If we ever need v3, then we will have to deal with this.
client, err := bufcheck.NewClient(logger, tracer, pluginrpcutil.NewRunnerProvider(runner))
client, err := bufcheck.NewClient(logger, tracer, bufcheck.NewRunnerProvider(runner))
if err != nil {
return nil, err
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
"github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/osext"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/storage/storageos"
"github.com/bufbuild/buf/private/pkg/storage/storagetesting"
Expand Down Expand Up @@ -1350,7 +1349,7 @@ func TestCheckLsBreakingRulesFromConfigExceptDeprecated(t *testing.T) {
t.Run(version.String(), func(t *testing.T) {
t.Parallel()
// Do not need any custom lint/breaking plugins here.
client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()))
client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner()))
require.NoError(t, err)
allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version)
require.NoError(t, err)
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/breaking/breaking.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/stringutil"
"github.com/bufbuild/buf/private/pkg/tracing"
Expand Down Expand Up @@ -210,7 +209,7 @@ func run(
tracer := tracing.NewTracer(container.Tracer())
var allFileAnnotations []bufanalysis.FileAnnotation
for i, imageWithConfig := range imageWithConfigs {
client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/config/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/normalpath"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/stringutil"
"github.com/bufbuild/buf/private/pkg/syserror"
Expand Down Expand Up @@ -186,7 +185,7 @@ func lsRun(
}
}
tracer := tracing.NewTracer(container.Tracer())
client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/stringutil"
"github.com/bufbuild/buf/private/pkg/tracing"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -135,7 +134,7 @@ func run(
tracer := tracing.NewTracer(container.Tracer())
var allFileAnnotations []bufanalysis.FileAnnotation
for _, imageWithConfig := range imageWithConfigs {
client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/mod/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/stringutil"
"github.com/bufbuild/buf/private/pkg/syserror"
Expand Down Expand Up @@ -176,7 +175,7 @@ func lsRun(
}
// BufYAMLFiles <=v1 never had plugins.
tracer := tracing.NewTracer(container.Tracer())
client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()))
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/protoc-gen-buf-breaking/breaking.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/bufbuild/buf/private/bufpkg/bufimage"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/encoding"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/protodescriptor"
"github.com/bufbuild/buf/private/pkg/protoencoding"
"github.com/bufbuild/buf/private/pkg/tracing"
Expand Down Expand Up @@ -126,7 +125,7 @@ func handle(
}
// The protoc plugins do not support custom lint/breaking change plugins for now.
tracer := tracing.NewTracer(container.Tracer())
client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr))
client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr))
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/protoc-gen-buf-lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/bufbuild/buf/private/bufpkg/bufimage"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/encoding"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/protodescriptor"
"github.com/bufbuild/buf/private/pkg/protoencoding"
"github.com/bufbuild/buf/private/pkg/tracing"
Expand Down Expand Up @@ -101,7 +100,7 @@ func handle(
}
// The protoc plugins do not support custom lint/breaking change plugins for now.
tracer := tracing.NewTracer(container.Tracer())
client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr))
client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr))
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions private/bufpkg/bufcheck/breaking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/bufbuild/buf/private/bufpkg/bufimage"
"github.com/bufbuild/buf/private/bufpkg/bufmodule"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/storage/storageos"
"github.com/bufbuild/buf/private/pkg/tracing"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1348,7 +1347,7 @@ func testBreaking(
require.NoError(t, err)
breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID)
require.NotNil(t, breakingConfig)
client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()))
client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner()))
require.NoError(t, err)
err = client.Breaking(
ctx,
Expand Down
30 changes: 25 additions & 5 deletions private/bufpkg/bufcheck/bufcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"buf.build/go/bufplugin/check"
"github.com/bufbuild/buf/private/bufpkg/bufconfig"
"github.com/bufbuild/buf/private/bufpkg/bufimage"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/syserror"
"github.com/bufbuild/buf/private/pkg/tracing"
Expand Down Expand Up @@ -162,17 +164,35 @@ func WithPluginsEnabled() ClientFunctionOption {
return pluginsEnabledOption{}
}

// RunnerProvider provides pluginrpc.Runners for program names and args.
// RunnerProvider provides pluginrpc.Runners for a given plugin config.
type RunnerProvider interface {
NewRunner(programName string, programArgs ...string) pluginrpc.Runner
NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error)
}

// RunnerProviderFunc is a function that implements RunnerProvider.
type RunnerProviderFunc func(programName string, programArgs ...string) pluginrpc.Runner
type RunnerProviderFunc func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error)

// NewRunner implements RunnerProvider.
func (r RunnerProviderFunc) NewRunner(programName string, programArgs ...string) pluginrpc.Runner {
return r(programName, programArgs...)
func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) {
return r(pluginConfig)
}

// NewRunnerProvider returns a new RunnerProvider for the command.Runner.
func NewRunnerProvider(delegate command.Runner) RunnerProvider {
return RunnerProviderFunc(
func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) {
if pluginConfig.Type() != bufconfig.PluginConfigTypeLocal {
return nil, syserror.New("we only handle local plugins for now with lint and breaking")
}
path := pluginConfig.Path()
return pluginrpcutil.NewRunner(
delegate,
// We know that Path is of at least length 1.
path[0],
path[1:]...,
), nil
},
)
}

// NewClient returns a new Client.
Expand Down
14 changes: 5 additions & 9 deletions private/bufpkg/bufcheck/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,21 +331,17 @@ func (c *client) getMultiClient(
)
}
for _, pluginConfig := range pluginConfigs {
if pluginConfig.Type() != bufconfig.PluginConfigTypeLocal {
return nil, syserror.New("we only handle local plugins for now with lint and breaking")
}
options, err := check.NewOptions(pluginConfig.Options())
if err != nil {
return nil, fmt.Errorf("could not parse options for plugin %q: %w", pluginConfig.Name(), err)
}
pluginPath := pluginConfig.Path()
runner, err := c.runnerProvider.NewRunner(pluginConfig)
if err != nil {
return nil, fmt.Errorf("could not create runner for plugin %q: %w", pluginConfig.Name(), err)
}
checkClient := check.NewClient(
pluginrpc.NewClient(
c.runnerProvider.NewRunner(
// We know that Path is of at least length 1.
pluginPath[0],
pluginPath[1:]...,
),
runner,
pluginrpc.ClientWithStderr(c.stderr),
// We have to set binary as some things cannot be encoded as JSON.
// Example: google.protobuf.Timestamps with positive seconds and negative nanos.
Expand Down
3 changes: 1 addition & 2 deletions private/bufpkg/bufcheck/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/bufbuild/buf/private/bufpkg/bufimage"
"github.com/bufbuild/buf/private/bufpkg/bufmodule"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/storage/storageos"
"github.com/bufbuild/buf/private/pkg/tracing"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1294,7 +1293,7 @@ func testLintWithOptions(

lintConfig := workspace.GetLintConfigForOpaqueID(opaqueID)
require.NotNil(t, lintConfig)
client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()))
client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner()))
require.NoError(t, err)
err = client.Lint(
ctx,
Expand Down
2 changes: 1 addition & 1 deletion private/bufpkg/bufcheck/multi_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ func (c *multiClient) Check(ctx context.Context, request check.Request) ([]*anno
}
return fmt.Errorf("plugin %q failed: %w", delegate.PluginName, err)
}
lock.Lock()
annotations := slicesext.Map(
delegateResponse.Annotations(),
func(checkAnnotation check.Annotation) *annotation {
return newAnnotation(checkAnnotation, delegate.PluginName)
},
)
lock.Lock()
allAnnotations = append(allAnnotations, annotations...)
lock.Unlock()
return nil
Expand Down
5 changes: 2 additions & 3 deletions private/bufpkg/bufcheck/multi_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"buf.build/go/bufplugin/check/checkutil"
"github.com/bufbuild/buf/private/bufpkg/bufconfig"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/stringutil"
"github.com/bufbuild/buf/private/pkg/tracing"
Expand Down Expand Up @@ -185,7 +184,7 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) {
client, err := newClient(
zaptest.NewLogger(t),
tracing.NopTracer,
pluginrpcutil.NewRunnerProvider(command.NewRunner()),
NewRunnerProvider(command.NewRunner()),
)
require.NoError(t, err)
duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig(
Expand Down Expand Up @@ -279,7 +278,7 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) {
client, err := newClient(
zaptest.NewLogger(t),
tracing.NopTracer,
pluginrpcutil.NewRunnerProvider(command.NewRunner()),
NewRunnerProvider(command.NewRunner()),
)
require.NoError(t, err)
duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig(
Expand Down
26 changes: 0 additions & 26 deletions private/pkg/pluginrpcutil/pluginrpcutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,3 @@ import (
func NewRunner(delegate command.Runner, programName string, programArgs ...string) pluginrpc.Runner {
return newRunner(delegate, programName, programArgs...)
}

// RunnerProvider provides pluginrpc.Runners for program names and args.
type RunnerProvider interface {
NewRunner(programName string, programArgs ...string) pluginrpc.Runner
}

// RunnerProviderFunc is a function that implements RunnerProvider.
type RunnerProviderFunc func(programName string, programArgs ...string) pluginrpc.Runner

// NewRunner implements RunnerProvider.
func (r RunnerProviderFunc) NewRunner(programName string, programArgs ...string) pluginrpc.Runner {
return r(programName, programArgs...)
}

// NewRunnerProvider returns a new RunnerProvider for the command.Runner.
func NewRunnerProvider(delegate command.Runner) RunnerProvider {
return RunnerProviderFunc(
func(programName string, programArgs ...string) pluginrpc.Runner {
return NewRunner(
delegate,
programName,
programArgs...,
)
},
)
}

0 comments on commit f214164

Please sign in to comment.