diff --git a/.golangci.yml b/.golangci.yml index de17f1bf56..a53b93cc05 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,7 +5,7 @@ linters-settings: forbid: # Use private/pkg/thread.Parallelize - '^errgroup\.' - # Use private/pkg/command.Runner + # Use private/pkg/execext - '^exec\.Cmd$' - '^exec\.Command$' - '^exec\.CommandContext$' @@ -213,6 +213,10 @@ issues: - containedctx # we actually want to embed a context here path: private/bufpkg/bufmodule/module_set_builder.go + - linters: + - containedctx + # we actually want to embed a context here + path: private/pkg/execext/process.go - linters: - gochecknoinits # we actually want to use init here @@ -225,15 +229,15 @@ issues: - linters: - forbidigo # this is one of two files we want to allow exec.Cmd functions in - path: private/pkg/command/process.go + path: private/pkg/execext/execext.go - linters: - forbidigo # this is one of two files we want to allow exec.Cmd functions in - path: private/pkg/command/runner.go + path: private/pkg/execext/process.go - linters: - gosec # G204 checks that exec.Command is not called with non-constants. - path: private/pkg/command/runner.go + path: private/pkg/execext/execext.go text: "G204:" - linters: - gosec diff --git a/private/buf/bufcli/cache.go b/private/buf/bufcli/cache.go index ddadfb50dc..cfe01a9579 100644 --- a/private/buf/bufcli/cache.go +++ b/private/buf/bufcli/cache.go @@ -29,7 +29,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufregistryapi/bufregistryapimodule" "github.com/bufbuild/buf/private/bufpkg/bufregistryapi/bufregistryapiowner" "github.com/bufbuild/buf/private/pkg/app/appext" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/filelock" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/storage/storageos" @@ -174,7 +173,6 @@ func NewWKTStore(container appext.Container) (bufwktstore.Store, error) { } return bufwktstore.NewStore( container.Logger(), - command.NewRunner(), cacheBucket, ), nil } diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index 0e1322348a..d6c044b7ea 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -38,7 +38,6 @@ import ( imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/app/appcmd" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/git" "github.com/bufbuild/buf/private/pkg/httpauth" "github.com/bufbuild/buf/private/pkg/ioext" @@ -178,7 +177,6 @@ type controller struct { fileAnnotationsToStdout bool copyToInMemory bool - commandRunner command.Runner storageosProvider storageos.Provider buffetchRefParser buffetch.RefParser buffetchReader buffetch.Reader @@ -214,7 +212,6 @@ func newController( if err := validateFileAnnotationErrorFormat(controller.fileAnnotationErrorFormat); err != nil { return nil, err } - controller.commandRunner = command.NewRunner() controller.storageosProvider = newStorageosProvider(controller.disableSymlinks) controller.buffetchRefParser = buffetch.NewRefParser(logger) controller.buffetchReader = buffetch.NewReader( @@ -225,7 +222,6 @@ func newController( git.NewCloner( logger, controller.storageosProvider, - controller.commandRunner, gitClonerOptions, ), moduleKeyProvider, diff --git a/private/buf/bufformat/formatter_test.go b/private/buf/bufformat/formatter_test.go index f57e1ede99..c8168437d6 100644 --- a/private/buf/bufformat/formatter_test.go +++ b/private/buf/bufformat/formatter_test.go @@ -21,7 +21,6 @@ import ( "testing" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/diff" "github.com/bufbuild/buf/private/pkg/slogtestext" "github.com/bufbuild/buf/private/pkg/storage" @@ -72,7 +71,6 @@ func testFormatProto3(t *testing.T) { func testFormatNoDiff(t *testing.T, path string) { t.Run(path, func(t *testing.T) { ctx := context.Background() - runner := command.NewRunner() bucket, err := storageos.NewProvider().NewReadWriteBucket(path) require.NoError(t, err) moduleSetBuilder := bufmodule.NewModuleSetBuilder(ctx, slogtestext.NewLogger(t), bufmodule.NopModuleDataProvider, bufmodule.NopCommitProvider) @@ -102,7 +100,7 @@ func testFormatNoDiff(t *testing.T, path string) { require.NoError(t, err) expectedData, err := io.ReadAll(expectedFile) require.NoError(t, err) - fileDiff, err := diff.Diff(ctx, runner, expectedData, formattedData, expectedPath, formattedFile.Path()+" (formatted)") + fileDiff, err := diff.Diff(ctx, expectedData, formattedData, expectedPath, formattedFile.Path()+" (formatted)") require.NoError(t, err) require.Empty(t, string(fileDiff)) }) diff --git a/private/buf/bufgen/bufgen.go b/private/buf/bufgen/bufgen.go index e0b0d94139..42790a9eb8 100644 --- a/private/buf/bufgen/bufgen.go +++ b/private/buf/bufgen/bufgen.go @@ -26,7 +26,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/connectclient" "github.com/bufbuild/buf/private/pkg/storage/storageos" ) @@ -88,7 +87,6 @@ type Generator interface { func NewGenerator( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, // Pass a clientConfig instead of a CodeGenerationServiceClient because the // plugins' remotes/registries is not known at this time, and remotes/registries // may be different for different plugins. @@ -97,7 +95,6 @@ func NewGenerator( return newGenerator( logger, storageosProvider, - runner, clientConfig, ) } diff --git a/private/buf/bufgen/generator.go b/private/buf/bufgen/generator.go index 2fe4905f78..2832f34864 100644 --- a/private/buf/bufgen/generator.go +++ b/private/buf/bufgen/generator.go @@ -33,7 +33,6 @@ import ( "github.com/bufbuild/buf/private/gen/proto/connect/buf/alpha/registry/v1alpha1/registryv1alpha1connect" registryv1alpha1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/registry/v1alpha1" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/connectclient" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage/storageos" @@ -52,13 +51,12 @@ type generator struct { func newGenerator( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, clientConfig *connectclient.Config, ) *generator { return &generator{ logger: logger, storageosProvider: storageosProvider, - pluginexecGenerator: bufprotopluginexec.NewGenerator(logger, storageosProvider, runner), + pluginexecGenerator: bufprotopluginexec.NewGenerator(logger, storageosProvider), clientConfig: clientConfig, } } diff --git a/private/buf/bufmigrate/bufmigrate.go b/private/buf/bufmigrate/bufmigrate.go index be63386b43..dd52b79ac1 100644 --- a/private/buf/bufmigrate/bufmigrate.go +++ b/private/buf/bufmigrate/bufmigrate.go @@ -22,7 +22,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "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/storage" "github.com/bufbuild/buf/private/pkg/syserror" @@ -77,13 +76,11 @@ type Migrator interface { // NewMigrator returns a new Migrator. func NewMigrator( logger *slog.Logger, - runner command.Runner, moduleKeyProvider bufmodule.ModuleKeyProvider, commitProvider bufmodule.CommitProvider, ) Migrator { return newMigrator( logger, - runner, moduleKeyProvider, commitProvider, ) diff --git a/private/buf/bufmigrate/migrate_builder.go b/private/buf/bufmigrate/migrate_builder.go index 3166039f72..602a94309a 100644 --- a/private/buf/bufmigrate/migrate_builder.go +++ b/private/buf/bufmigrate/migrate_builder.go @@ -23,7 +23,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "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/slicesext" "github.com/bufbuild/buf/private/pkg/storage" @@ -35,7 +34,6 @@ import ( type migrateBuilder struct { logger *slog.Logger - runner command.Runner commitProvider bufmodule.CommitProvider bucket storage.ReadBucket destinationDirPath string @@ -55,14 +53,12 @@ type migrateBuilder struct { func newMigrateBuilder( logger *slog.Logger, - runner command.Runner, commitProvider bufmodule.CommitProvider, bucket storage.ReadBucket, destinationDirPath string, ) *migrateBuilder { return &migrateBuilder{ logger: logger, - runner: runner, commitProvider: commitProvider, bucket: bucket, destinationDirPath: destinationDirPath, @@ -268,11 +264,11 @@ func (m *migrateBuilder) addModule(ctx context.Context, moduleDirPath string) (r if err != nil { return err } - lintConfigForRoot, err := equivalentLintConfigInV2(ctx, m.logger, m.runner, moduleConfig.LintConfig()) + lintConfigForRoot, err := equivalentLintConfigInV2(ctx, m.logger, moduleConfig.LintConfig()) if err != nil { return err } - breakingConfigForRoot, err := equivalentBreakingConfigInV2(ctx, m.logger, m.runner, moduleConfig.BreakingConfig()) + breakingConfigForRoot, err := equivalentBreakingConfigInV2(ctx, m.logger, moduleConfig.BreakingConfig()) if err != nil { return err } @@ -304,11 +300,11 @@ func (m *migrateBuilder) addModule(ctx context.Context, moduleDirPath string) (r if err != nil { return err } - lintConfig, err := equivalentLintConfigInV2(ctx, m.logger, m.runner, moduleConfig.LintConfig()) + lintConfig, err := equivalentLintConfigInV2(ctx, m.logger, moduleConfig.LintConfig()) if err != nil { return err } - breakingConfig, err := equivalentBreakingConfigInV2(ctx, m.logger, m.runner, moduleConfig.BreakingConfig()) + breakingConfig, err := equivalentBreakingConfigInV2(ctx, m.logger, moduleConfig.BreakingConfig()) if err != nil { return err } diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index 37d38bf5e9..0f220c9cad 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/private/buf/bufmigrate/migrator.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "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/slicesext" "github.com/bufbuild/buf/private/pkg/storage" @@ -40,20 +39,17 @@ import ( type migrator struct { logger *slog.Logger - runner command.Runner moduleKeyProvider bufmodule.ModuleKeyProvider commitProvider bufmodule.CommitProvider } func newMigrator( logger *slog.Logger, - runner command.Runner, moduleKeyProvider bufmodule.ModuleKeyProvider, commitProvider bufmodule.CommitProvider, ) *migrator { return &migrator{ logger: logger, - runner: runner, moduleKeyProvider: moduleKeyProvider, commitProvider: commitProvider, } @@ -140,7 +136,6 @@ func (m *migrator) getMigrateBuilder( } migrateBuilder := newMigrateBuilder( m.logger, - m.runner, m.commitProvider, bucket, destinationDirPath, @@ -209,7 +204,6 @@ func (m *migrator) diff( } return storage.Diff( ctx, - m.runner, writer, originalFileBucket, addedFileBucket, @@ -647,13 +641,11 @@ func resolvedDeclaredAndLockedDependencies( func equivalentLintConfigInV2( ctx context.Context, logger *slog.Logger, - runner command.Runner, lintConfig bufconfig.LintConfig, ) (bufconfig.LintConfig, error) { equivalentCheckConfigV2, err := equivalentCheckConfigInV2( ctx, logger, - runner, check.RuleTypeLint, lintConfig, ) @@ -674,13 +666,11 @@ func equivalentLintConfigInV2( func equivalentBreakingConfigInV2( ctx context.Context, logger *slog.Logger, - runner command.Runner, breakingConfig bufconfig.BreakingConfig, ) (bufconfig.BreakingConfig, error) { equivalentCheckConfigV2, err := equivalentCheckConfigInV2( ctx, logger, - runner, check.RuleTypeBreaking, breakingConfig, ) @@ -698,13 +688,12 @@ func equivalentBreakingConfigInV2( func equivalentCheckConfigInV2( ctx context.Context, logger *slog.Logger, - runner command.Runner, ruleType check.RuleType, checkConfig bufconfig.CheckConfig, ) (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, bufcheck.NewRunnerProvider(runner, wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(logger, bufcheck.NewRunnerProvider(wasm.UnimplementedRuntime)) if err != nil { return nil, err } diff --git a/private/buf/bufprotopluginexec/binary_handler.go b/private/buf/bufprotopluginexec/binary_handler.go index c617a5da29..98e2f284cd 100644 --- a/private/buf/bufprotopluginexec/binary_handler.go +++ b/private/buf/bufprotopluginexec/binary_handler.go @@ -21,7 +21,7 @@ import ( "log/slog" "path/filepath" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/ioext" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/slogext" @@ -31,20 +31,17 @@ import ( type binaryHandler struct { logger *slog.Logger - runner command.Runner pluginPath string pluginArgs []string } func newBinaryHandler( logger *slog.Logger, - runner command.Runner, pluginPath string, pluginArgs []string, ) *binaryHandler { return &binaryHandler{ logger: logger, - runner: runner, pluginPath: pluginPath, pluginArgs: pluginArgs, } @@ -64,16 +61,16 @@ func (h *binaryHandler) Handle( } responseBuffer := bytes.NewBuffer(nil) stderrWriteCloser := newStderrWriteCloser(pluginEnv.Stderr, h.pluginPath) - runOptions := []command.RunOption{ - command.RunWithEnviron(pluginEnv.Environ), - command.RunWithStdin(bytes.NewReader(requestData)), - command.RunWithStdout(responseBuffer), - command.RunWithStderr(stderrWriteCloser), + runOptions := []execext.RunOption{ + execext.WithEnv(pluginEnv.Environ), + execext.WithStdin(bytes.NewReader(requestData)), + execext.WithStdout(responseBuffer), + execext.WithStderr(stderrWriteCloser), } if len(h.pluginArgs) > 0 { - runOptions = append(runOptions, command.RunWithArgs(h.pluginArgs...)) + runOptions = append(runOptions, execext.WithArgs(h.pluginArgs...)) } - if err := h.runner.Run( + if err := execext.Run( ctx, h.pluginPath, runOptions..., diff --git a/private/buf/bufprotopluginexec/bufprotopluginexec.go b/private/buf/bufprotopluginexec/bufprotopluginexec.go index c81ad755f3..8226d7cc16 100644 --- a/private/buf/bufprotopluginexec/bufprotopluginexec.go +++ b/private/buf/bufprotopluginexec/bufprotopluginexec.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/protoplugin" "google.golang.org/protobuf/types/pluginpb" @@ -77,9 +76,8 @@ type Generator interface { func NewGenerator( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, ) Generator { - return newGenerator(logger, storageosProvider, runner) + return newGenerator(logger, storageosProvider) } // GenerateOption is an option for Generate. @@ -114,7 +112,6 @@ func GenerateWithProtocPath(protocPath ...string) GenerateOption { func NewHandler( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, pluginName string, options ...HandlerOption, ) (protoplugin.Handler, error) { @@ -126,11 +123,11 @@ func NewHandler( // Initialize binary plugin handler when path is specified with optional args. Return // on error as something is wrong with the supplied pluginPath option. if len(handlerOptions.pluginPath) > 0 { - return NewBinaryHandler(logger, runner, handlerOptions.pluginPath[0], handlerOptions.pluginPath[1:]) + return NewBinaryHandler(logger, handlerOptions.pluginPath[0], handlerOptions.pluginPath[1:]) } // Initialize binary plugin handler based on plugin name. - if handler, err := NewBinaryHandler(logger, runner, "protoc-gen-"+pluginName, nil); err == nil { + if handler, err := NewBinaryHandler(logger, "protoc-gen-"+pluginName, nil); err == nil { return handler, nil } @@ -145,7 +142,7 @@ func NewHandler( if err != nil { return nil, err } - return newProtocProxyHandler(logger, storageosProvider, runner, protocPath, protocExtraArgs, pluginName), nil + return newProtocProxyHandler(logger, storageosProvider, 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", @@ -180,12 +177,12 @@ func HandlerWithPluginPath(pluginPath ...string) HandlerOption { // NewBinaryHandler returns a new Handler that invokes the specific plugin // specified by pluginPath. -func NewBinaryHandler(logger *slog.Logger, runner command.Runner, pluginPath string, pluginArgs []string) (protoplugin.Handler, error) { +func NewBinaryHandler(logger *slog.Logger, pluginPath string, pluginArgs []string) (protoplugin.Handler, error) { pluginPath, err := unsafeLookPath(pluginPath) if err != nil { return nil, err } - return newBinaryHandler(logger, runner, pluginPath, pluginArgs), nil + return newBinaryHandler(logger, pluginPath, pluginArgs), nil } type handlerOptions struct { diff --git a/private/buf/bufprotopluginexec/generator.go b/private/buf/bufprotopluginexec/generator.go index 1aec1a244f..6cbe690701 100644 --- a/private/buf/bufprotopluginexec/generator.go +++ b/private/buf/bufprotopluginexec/generator.go @@ -20,7 +20,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufprotoplugin" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "google.golang.org/protobuf/types/pluginpb" ) @@ -28,18 +27,15 @@ import ( type generator struct { logger *slog.Logger storageosProvider storageos.Provider - runner command.Runner } func newGenerator( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, ) *generator { return &generator{ logger: logger, storageosProvider: storageosProvider, - runner: runner, } } @@ -61,7 +57,6 @@ func (g *generator) Generate( handler, err := NewHandler( g.logger, g.storageosProvider, - g.runner, pluginName, handlerOptions..., ) diff --git a/private/buf/bufprotopluginexec/protoc_proxy_handler.go b/private/buf/bufprotopluginexec/protoc_proxy_handler.go index 973ee17096..0a06674a95 100644 --- a/private/buf/bufprotopluginexec/protoc_proxy_handler.go +++ b/private/buf/bufprotopluginexec/protoc_proxy_handler.go @@ -25,7 +25,7 @@ import ( "strings" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/ioext" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -42,7 +42,6 @@ import ( type protocProxyHandler struct { logger *slog.Logger storageosProvider storageos.Provider - runner command.Runner protocPath string protocExtraArgs []string pluginName string @@ -51,7 +50,6 @@ type protocProxyHandler struct { func newProtocProxyHandler( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, protocPath string, protocExtraArgs []string, pluginName string, @@ -59,7 +57,6 @@ func newProtocProxyHandler( return &protocProxyHandler{ logger: logger, storageosProvider: storageosProvider, - runner: runner, protocPath: protocPath, protocExtraArgs: protocExtraArgs, pluginName: pluginName, @@ -149,13 +146,13 @@ func (h *protocProxyHandler) Handle( if descriptorFilePath != "" && descriptorFilePath == app.DevStdinFilePath { stdin = bytes.NewReader(fileDescriptorSetData) } - if err := h.runner.Run( + if err := execext.Run( ctx, h.protocPath, - command.RunWithArgs(args...), - command.RunWithEnviron(pluginEnv.Environ), - command.RunWithStdin(stdin), - command.RunWithStderr(pluginEnv.Stderr), + execext.WithArgs(args...), + execext.WithEnv(pluginEnv.Environ), + execext.WithStdin(stdin), + execext.WithStderr(pluginEnv.Stderr), ); err != nil { // TODO: strip binary path as well? // We don't know if this is a system error or plugin error, so we assume system error @@ -195,12 +192,12 @@ func (h *protocProxyHandler) getProtocVersion( pluginEnv protoplugin.PluginEnv, ) (*pluginpb.Version, error) { stdoutBuffer := bytes.NewBuffer(nil) - if err := h.runner.Run( + if err := execext.Run( ctx, h.protocPath, - command.RunWithArgs(slicesext.Concat(h.protocExtraArgs, []string{"--version"})...), - command.RunWithEnviron(pluginEnv.Environ), - command.RunWithStdout(stdoutBuffer), + execext.WithArgs(slicesext.Concat(h.protocExtraArgs, []string{"--version"})...), + execext.WithEnv(pluginEnv.Environ), + execext.WithStdout(stdoutBuffer), ); err != nil { // TODO: strip binary path as well? return nil, handlePotentialTooManyFilesError(err) diff --git a/private/buf/buftesting/buftesting.go b/private/buf/buftesting/buftesting.go index 1942e1ca3b..763505bc4a 100644 --- a/private/buf/buftesting/buftesting.go +++ b/private/buf/buftesting/buftesting.go @@ -24,7 +24,6 @@ import ( "github.com/bufbuild/buf/private/buf/bufprotoc" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/github/githubtesting" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/prototesting" @@ -59,7 +58,6 @@ var ( // GetActualProtocFileDescriptorSet gets the FileDescriptorSet for actual protoc. func GetActualProtocFileDescriptorSet( t *testing.T, - runner command.Runner, includeImports bool, includeSourceInfo bool, dirPath string, @@ -67,7 +65,6 @@ func GetActualProtocFileDescriptorSet( ) *descriptorpb.FileDescriptorSet { fileDescriptorSet, err := prototesting.GetProtocFileDescriptorSet( context.Background(), - runner, []string{dirPath}, filePaths, includeImports, @@ -80,7 +77,6 @@ func GetActualProtocFileDescriptorSet( // RunActualProtoc runs actual protoc. func RunActualProtoc( t *testing.T, - runner command.Runner, includeImports bool, includeSourceInfo bool, dirPath string, @@ -91,7 +87,6 @@ func RunActualProtoc( ) { err := prototesting.RunProtoc( context.Background(), - runner, []string{dirPath}, filePaths, includeImports, diff --git a/private/buf/bufwkt/bufwktstore/bufwktstore.go b/private/buf/bufwkt/bufwktstore/bufwktstore.go index d0f5dc597f..8377557768 100644 --- a/private/buf/bufwkt/bufwktstore/bufwktstore.go +++ b/private/buf/bufwkt/bufwktstore/bufwktstore.go @@ -18,7 +18,6 @@ import ( "context" "log/slog" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage" ) @@ -35,8 +34,7 @@ type Store interface { // It is assumed that the Store has complete control of the bucket. func NewStore( logger *slog.Logger, - runner command.Runner, bucket storage.ReadWriteBucket, ) Store { - return newStore(logger, runner, bucket) + return newStore(logger, bucket) } diff --git a/private/buf/bufwkt/bufwktstore/store.go b/private/buf/bufwkt/bufwktstore/store.go index 926fb6a9db..835d8ba978 100644 --- a/private/buf/bufwkt/bufwktstore/store.go +++ b/private/buf/bufwkt/bufwktstore/store.go @@ -19,24 +19,20 @@ import ( "log/slog" "github.com/bufbuild/buf/private/gen/data/datawkt" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage" ) type store struct { logger *slog.Logger - runner command.Runner bucket storage.ReadWriteBucket } func newStore( logger *slog.Logger, - runner command.Runner, bucket storage.ReadWriteBucket, ) *store { return &store{ logger: logger, - runner: runner, bucket: bucket, } } @@ -53,7 +49,7 @@ func (s *store) GetBucket(ctx context.Context) (storage.ReadBucket, error) { return nil, err } } else { - diff, err := storage.DiffBytes(ctx, s.runner, datawkt.ReadBucket, wktBucket) + diff, err := storage.DiffBytes(ctx, datawkt.ReadBucket, wktBucket) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go index 18fe6e26c8..1bb1c22b16 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/private/buf/cmd/buf/buf_test.go @@ -38,7 +38,6 @@ import ( imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/private/pkg/app/appcmd" "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/protoencoding" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -1349,7 +1348,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(slogtestext.NewLogger(t), bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(slogtestext.NewLogger(t), bufcheck.NewRunnerProvider(wasm.UnimplementedRuntime)) require.NoError(t, err) allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version) require.NoError(t, err) diff --git a/private/buf/cmd/buf/command/alpha/protoc/plugin.go b/private/buf/cmd/buf/command/alpha/protoc/plugin.go index 56501842cc..931e910cb2 100644 --- a/private/buf/cmd/buf/command/alpha/protoc/plugin.go +++ b/private/buf/cmd/buf/command/alpha/protoc/plugin.go @@ -23,7 +23,6 @@ import ( "github.com/bufbuild/buf/private/buf/bufprotopluginexec" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "google.golang.org/protobuf/types/pluginpb" ) @@ -45,7 +44,6 @@ func executePlugin( ctx context.Context, logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, container app.EnvStderrContainer, images []bufimage.Image, pluginName string, @@ -54,7 +52,6 @@ func executePlugin( generator := bufprotopluginexec.NewGenerator( logger, storageosProvider, - runner, ) requests, err := bufimage.ImagesToCodeGeneratorRequests( images, diff --git a/private/buf/cmd/buf/command/alpha/protoc/protoc.go b/private/buf/cmd/buf/command/alpha/protoc/protoc.go index 20e78c18f4..ce2180d02e 100644 --- a/private/buf/cmd/buf/command/alpha/protoc/protoc.go +++ b/private/buf/cmd/buf/command/alpha/protoc/protoc.go @@ -34,7 +34,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app" "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/slogext" "github.com/bufbuild/buf/private/pkg/storage/storageos" ) @@ -87,7 +86,6 @@ func run( container appext.Container, env *env, ) (retErr error) { - runner := command.NewRunner() logger := container.Logger() defer slogext.DebugProfile(logger)() @@ -191,7 +189,6 @@ func run( ctx, logger, storageosProvider, - runner, container, images, pluginName, diff --git a/private/buf/cmd/buf/command/alpha/protoc/protoc_test.go b/private/buf/cmd/buf/command/alpha/protoc/protoc_test.go index 6436abecbe..a6db3349fc 100644 --- a/private/buf/cmd/buf/command/alpha/protoc/protoc_test.go +++ b/private/buf/cmd/buf/command/alpha/protoc/protoc_test.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting" "github.com/bufbuild/buf/private/pkg/app/appext" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/prototesting" "github.com/bufbuild/buf/private/pkg/storage" @@ -88,10 +87,8 @@ func TestComparePrintFreeFieldNumbersGoogleapis(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) filePaths := buftesting.GetProtocFilePaths(t, googleapisDirPath, 100) actualProtocStdout := bytes.NewBuffer(nil) - runner := command.NewRunner() buftesting.RunActualProtoc( t, - runner, false, false, googleapisDirPath, @@ -127,17 +124,15 @@ func TestCompareOutputGoogleapis(t *testing.T) { t.Parallel() googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) filePaths := buftesting.GetProtocFilePaths(t, googleapisDirPath, 100) - runner := command.NewRunner() actualProtocFileDescriptorSet := buftesting.GetActualProtocFileDescriptorSet( t, - runner, false, false, googleapisDirPath, filePaths, ) bufProtocFileDescriptorSet := testGetBufProtocFileDescriptorSet(t, googleapisDirPath) - prototesting.AssertFileDescriptorSetsEqual(t, command.NewRunner(), bufProtocFileDescriptorSet, actualProtocFileDescriptorSet) + prototesting.AssertFileDescriptorSetsEqual(t, bufProtocFileDescriptorSet, actualProtocFileDescriptorSet) } func TestCompareGeneratedStubsGoogleapisGo(t *testing.T) { @@ -146,7 +141,6 @@ func TestCompareGeneratedStubsGoogleapisGo(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubs( t, - command.NewRunner(), googleapisDirPath, []testPluginInfo{ {name: "go", opt: "Mgoogle/api/auth.proto=foo"}, @@ -160,7 +154,6 @@ func TestCompareGeneratedStubsGoogleapisGoZip(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubsArchive( t, - command.NewRunner(), googleapisDirPath, []testPluginInfo{ {name: "go", opt: "Mgoogle/api/auth.proto=foo"}, @@ -175,7 +168,6 @@ func TestCompareGeneratedStubsGoogleapisGoJar(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubsArchive( t, - command.NewRunner(), googleapisDirPath, []testPluginInfo{ {name: "go", opt: "Mgoogle/api/auth.proto=foo"}, @@ -190,7 +182,6 @@ func TestCompareGeneratedStubsGoogleapisObjc(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubs( t, - command.NewRunner(), googleapisDirPath, []testPluginInfo{{name: "objc"}}, ) @@ -202,7 +193,6 @@ func TestCompareInsertionPointOutput(t *testing.T) { insertionTestdataDirPath := filepath.Join("testdata", "insertion") testCompareGeneratedStubs( t, - command.NewRunner(), insertionTestdataDirPath, []testPluginInfo{ {name: "insertion-point-receiver"}, @@ -216,15 +206,14 @@ func TestInsertionPointMixedPathsSuccess(t *testing.T) { t.Parallel() wd, err := os.Getwd() require.NoError(t, err) - runner := command.NewRunner() - testInsertionPointMixedPathsSuccess(t, runner, ".", wd) - testInsertionPointMixedPathsSuccess(t, runner, wd, ".") + testInsertionPointMixedPathsSuccess(t, ".", wd) + testInsertionPointMixedPathsSuccess(t, wd, ".") } // testInsertionPointMixedPathsSuccess demonstrates that insertion points are able // to generate to the same output directory, even if the absolute path points to // the same place. -func testInsertionPointMixedPathsSuccess(t *testing.T, runner command.Runner, receiverOut string, writerOut string) { +func testInsertionPointMixedPathsSuccess(t *testing.T, receiverOut string, writerOut string) { dirPath := filepath.Join("testdata", "insertion") filePaths := buftesting.GetProtocFilePaths(t, dirPath, 100) protocFlags := []string{ @@ -233,7 +222,6 @@ func testInsertionPointMixedPathsSuccess(t *testing.T, runner command.Runner, re } err := prototesting.RunProtoc( context.Background(), - runner, []string{dirPath}, filePaths, false, @@ -274,7 +262,6 @@ func testInsertionPointMixedPathsSuccess(t *testing.T, runner command.Runner, re func testCompareGeneratedStubs( t *testing.T, - runner command.Runner, dirPath string, plugins []testPluginInfo, ) { @@ -290,7 +277,6 @@ func testCompareGeneratedStubs( } buftesting.RunActualProtoc( t, - runner, false, false, dirPath, @@ -346,7 +332,6 @@ func testCompareGeneratedStubs( require.NoError(t, err) diff, err := storage.DiffBytes( context.Background(), - runner, actualReadWriteBucket, bufReadWriteBucket, ) @@ -356,7 +341,6 @@ func testCompareGeneratedStubs( func testCompareGeneratedStubsArchive( t *testing.T, - runner command.Runner, dirPath string, plugins []testPluginInfo, useJar bool, @@ -378,7 +362,6 @@ func testCompareGeneratedStubsArchive( } buftesting.RunActualProtoc( t, - runner, false, false, dirPath, @@ -443,7 +426,6 @@ func testCompareGeneratedStubsArchive( require.NoError(t, err) diff, err := storage.DiffBytes( context.Background(), - runner, actualReadWriteBucket, bufReadWriteBucket, ) diff --git a/private/buf/cmd/buf/command/beta/lsp/lsp.go b/private/buf/cmd/buf/command/beta/lsp/lsp.go index adb32c87e2..52908c8f81 100644 --- a/private/buf/cmd/buf/command/beta/lsp/lsp.go +++ b/private/buf/cmd/buf/command/beta/lsp/lsp.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "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/ioext" "github.com/bufbuild/buf/private/pkg/wasm" "github.com/spf13/pflag" @@ -116,7 +115,7 @@ func run( }() checkClient, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 0cfd5c5a01..36b4a5cb92 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "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/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/wasm" @@ -222,7 +221,7 @@ func run( for i, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/config/configmigrate/configmigrate.go b/private/buf/cmd/buf/command/config/configmigrate/configmigrate.go index 8ff7a834ca..13e8f366f2 100644 --- a/private/buf/cmd/buf/command/config/configmigrate/configmigrate.go +++ b/private/buf/cmd/buf/command/config/configmigrate/configmigrate.go @@ -21,7 +21,6 @@ import ( "github.com/bufbuild/buf/private/buf/bufmigrate" "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/storage/storageos" "github.com/spf13/pflag" ) @@ -109,7 +108,6 @@ func run( container appext.Container, flags *flags, ) error { - runner := command.NewRunner() moduleKeyProvider, err := bufcli.NewModuleKeyProvider(container) if err != nil { return err @@ -127,7 +125,6 @@ func run( } migrator := bufmigrate.NewMigrator( container.Logger(), - runner, moduleKeyProvider, commitProvider, ) diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 959907bd70..919607d1a0 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -26,7 +26,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "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/normalpath" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" @@ -198,7 +197,7 @@ func lsRun( }() client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/format/format.go b/private/buf/cmd/buf/command/format/format.go index b953ba7f90..e991a44f76 100644 --- a/private/buf/cmd/buf/command/format/format.go +++ b/private/buf/cmd/buf/command/format/format.go @@ -30,7 +30,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufmodule" "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/slicesext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" @@ -290,7 +289,6 @@ func run( return err } - runner := command.NewRunner() controller, err := bufcli.NewController( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), @@ -326,7 +324,6 @@ func run( diffBuffer := bytes.NewBuffer(nil) changedPaths, err := storage.DiffWithFilenames( ctx, - runner, diffBuffer, originalReadBucket, formattedReadBucket, diff --git a/private/buf/cmd/buf/command/generate/generate.go b/private/buf/cmd/buf/command/generate/generate.go index 19a8f25aa5..d35213a50e 100644 --- a/private/buf/cmd/buf/command/generate/generate.go +++ b/private/buf/cmd/buf/command/generate/generate.go @@ -31,7 +31,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "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/storage/storageos" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/spf13/pflag" @@ -542,7 +541,6 @@ func run( return bufgen.NewGenerator( logger, storageosProvider, - command.NewRunner(), clientConfig, ).Generate( ctx, diff --git a/private/buf/cmd/buf/command/generate/generate_test.go b/private/buf/cmd/buf/command/generate/generate_test.go index 22758a0b10..24e220e274 100644 --- a/private/buf/cmd/buf/command/generate/generate_test.go +++ b/private/buf/cmd/buf/command/generate/generate_test.go @@ -31,7 +31,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting" "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/slicesext" "github.com/bufbuild/buf/private/pkg/storage" @@ -63,7 +62,6 @@ func TestCompareGeneratedStubsGoogleapisGo(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubs( t, - command.NewRunner(), googleapisDirPath, []*testPluginInfo{ {name: "go", opt: "Mgoogle/api/auth.proto=foo"}, @@ -77,7 +75,6 @@ func TestCompareGeneratedStubsGoogleapisGoZip(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubsArchive( t, - command.NewRunner(), googleapisDirPath, []*testPluginInfo{ {name: "go", opt: "Mgoogle/api/auth.proto=foo"}, @@ -92,7 +89,6 @@ func TestCompareGeneratedStubsGoogleapisGoJar(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubsArchive( t, - command.NewRunner(), googleapisDirPath, []*testPluginInfo{ {name: "go", opt: "Mgoogle/api/auth.proto=foo"}, @@ -107,7 +103,6 @@ func TestCompareGeneratedStubsGoogleapisObjc(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubs( t, - command.NewRunner(), googleapisDirPath, []*testPluginInfo{{name: "objc"}}, ) @@ -119,7 +114,6 @@ func TestCompareGeneratedStubsGoogleapisPyi(t *testing.T) { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) testCompareGeneratedStubs( t, - command.NewRunner(), googleapisDirPath, []*testPluginInfo{{name: "pyi"}}, ) @@ -131,7 +125,6 @@ func TestCompareInsertionPointOutput(t *testing.T) { insertionTestdataDirPath := filepath.Join("testdata", "insertion") testCompareGeneratedStubs( t, - command.NewRunner(), insertionTestdataDirPath, []*testPluginInfo{ {name: "insertion-point-receiver"}, @@ -172,7 +165,7 @@ func TestGenerateV2LocalPluginBasic(t *testing.T) { actual, err := storageos.NewProvider().NewReadWriteBucket(tempDirPath) require.NoError(t, err) - diff, err := storage.DiffBytes(context.Background(), command.NewRunner(), expected, actual) + diff, err := storage.DiffBytes(context.Background(), expected, actual) require.NoError(t, err) require.Empty(t, string(diff)) } @@ -195,7 +188,7 @@ func TestGenerateV2LocalPluginTypes(t *testing.T) { actual, err := storageos.NewProvider().NewReadWriteBucket(tempDirPath) require.NoError(t, err) - diff, err := storage.DiffBytes(context.Background(), command.NewRunner(), expected, actual) + diff, err := storage.DiffBytes(context.Background(), expected, actual) require.NoError(t, err) require.Empty(t, string(diff)) } @@ -429,13 +422,12 @@ func TestOutputWithPathEqualToExclude(t *testing.T) { func TestGenerateInsertionPoint(t *testing.T) { t.Parallel() - runner := command.NewRunner() - testGenerateInsertionPointV1(t, runner, ".", ".", filepath.Join("testdata", "insertion_point")) - testGenerateInsertionPointV1(t, runner, "gen/proto/insertion", "gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) - testGenerateInsertionPointV1(t, runner, "gen/proto/insertion/", "./gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) - testGenerateInsertionPointV2(t, runner, ".", ".", filepath.Join("testdata", "insertion_point")) - testGenerateInsertionPointV2(t, runner, "gen/proto/insertion", "gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) - testGenerateInsertionPointV2(t, runner, "gen/proto/insertion/", "./gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) + testGenerateInsertionPointV1(t, ".", ".", filepath.Join("testdata", "insertion_point")) + testGenerateInsertionPointV1(t, "gen/proto/insertion", "gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) + testGenerateInsertionPointV1(t, "gen/proto/insertion/", "./gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) + testGenerateInsertionPointV2(t, ".", ".", filepath.Join("testdata", "insertion_point")) + testGenerateInsertionPointV2(t, "gen/proto/insertion", "gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) + testGenerateInsertionPointV2(t, "gen/proto/insertion/", "./gen/proto/insertion", filepath.Join("testdata", "nested_insertion_point")) } func TestGenerateInsertionPointFail(t *testing.T) { @@ -591,14 +583,12 @@ func TestBoolPointerFlagUnspecified(t *testing.T) { func testGenerateInsertionPointV1( t *testing.T, - runner command.Runner, receiverOut string, writerOut string, expectedOutputPath string, ) { testGenerateInsertionPoint( t, - runner, receiverOut, writerOut, expectedOutputPath, @@ -615,14 +605,12 @@ plugins: func testGenerateInsertionPointV2( t *testing.T, - runner command.Runner, receiverOut string, writerOut string, expectedOutputPath string, ) { testGenerateInsertionPoint( t, - runner, receiverOut, writerOut, expectedOutputPath, @@ -639,7 +627,6 @@ plugins: func testGenerateInsertionPoint( t *testing.T, - runner command.Runner, receiverOut string, writerOut string, expectedOutputPath string, @@ -662,7 +649,7 @@ func testGenerateInsertionPoint( ) expectedOutput, err := storageosProvider.NewReadWriteBucket(expectedOutputPath) require.NoError(t, err) - diff, err := storage.DiffBytes(context.Background(), runner, expectedOutput, readWriteBucket) + diff, err := storage.DiffBytes(context.Background(), expectedOutput, readWriteBucket) require.NoError(t, err) require.Empty(t, string(diff)) } @@ -732,7 +719,6 @@ func testGenerateInsertionPointMixedPathsFail( func testCompareGeneratedStubs( t *testing.T, - runner command.Runner, dirPath string, testPluginInfos []*testPluginInfo, ) { @@ -748,7 +734,6 @@ func testCompareGeneratedStubs( } buftesting.RunActualProtoc( t, - runner, false, false, dirPath, @@ -797,7 +782,6 @@ func testCompareGeneratedStubs( require.NoError(t, err) diff, err := storage.DiffBytes( context.Background(), - runner, actualReadWriteBucket, bufReadWriteBucket, transformGolangProtocVersionToUnknown(t), @@ -808,7 +792,6 @@ func testCompareGeneratedStubs( func testCompareGeneratedStubsArchive( t *testing.T, - runner command.Runner, dirPath string, testPluginInfos []*testPluginInfo, useJar bool, @@ -830,7 +813,6 @@ func testCompareGeneratedStubsArchive( } buftesting.RunActualProtoc( t, - runner, false, false, dirPath, @@ -879,7 +861,6 @@ func testCompareGeneratedStubsArchive( require.NoError(t, err) diff, err := storage.DiffBytes( context.Background(), - runner, actualReadWriteBucket, bufReadWriteBucket, transformGolangProtocVersionToUnknown(t), diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 42066f96c1..f0506a930a 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -25,7 +25,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "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/stringutil" "github.com/bufbuild/buf/private/pkg/wasm" "github.com/spf13/pflag" @@ -147,7 +146,7 @@ func run( for _, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/mod/internal/internal.go b/private/buf/cmd/buf/command/mod/internal/internal.go index accfd9aec8..c802e34e50 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/private/buf/cmd/buf/command/mod/internal/internal.go @@ -26,7 +26,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "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/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/syserror" @@ -176,7 +175,7 @@ func lsRun( // BufYAMLFiles <=v1 never had plugins. client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + bufcheck.NewRunnerProvider(wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 49ef16c5ea..213db7689e 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -29,7 +29,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app" "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/git" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" @@ -456,31 +455,30 @@ func getGitMetadataUploadOptions( if err != nil { return nil, err } - runner := command.NewRunner() // validate that input is a dirRef and is a valid git checkout - if err := validateInputIsValidDirAndGitCheckout(ctx, runner, container, input); err != nil { + if err := validateInputIsValidDirAndGitCheckout(ctx, container, input); err != nil { return nil, err } - uncommittedFiles, err := git.CheckForUncommittedGitChanges(ctx, runner, container, input) + uncommittedFiles, err := git.CheckForUncommittedGitChanges(ctx, container, input) if err != nil { return nil, err } if len(uncommittedFiles) > 0 { return nil, fmt.Errorf("--%s requires that there are no uncommitted changes, uncommitted changes found in the following files: %v", gitMetadataFlagName, uncommittedFiles) } - originRemote, err := git.GetRemote(ctx, runner, container, input, gitOriginRemote) + originRemote, err := git.GetRemote(ctx, container, input, gitOriginRemote) if err != nil { if errors.Is(err, git.ErrRemoteNotFound) { return nil, appcmd.NewInvalidArgumentErrorf("remote %s must be present on Git checkout: %w", gitOriginRemote, err) } return nil, err } - currentGitCommit, err := git.GetCurrentHEADGitCommit(ctx, runner, container, input) + currentGitCommit, err := git.GetCurrentHEADGitCommit(ctx, container, input) if err != nil { return nil, err } var gitMetadataUploadOptions []bufmodule.UploadOption - gitLabelsUploadOption, err := getGitMetadataLabelsUploadOptions(ctx, runner, container, input, currentGitCommit) + gitLabelsUploadOption, err := getGitMetadataLabelsUploadOptions(ctx, container, input, currentGitCommit) if err != nil { return nil, err } @@ -515,14 +513,13 @@ func getGitMetadataUploadOptions( func validateInputIsValidDirAndGitCheckout( ctx context.Context, - runner command.Runner, container appext.Container, input string, ) error { if _, err := buffetch.NewDirRefParser(container.Logger()).GetDirRef(ctx, input); err != nil { return appcmd.NewInvalidArgumentErrorf("input %q is not a valid directory: %w", input, err) } - if err := git.CheckDirectoryIsValidGitCheckout(ctx, runner, container, input); err != nil { + if err := git.CheckDirectoryIsValidGitCheckout(ctx, container, input); err != nil { if errors.Is(err, git.ErrInvalidGitCheckout) { return appcmd.NewInvalidArgumentErrorf("input %q is not a local Git repository checkout", input) } @@ -533,12 +530,11 @@ func validateInputIsValidDirAndGitCheckout( func getGitMetadataLabelsUploadOptions( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, input string, gitCommitSha string, ) (bufmodule.UploadOption, error) { - refs, err := git.GetRefsForGitCommitAndRemote(ctx, runner, envContainer, input, gitOriginRemote, gitCommitSha) + refs, err := git.GetRefsForGitCommitAndRemote(ctx, envContainer, input, gitOriginRemote, gitCommitSha) if err != nil { return nil, err } diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index ec29b5f6a2..1de59b0149 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "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/protodescriptor" "github.com/bufbuild/buf/private/pkg/protoencoding" @@ -126,7 +125,7 @@ func handle( // The protoc plugins do not support custom lint/breaking change plugins for now. client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + bufcheck.NewRunnerProvider(wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index 6ee136a4b6..d7ed7b0a1a 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "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/protodescriptor" "github.com/bufbuild/buf/private/pkg/protoencoding" @@ -101,7 +100,7 @@ func handle( // The protoc plugins do not support custom lint/breaking change plugins for now. client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + bufcheck.NewRunnerProvider(wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { 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 5aa72639f2..9185cbb001 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint_test.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint_test.go @@ -22,7 +22,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/prototesting" @@ -258,7 +257,6 @@ func testRunLint( expectedExitCode int, expectedErrorString string, ) { - runner := command.NewRunner() testRunHandlerFunc( t, protoplugin.HandlerFunc( @@ -278,7 +276,6 @@ func testRunLint( ), testBuildRequest( t, - runner, root, realFilePaths, parameter, @@ -328,7 +325,6 @@ func testRunHandlerFunc( func testBuildRequest( t *testing.T, - runner command.Runner, root string, realFilePaths []string, parameter string, @@ -336,7 +332,6 @@ func testBuildRequest( ) protoplugin.Request { fileDescriptorSet, err := prototesting.GetProtocFileDescriptorSet( context.Background(), - runner, []string{root}, realFilePaths, true, diff --git a/private/bufpkg/bufcheck/breaking_test.go b/private/bufpkg/bufcheck/breaking_test.go index c87af2d1e6..2bab7cbf4d 100644 --- a/private/bufpkg/bufcheck/breaking_test.go +++ b/private/bufpkg/bufcheck/breaking_test.go @@ -30,7 +30,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "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/slogtestext" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/wasm" @@ -1345,7 +1344,7 @@ func testBreaking( require.NoError(t, err) breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID) require.NotNil(t, breakingConfig) - client, err := bufcheck.NewClient(logger, bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(logger, bufcheck.NewRunnerProvider(wasm.UnimplementedRuntime)) require.NoError(t, err) err = client.Breaking( ctx, diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 0cff78425d..91a6cda45f 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -22,7 +22,6 @@ 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/slicesext" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/wasm" @@ -170,7 +169,7 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug return r(pluginConfig) } -// NewRunnerProvider returns a new RunnerProvider for the command.Runner and wasm.Runtime. +// NewRunnerProvider returns a new RunnerProvider for the wasm.Runtime. // // This implementation should only be used for local applications. It is safe to // use concurrently. @@ -181,8 +180,8 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug // - bufconfig.PluginConfigTypeLocalWasm // // If the PluginConfigType is not supported, an error is returned. -func NewRunnerProvider(commandRunner command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { - return newRunnerProvider(commandRunner, wasmRuntime) +func NewRunnerProvider(wasmRuntime wasm.Runtime) RunnerProvider { + return newRunnerProvider(wasmRuntime) } // NewClient returns a new Client. diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index 1ff56ab7f3..626f3738b2 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "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/slogtestext" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/wasm" @@ -1356,7 +1355,7 @@ func testLintWithOptions( }) client, err := bufcheck.NewClient( logger, - bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(wasmRuntime), ) require.NoError(t, err) err = client.Lint( diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index 810887cd9a..8fde74ddae 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -24,7 +24,6 @@ import ( "buf.build/go/bufplugin/check/checkutil" "buf.build/go/bufplugin/option" "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/slogtestext" "github.com/bufbuild/buf/private/pkg/stringutil" @@ -183,7 +182,7 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) { client, err := newClient( slogtestext.NewLogger(t), - NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + NewRunnerProvider(wasm.UnimplementedRuntime), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( @@ -276,7 +275,7 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) { client, err := newClient( slogtestext.NewLogger(t), - NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + NewRunnerProvider(wasm.UnimplementedRuntime), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index e5788c9017..ca8a8ed626 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -16,7 +16,6 @@ package bufcheck import ( "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/syserror" "github.com/bufbuild/buf/private/pkg/wasm" @@ -24,14 +23,12 @@ import ( ) type runnerProvider struct { - commandRunner command.Runner - wasmRuntime wasm.Runtime + wasmRuntime wasm.Runtime } -func newRunnerProvider(commandRunner command.Runner, wasmRuntime wasm.Runtime) *runnerProvider { +func newRunnerProvider(wasmRuntime wasm.Runtime) *runnerProvider { return &runnerProvider{ - commandRunner: commandRunner, - wasmRuntime: wasmRuntime, + wasmRuntime: wasmRuntime, } } @@ -40,7 +37,6 @@ func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginr case bufconfig.PluginConfigTypeLocal: path := pluginConfig.Path() return pluginrpcutil.NewRunner( - r.commandRunner, // We know that Path is of at least length 1. path[0], path[1:]..., diff --git a/private/bufpkg/bufimage/bufimagefuzz/bufimagefuzz_unix_test.go b/private/bufpkg/bufimage/bufimagefuzz/bufimagefuzz_unix_test.go index 71ae4c9550..90d3e3a008 100644 --- a/private/bufpkg/bufimage/bufimagefuzz/bufimagefuzz_unix_test.go +++ b/private/bufpkg/bufimage/bufimagefuzz/bufimagefuzz_unix_test.go @@ -30,7 +30,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduletesting" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/prototesting" "github.com/bufbuild/buf/private/pkg/slogext" "github.com/bufbuild/buf/private/pkg/tmp" @@ -45,7 +44,6 @@ func TestCorpus(t *testing.T) { // To focus on just one test in the corpus, put its file name here. Don't forget to revert before committing. focus := "" ctx := context.Background() - runner := command.NewRunner() require.NoError(t, filepath.Walk("corpus", func(path string, info fs.FileInfo, err error) error { if err != nil { return err @@ -59,7 +57,7 @@ func TestCorpus(t *testing.T) { t.Run(info.Name(), func(t *testing.T) { data, err := os.ReadFile(filepath.Join("corpus", info.Name())) require.NoError(t, err) - result, err := fuzz(ctx, runner, data) + result, err := fuzz(ctx, data) require.NoError(t, err) require.NoError(t, result.error(ctx)) }) @@ -94,8 +92,7 @@ func TestCorpus(t *testing.T) { // go-fuzz -bin $(TMP)/gofuzz/gofuzz.zip -workdir $(TMP)/gofuzz //func Fuzz(data []byte) int { //ctx := context.Background() -//runner := command.NewRunner() -//result, err := fuzz(ctx, runner, data) +//result, err := fuzz(ctx, data) //if err != nil { //// data was invalid in some way //return -1 @@ -103,7 +100,7 @@ func TestCorpus(t *testing.T) { //return result.panicOrN(ctx) //} -func fuzz(ctx context.Context, runner command.Runner, data []byte) (_ *fuzzResult, retErr error) { +func fuzz(ctx context.Context, data []byte) (_ *fuzzResult, retErr error) { dir, err := tmp.NewDir(ctx) if err != nil { return nil, err @@ -122,7 +119,6 @@ func fuzz(ctx context.Context, runner command.Runner, data []byte) (_ *fuzzResul actualProtocFileDescriptorSet, protocErr := prototesting.GetProtocFileDescriptorSet( ctx, - runner, []string{dir.Path()}, filePaths, false, @@ -131,7 +127,6 @@ func fuzz(ctx context.Context, runner command.Runner, data []byte) (_ *fuzzResul image, bufErr := fuzzBuild(ctx, dir.Path()) return newFuzzResult( - runner, bufErr, protocErr, actualProtocFileDescriptorSet, @@ -194,7 +189,6 @@ func untxtar(data []byte, destDirPath string) error { } type fuzzResult struct { - runner command.Runner bufErr error protocErr error actualProtocFileDescriptorSet *descriptorpb.FileDescriptorSet @@ -202,14 +196,12 @@ type fuzzResult struct { } func newFuzzResult( - runner command.Runner, bufErr error, protocErr error, actualProtocFileDescriptorSet *descriptorpb.FileDescriptorSet, image bufimage.Image, ) *fuzzResult { return &fuzzResult{ - runner: runner, bufErr: bufErr, protocErr: protocErr, actualProtocFileDescriptorSet: actualProtocFileDescriptorSet, @@ -246,7 +238,6 @@ func (f *fuzzResult) error(ctx context.Context) error { diff, err := prototesting.DiffFileDescriptorSetsJSON( ctx, - f.runner, fileDescriptorSet, f.actualProtocFileDescriptorSet, "buf", diff --git a/private/bufpkg/bufimage/build_image_test.go b/private/bufpkg/bufimage/build_image_test.go index 7f02a3a046..0d05fb1148 100644 --- a/private/bufpkg/bufimage/build_image_test.go +++ b/private/bufpkg/bufimage/build_image_test.go @@ -29,7 +29,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduletesting" "github.com/bufbuild/buf/private/bufpkg/bufprotosource" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/prototesting" "github.com/bufbuild/buf/private/pkg/slogtestext" @@ -219,20 +218,17 @@ func TestGoogleapis(t *testing.T) { func TestCompareCustomOptions1(t *testing.T) { t.Parallel() - runner := command.NewRunner() - testCompare(t, runner, "customoptions1") + testCompare(t, "customoptions1") } func TestCompareProto3Optional1(t *testing.T) { t.Parallel() - runner := command.NewRunner() - testCompare(t, runner, "proto3optional1") + testCompare(t, "proto3optional1") } func TestCompareTrailingComments(t *testing.T) { t.Parallel() - runner := command.NewRunner() - testCompare(t, runner, "trailingcomments") + testCompare(t, "trailingcomments") } func TestCustomOptionsError1(t *testing.T) { @@ -314,8 +310,7 @@ func TestOptionPanic(t *testing.T) { func TestCompareSemicolons(t *testing.T) { t.Parallel() - runner := command.NewRunner() - testCompare(t, runner, "semicolons") + testCompare(t, "semicolons") } func TestModuleTargetFiles(t *testing.T) { @@ -369,15 +364,15 @@ func TestModuleTargetFiles(t *testing.T) { testTagetImageFiles(t, []string{"b.proto", "c.proto"}, "buf.build/foo/b", "buf.build/foo/c") } -func testCompare(t *testing.T, runner command.Runner, relDirPath string) { +func testCompare(t *testing.T, relDirPath string) { dirPath := filepath.Join("testdata", relDirPath) image, fileAnnotations := testBuild(t, false, dirPath, false) require.Equal(t, 0, len(fileAnnotations), fileAnnotations) image = bufimage.ImageWithoutImports(image) fileDescriptorSet := bufimage.ImageToFileDescriptorSet(image) filePaths := buftesting.GetProtocFilePaths(t, dirPath, 0) - actualProtocFileDescriptorSet := buftesting.GetActualProtocFileDescriptorSet(t, runner, false, false, dirPath, filePaths) - prototesting.AssertFileDescriptorSetsEqual(t, runner, fileDescriptorSet, actualProtocFileDescriptorSet) + actualProtocFileDescriptorSet := buftesting.GetActualProtocFileDescriptorSet(t, false, false, dirPath, filePaths) + prototesting.AssertFileDescriptorSetsEqual(t, fileDescriptorSet, actualProtocFileDescriptorSet) } func testBuildGoogleapis(t *testing.T, includeSourceInfo bool) bufimage.Image { diff --git a/private/bufpkg/bufimage/build_image_unix_test.go b/private/bufpkg/bufimage/build_image_unix_test.go index cb6d0636d2..b339acd2c4 100644 --- a/private/bufpkg/bufimage/build_image_unix_test.go +++ b/private/bufpkg/bufimage/build_image_unix_test.go @@ -22,7 +22,6 @@ import ( "github.com/bufbuild/buf/private/buf/buftesting" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/prototesting" "github.com/bufbuild/buf/private/pkg/testingext" "github.com/stretchr/testify/assert" @@ -36,20 +35,18 @@ func TestCompareGoogleapis(t *testing.T) { // code infos that protoc does not image := testBuildGoogleapis(t, false) fileDescriptorSet := bufimage.ImageToFileDescriptorSet(image) - runner := command.NewRunner() - actualProtocFileDescriptorSet := testBuildActualProtocGoogleapis(t, runner, false) + actualProtocFileDescriptorSet := testBuildActualProtocGoogleapis(t, false) prototesting.AssertFileDescriptorSetsEqual( t, - runner, fileDescriptorSet, actualProtocFileDescriptorSet, ) } -func testBuildActualProtocGoogleapis(t *testing.T, runner command.Runner, includeSourceInfo bool) *descriptorpb.FileDescriptorSet { +func testBuildActualProtocGoogleapis(t *testing.T, includeSourceInfo bool) *descriptorpb.FileDescriptorSet { googleapisDirPath := buftesting.GetGoogleapisDirPath(t, buftestingDirPath) filePaths := buftesting.GetProtocFilePaths(t, googleapisDirPath, 0) - fileDescriptorSet := buftesting.GetActualProtocFileDescriptorSet(t, runner, true, includeSourceInfo, googleapisDirPath, filePaths) + fileDescriptorSet := buftesting.GetActualProtocFileDescriptorSet(t, true, includeSourceInfo, googleapisDirPath, filePaths) assert.Equal(t, buftesting.NumGoogleapisFilesWithImports, len(fileDescriptorSet.GetFile())) return fileDescriptorSet diff --git a/private/bufpkg/bufremoteplugin/bufremoteplugindocker/docker_test.go b/private/bufpkg/bufremoteplugin/bufremoteplugindocker/docker_test.go index a60c9f5c5d..d27cc40265 100644 --- a/private/bufpkg/bufremoteplugin/bufremoteplugindocker/docker_test.go +++ b/private/bufpkg/bufremoteplugin/bufremoteplugindocker/docker_test.go @@ -33,7 +33,7 @@ import ( "time" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/slogtestext" "github.com/docker/docker/api/types" dockerimage "github.com/docker/docker/api/types/image" @@ -148,29 +148,28 @@ func buildDockerPlugin(t testing.TB, dockerfilePath string, pluginIdentity strin return "", err } imageName := fmt.Sprintf("%s:%s", pluginIdentity, stringid.GenerateRandomID()) - cmd := command.NewRunner() envContainer, err := app.NewEnvContainerForOS() require.NoError(t, err) - if err := cmd.Run( + if err := execext.Run( context.Background(), docker, - command.RunWithArgs("build", "-t", imageName, "."), - command.RunWithDir(filepath.Dir(dockerfilePath)), - command.RunWithStdout(os.Stdout), - command.RunWithStderr(os.Stderr), - command.RunWithEnv(app.EnvironMap(envContainer)), + execext.WithArgs("build", "-t", imageName, "."), + execext.WithDir(filepath.Dir(dockerfilePath)), + execext.WithStdout(os.Stdout), + execext.WithStderr(os.Stderr), + execext.WithEnv(app.Environ(envContainer)), ); err != nil { return "", err } t.Logf("created image: %s", imageName) t.Cleanup(func() { - if err := cmd.Run( + if err := execext.Run( context.Background(), docker, - command.RunWithArgs("rmi", "--force", imageName), - command.RunWithDir(filepath.Dir(dockerfilePath)), - command.RunWithStdout(os.Stdout), - command.RunWithStderr(os.Stderr), + execext.WithArgs("rmi", "--force", imageName), + execext.WithDir(filepath.Dir(dockerfilePath)), + execext.WithStdout(os.Stdout), + execext.WithStderr(os.Stderr), ); err != nil { t.Logf("failed to remove temporary docker image: %v", err) } diff --git a/private/pkg/bandeps/bandeps.go b/private/pkg/bandeps/bandeps.go index ba7cc195b2..ebd0a1c793 100644 --- a/private/pkg/bandeps/bandeps.go +++ b/private/pkg/bandeps/bandeps.go @@ -20,7 +20,6 @@ import ( "log/slog" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" ) // Checker is a checker. @@ -33,8 +32,8 @@ type Checker interface { ) ([]Violation, error) } -func NewChecker(logger *slog.Logger, runner command.Runner) Checker { - return newChecker(logger, runner) +func NewChecker(logger *slog.Logger) Checker { + return newChecker(logger) } // Violation is a violation. diff --git a/private/pkg/bandeps/checker.go b/private/pkg/bandeps/checker.go index 7f8160f2e1..ccc59af620 100644 --- a/private/pkg/bandeps/checker.go +++ b/private/pkg/bandeps/checker.go @@ -20,19 +20,16 @@ import ( "sync" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/thread" ) type checker struct { logger *slog.Logger - runner command.Runner } -func newChecker(logger *slog.Logger, runner command.Runner) *checker { +func newChecker(logger *slog.Logger) *checker { return &checker{ logger: logger, - runner: runner, } } @@ -41,7 +38,7 @@ func (c *checker) Check( envStdioContainer app.EnvStdioContainer, externalConfig ExternalConfig, ) ([]Violation, error) { - state := newState(c.logger, envStdioContainer, c.runner) + state := newState(c.logger, envStdioContainer) if err := c.populateState(ctx, state, externalConfig); err != nil { return nil, err } diff --git a/private/pkg/bandeps/cmd/bandeps/main.go b/private/pkg/bandeps/cmd/bandeps/main.go index be156ec6d6..ed8196db03 100644 --- a/private/pkg/bandeps/cmd/bandeps/main.go +++ b/private/pkg/bandeps/cmd/bandeps/main.go @@ -24,7 +24,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/bandeps" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/slogapp" "github.com/spf13/pflag" @@ -92,7 +91,6 @@ func run(ctx context.Context, container appext.Container, flags *flags) error { } violations, err := bandeps.NewChecker( container.Logger(), - command.NewRunner(), ).Check( ctx, container, diff --git a/private/pkg/bandeps/state.go b/private/pkg/bandeps/state.go index ffe877ecc9..7d3be9e885 100644 --- a/private/pkg/bandeps/state.go +++ b/private/pkg/bandeps/state.go @@ -15,19 +15,19 @@ package bandeps import ( + "bytes" "context" "log/slog" "sync" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/slicesext" ) type state struct { logger *slog.Logger envStdioContainer app.EnvStdioContainer - runner command.Runner violationMap map[string]Violation // map from ./foo/bar/... to actual packages packageExpressionToPackages map[string]*packagesResult @@ -43,12 +43,10 @@ type state struct { func newState( logger *slog.Logger, envStdioContainer app.EnvStdioContainer, - runner command.Runner, ) *state { return &state{ logger: logger, envStdioContainer: envStdioContainer, - runner: runner, violationMap: make(map[string]Violation), packageExpressionToPackages: make(map[string]*packagesResult), packageExpressionToPackagesLock: newKeyRWLock(), @@ -155,7 +153,7 @@ func (s *state) packagesForPackageExpressionUncached( ctx context.Context, packageExpression string, ) (map[string]struct{}, error) { - data, err := command.RunStdout(ctx, s.envStdioContainer, s.runner, `go`, `list`, packageExpression) + data, err := s.runStdout(ctx, `go`, `list`, packageExpression) if err != nil { return nil, err } @@ -210,13 +208,29 @@ func (s *state) depsForPackageUncached( ctx context.Context, pkg string, ) (map[string]struct{}, error) { - data, err := command.RunStdout(ctx, s.envStdioContainer, s.runner, `go`, `list`, `-f`, `{{join .Deps "\n"}}`, pkg) + data, err := s.runStdout(ctx, `go`, `list`, `-f`, `{{join .Deps "\n"}}`, pkg) if err != nil { return nil, err } return slicesext.ToStructMap(getNonEmptyLines(string(data))), nil } +func (s *state) runStdout(ctx context.Context, name string, args ...string) ([]byte, error) { + buffer := bytes.NewBuffer(nil) + if err := execext.Run( + ctx, + name, + execext.WithArgs(args...), + execext.WithEnv(app.Environ(s.envStdioContainer)), + execext.WithStdin(s.envStdioContainer.Stdin()), + execext.WithStdout(buffer), + execext.WithStderr(s.envStdioContainer.Stderr()), + ); err != nil { + return nil, err + } + return buffer.Bytes(), nil +} + type packagesResult struct { Packages map[string]struct{} Err error diff --git a/private/pkg/command/command.go b/private/pkg/command/command.go deleted file mode 100644 index 600646c287..0000000000 --- a/private/pkg/command/command.go +++ /dev/null @@ -1,243 +0,0 @@ -// 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 command - -import ( - "bytes" - "context" - "io" - - "github.com/bufbuild/buf/private/pkg/app" -) - -// Process represents a background process. -type Process interface { - // Wait blocks to wait for the process to exit. It will attempt to kill the - // process if the passed context expires. - Wait(ctx context.Context) error -} - -// Runner runs external commands. -// -// A Runner will limit the number of concurrent commands, as well as explicitly -// set stdin, stdout, stderr, and env to nil/empty values if not set with options. -// -// All external commands in buf MUST use command.Runner instead of -// exec.Command, exec.CommandContext. -type Runner interface { - // Run runs the external command. It blocks until the command exits. - // - // This should be used instead of exec.CommandContext(...).Run(). - Run(ctx context.Context, name string, options ...RunOption) error - - // Start runs the external command, returning a [Process] handle to track - // its progress. - // - // This should be used instead of exec.Command(...).Start(). - Start(name string, options ...StartOption) (Process, error) - - isRunner() -} - -// RunOption is an option for Run. -type RunOption func(*execOptions) - -// RunWithArgs returns a new RunOption that sets the arguments other -// than the name. -// -// The default is no arguments. -func RunWithArgs(args ...string) RunOption { - return func(execOptions *execOptions) { - execOptions.args = args - } -} - -// RunWithEnv returns a new RunOption that sets the environment variables. -// -// The default is to use the single environment variable __EMPTY_ENV__=1 as we -// cannot explicitly set an empty environment with the exec package. -// -// If this and RunWithEnviron are specified, the last option specified wins. -func RunWithEnv(env map[string]string) RunOption { - return func(execOptions *execOptions) { - execOptions.environ = envSlice(env) - } -} - -// RunWithEnviron returns a new RunOption that sets the environment variables. -// -// The default is to use the single environment variable __EMPTY_ENV__=1 as we -// cannot explicitly set an empty environment with the exec package. -// -// If this and RunWithEnv are specified, the last option specified wins. -func RunWithEnviron(environ []string) RunOption { - return func(execOptions *execOptions) { - execOptions.environ = environ - } -} - -// RunWithStdin returns a new RunOption that sets the stdin. -// -// The default is ioext.DiscardReader. -func RunWithStdin(stdin io.Reader) RunOption { - return func(execOptions *execOptions) { - execOptions.stdin = stdin - } -} - -// RunWithStdout returns a new RunOption that sets the stdout. -// -// The default is the null device (os.DevNull). -func RunWithStdout(stdout io.Writer) RunOption { - return func(execOptions *execOptions) { - execOptions.stdout = stdout - } -} - -// RunWithStderr returns a new RunOption that sets the stderr. -// -// The default is the null device (os.DevNull). -func RunWithStderr(stderr io.Writer) RunOption { - return func(execOptions *execOptions) { - execOptions.stderr = stderr - } -} - -// RunWithDir returns a new RunOption that sets the working directory. -// -// The default is the current working directory. -func RunWithDir(dir string) RunOption { - return func(execOptions *execOptions) { - execOptions.dir = dir - } -} - -// StartOption is an option for Start. -type StartOption func(*execOptions) - -// StartWithArgs returns a new RunOption that sets the arguments other -// than the name. -// -// The default is no arguments. -func StartWithArgs(args ...string) StartOption { - return func(execOptions *execOptions) { - execOptions.args = args - } -} - -// StartWithEnv returns a new RunOption that sets the environment variables. -// -// The default is to use the single environment variable __EMPTY_ENV__=1 as we -// cannot explicitly set an empty environment with the exec package. -// -// If this and StartWithEnviron are specified, the last option specified wins. -func StartWithEnv(env map[string]string) StartOption { - return func(execOptions *execOptions) { - execOptions.environ = envSlice(env) - } -} - -// StartWithEnviron returns a new RunOption that sets the environment variables. -// -// The default is to use the single environment variable __EMPTY_ENV__=1 as we -// cannot explicitly set an empty environment with the exec package. -// -// If this and StartWithEnv are specified, the last option specified wins. -func StartWithEnviron(environ []string) StartOption { - return func(execOptions *execOptions) { - execOptions.environ = environ - } -} - -// StartWithStdin returns a new RunOption that sets the stdin. -// -// The default is ioext.DiscardReader. -func StartWithStdin(stdin io.Reader) StartOption { - return func(execOptions *execOptions) { - execOptions.stdin = stdin - } -} - -// StartWithStdout returns a new RunOption that sets the stdout. -// -// The default is the null device (os.DevNull). -func StartWithStdout(stdout io.Writer) StartOption { - return func(execOptions *execOptions) { - execOptions.stdout = stdout - } -} - -// StartWithStderr returns a new RunOption that sets the stderr. -// -// The default is the null device (os.DevNull). -func StartWithStderr(stderr io.Writer) StartOption { - return func(execOptions *execOptions) { - execOptions.stderr = stderr - } -} - -// StartWithDir returns a new RunOption that sets the working directory. -// -// The default is the current working directory. -func StartWithDir(dir string) StartOption { - return func(execOptions *execOptions) { - execOptions.dir = dir - } -} - -// NewRunner returns a new Runner. -func NewRunner(options ...RunnerOption) Runner { - return newRunner(options...) -} - -// RunnerOption is an option for a new Runner. -type RunnerOption func(*runner) - -// RunnerWithParallelism returns a new Runner that sets the number of -// external commands that can be run concurrently. -// -// The default is thread.Parallelism(). -func RunnerWithParallelism(parallelism int) RunnerOption { - if parallelism < 1 { - parallelism = 1 - } - return func(runner *runner) { - runner.parallelism = parallelism - } -} - -// RunStdout is a convenience function that attaches the container environment, -// stdin, and stderr, and returns the stdout as a byte slice. -func RunStdout( - ctx context.Context, - container app.EnvStdioContainer, - runner Runner, - name string, - args ...string, -) ([]byte, error) { - buffer := bytes.NewBuffer(nil) - if err := runner.Run( - ctx, - name, - RunWithArgs(args...), - RunWithEnv(app.EnvironMap(container)), - RunWithStdin(container.Stdin()), - RunWithStdout(buffer), - RunWithStderr(container.Stderr()), - ); err != nil { - return nil, err - } - return buffer.Bytes(), nil -} diff --git a/private/pkg/command/runner.go b/private/pkg/command/runner.go deleted file mode 100644 index 52c97730d4..0000000000 --- a/private/pkg/command/runner.go +++ /dev/null @@ -1,133 +0,0 @@ -// 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 command - -import ( - "context" - "io" - "os/exec" - "sort" - - "github.com/bufbuild/buf/private/pkg/ioext" - "github.com/bufbuild/buf/private/pkg/thread" -) - -var emptyEnv = envSlice( - map[string]string{ - "__EMPTY_ENV": "1", - }, -) - -type runner struct { - parallelism int - - semaphoreC chan struct{} -} - -func newRunner(options ...RunnerOption) *runner { - runner := &runner{ - parallelism: thread.Parallelism(), - } - for _, option := range options { - option(runner) - } - runner.semaphoreC = make(chan struct{}, runner.parallelism) - return runner -} - -func (r *runner) Run(ctx context.Context, name string, options ...RunOption) error { - execOptions := newExecOptions() - for _, option := range options { - option(execOptions) - } - cmd := exec.CommandContext(ctx, name, execOptions.args...) - execOptions.ApplyToCmd(cmd) - r.increment() - err := cmd.Run() - r.decrement() - return err -} - -func (r *runner) Start(name string, options ...StartOption) (Process, error) { - execOptions := newExecOptions() - for _, option := range options { - option(execOptions) - } - cmd := exec.Command(name, execOptions.args...) - execOptions.ApplyToCmd(cmd) - r.increment() - if err := cmd.Start(); err != nil { - return nil, err - } - process := newProcess(cmd, r.decrement) - process.Monitor() - return process, nil -} - -func (r *runner) increment() { - r.semaphoreC <- struct{}{} -} - -func (r *runner) decrement() { - <-r.semaphoreC -} - -func (*runner) isRunner() {} - -type execOptions struct { - args []string - environ []string - stdin io.Reader - stdout io.Writer - stderr io.Writer - dir string -} - -func newExecOptions() *execOptions { - return &execOptions{} -} - -func (e *execOptions) ApplyToCmd(cmd *exec.Cmd) { - // If the user did not specify env vars, we want to make sure - // the command has access to none, as the default is the current env. - if len(e.environ) == 0 { - cmd.Env = emptyEnv - } else { - cmd.Env = e.environ - } - // If the user did not specify any stdin, we want to make sure - // the command has access to none, as the default is the default stdin. - if e.stdin == nil { - cmd.Stdin = ioext.DiscardReader - } else { - cmd.Stdin = e.stdin - } - // If Stdout or Stderr are nil, os/exec connects the process output directly - // to the null device. - cmd.Stdout = e.stdout - cmd.Stderr = e.stderr - // The default behavior for dir is what we want already, i.e. the current - // working directory. - cmd.Dir = e.dir -} - -func envSlice(env map[string]string) []string { - var environ []string - for key, value := range env { - environ = append(environ, key+"="+value) - } - sort.Strings(environ) - return environ -} diff --git a/private/pkg/diff/diff.go b/private/pkg/diff/diff.go index 2c756d2132..4a0a8ea2de 100644 --- a/private/pkg/diff/diff.go +++ b/private/pkg/diff/diff.go @@ -33,7 +33,7 @@ import ( "path/filepath" "runtime" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" ) // Diff does a diff. @@ -41,7 +41,6 @@ import ( // Returns nil if no diff. func Diff( ctx context.Context, - runner command.Runner, b1 []byte, b2 []byte, filename1 string, @@ -54,7 +53,6 @@ func Diff( } return doDiff( ctx, - runner, b1, b2, filename1, @@ -83,7 +81,6 @@ func DiffWithSuppressTimestamps() DiffOption { func doDiff( ctx context.Context, - runner command.Runner, b1 []byte, b2 []byte, filename1 string, @@ -117,12 +114,12 @@ func doDiff( } buffer := bytes.NewBuffer(nil) - err = runner.Run( + err = execext.Run( ctx, binaryPath, - command.RunWithArgs("-u", f1, f2), - command.RunWithStdout(buffer), - command.RunWithStderr(buffer), + execext.WithArgs("-u", f1, f2), + execext.WithStdout(buffer), + execext.WithStderr(buffer), ) data := buffer.Bytes() if len(data) > 0 { diff --git a/private/pkg/execext/execext.go b/private/pkg/execext/execext.go new file mode 100644 index 0000000000..2c4c554a4f --- /dev/null +++ b/private/pkg/execext/execext.go @@ -0,0 +1,259 @@ +// 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 execext + +import ( + "context" + "io" + "os/exec" + "slices" +) + +var emptyEnv = []string{"__EMPTY_ENV__=1"} + +// Run runs the external command. It blocks until the command exits. +// +// Stdin, stdout, stderr, and env will be explicitly set to nil/empty values if not set with options. +// The command will be killed if the Context is cancelled. +// +// This should be used instead of exec.CommandContext(...).Run(). +func Run(ctx context.Context, name string, options ...RunOption) error { + runStartOptions := newRunStartOptions() + for _, option := range options { + option.applyRun(runStartOptions) + } + cmd := exec.CommandContext(ctx, name, runStartOptions.args...) + runStartOptions.applyCmd(cmd) + return cmd.Run() +} + +// Start runs the external command, returning a [Process] to track its progress. +// +// Stdin, stdout, stderr, and env will be explicitly set to nil/empty values if not set with options. +// The command will be killed if the Context is cancelled. +// +// This should be used instead of exec.Command(...).Start(). +func Start(ctx context.Context, name string, options ...StartOption) (Process, error) { + runStartOptions := newRunStartOptions() + for _, option := range options { + option.applyStart(runStartOptions) + } + cmd := exec.CommandContext(ctx, name, runStartOptions.args...) + runStartOptions.applyCmd(cmd) + if err := cmd.Start(); err != nil { + return nil, err + } + process := newProcess(ctx, cmd) + process.monitor() + return process, nil +} + +// Process represents a background process. +type Process interface { + // Wait blocks to wait for the process to exit. + Wait() error + + isProcess() +} + +// RunOption is an option for [Run]. +type RunOption interface { + applyRun(*runStartOptions) +} + +// StartOption is an option for [Start]. +type StartOption interface { + applyStart(*runStartOptions) +} + +// RunStartOption is both a [RunOption] and a [StartOption]. +// +// We split out RunOptions and StartOptions for maximum future flexibility, in case we ever want +// the options for [Run] and [Start] to deviate. +type RunStartOption interface { + RunOption + StartOption +} + +// WithArgs returns a new option that sets the arguments other than the name. +// +// The default is no additional arguments. +func WithArgs(args ...string) RunStartOption { + return &argsOption{args: slices.Clone(args)} +} + +// WithEnv returns a new option that sets the environment variables. +// +// The default is to use the single envment variable __EMPTY_ENV__=1 as we +// cannot explicitly set an empty envment with the exec package. +// +// If this and WithEnv are specified, the last option specified wins. +func WithEnv(env []string) RunStartOption { + return &envOption{env: slices.Clone(env)} +} + +// WithStdin returns a new option that sets the stdin. +// +// The default is a [io.Reader] that always returns empty.. +func WithStdin(stdin io.Reader) RunStartOption { + return &stdinOption{stdin: stdin} +} + +// WithStdout returns a new option that sets the stdout. +// +// The default is a [io.Writer] that ignores all writes.. +func WithStdout(stdout io.Writer) RunStartOption { + return &stdoutOption{stdout: stdout} +} + +// WithStderr returns a new option that sets the stderr. +// +// The default is a [io.Writer] that ignores all writes.. +func WithStderr(stderr io.Writer) RunStartOption { + return &stderrOption{stderr: stderr} +} + +// WithDir returns a new option that sets the working directory. +// +// The default is the current working directory. +func WithDir(dir string) RunStartOption { + return &dirOption{dir: dir} +} + +// *** PRIVATE *** + +type argsOption struct { + args []string +} + +func (a *argsOption) applyRun(runStartOptions *runStartOptions) { + runStartOptions.args = a.args +} + +func (a *argsOption) applyStart(runStartOptions *runStartOptions) { + runStartOptions.args = a.args +} + +type envOption struct { + env []string +} + +func (e *envOption) applyRun(runStartOptions *runStartOptions) { + runStartOptions.env = e.env +} + +func (e *envOption) applyStart(runStartOptions *runStartOptions) { + runStartOptions.env = e.env +} + +type stdinOption struct { + stdin io.Reader +} + +func (i *stdinOption) applyRun(runStartOptions *runStartOptions) { + runStartOptions.stdin = i.stdin +} + +func (i *stdinOption) applyStart(runStartOptions *runStartOptions) { + runStartOptions.stdin = i.stdin +} + +type stdoutOption struct { + stdout io.Writer +} + +func (o *stdoutOption) applyRun(runStartOptions *runStartOptions) { + runStartOptions.stdout = o.stdout +} + +func (o *stdoutOption) applyStart(runStartOptions *runStartOptions) { + runStartOptions.stdout = o.stdout +} + +type stderrOption struct { + stderr io.Writer +} + +func (r *stderrOption) applyRun(runStartOptions *runStartOptions) { + runStartOptions.stderr = r.stderr +} + +func (r *stderrOption) applyStart(runStartOptions *runStartOptions) { + runStartOptions.stderr = r.stderr +} + +type dirOption struct { + dir string +} + +func (d *dirOption) applyRun(runStartOptions *runStartOptions) { + runStartOptions.dir = d.dir +} + +func (d *dirOption) applyStart(runStartOptions *runStartOptions) { + runStartOptions.dir = d.dir +} + +type runStartOptions struct { + args []string + env []string + stdin io.Reader + stdout io.Writer + stderr io.Writer + dir string +} + +func newRunStartOptions() *runStartOptions { + return &runStartOptions{} +} + +func (rs *runStartOptions) applyCmd(cmd *exec.Cmd) { + // If the user did not specify env vars, we want to make sure + // the command has access to none, as the default is the current env. + if len(rs.env) == 0 { + cmd.Env = emptyEnv + } else { + cmd.Env = rs.env + } + // If the user did not specify any stdin, we want to make sure + // the command has access to none, as the default is the default stdin. + // + // Note: This *should* be the same as just having cmd.Stdin = nil, given that + // exec.Cmd documents that Stdin has the same behavior as Stdout/Stderr, namely + // that os.DevNull is used. This has been the case for Stdin since at least 2014. + // However, way back when this package was first introduced, we set up the discardReader + // for some reason, and we can't remember why. We're terrified to change it, as there + // *may* have been some reason to do so. os.DevNull is actually just a string such + // as "/dev/null" on Unix systems, so how Golang actually handles this is somewhat + // unknown. Honestly, I might just want to change Stdout/Stderr to use a discardWriter. + if rs.stdin == nil { + cmd.Stdin = discardReader{} + } else { + cmd.Stdin = rs.stdin + } + // If Stdout or Stderr are nil, os/exec connects the process output directly + // to the null device. + cmd.Stdout = rs.stdout + cmd.Stderr = rs.stderr + // The default behavior for dir is what we want already, i.e. the current + // working directory. + cmd.Dir = rs.dir +} + +type discardReader struct{} + +func (discardReader) Read([]byte) (int, error) { + return 0, io.EOF +} diff --git a/private/pkg/command/runner_unix_test.go b/private/pkg/execext/execext_unix_test.go similarity index 62% rename from private/pkg/command/runner_unix_test.go rename to private/pkg/execext/execext_unix_test.go index fb46e20df3..2575c5edb6 100644 --- a/private/pkg/command/runner_unix_test.go +++ b/private/pkg/execext/execext_unix_test.go @@ -12,10 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build aix || darwin || dragonfly || freebsd || (js && wasm) || linux || netbsd || openbsd || solaris -// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris +//go:build unix -package command +package execext import ( "context" @@ -24,29 +23,27 @@ import ( "github.com/stretchr/testify/require" ) -func TestDoubleWait(t *testing.T) { +func TestStartSimple(t *testing.T) { t.Parallel() - runner := NewRunner() - process, err := runner.Start("echo") - require.NoError(t, err) ctx := context.Background() - _ = process.Wait(ctx) - require.Equal(t, process.Wait(ctx), errWaitAlreadyCalled) -} - -func TestNoDeadlock(t *testing.T) { - t.Parallel() - - runner := NewRunner(RunnerWithParallelism(2)) processes := make([]Process, 4) for i := 0; i < 4; i++ { - process, err := runner.Start("sleep", StartWithArgs("1")) + process, err := Start(ctx, "sleep", WithArgs("1")) require.NoError(t, err) processes[i] = process } - ctx := context.Background() for _, process := range processes { - require.NoError(t, process.Wait(ctx)) + require.NoError(t, process.Wait()) } } + +func TestStartDoubleWait(t *testing.T) { + t.Parallel() + + ctx := context.Background() + process, err := Start(ctx, "echo") + require.NoError(t, err) + _ = process.Wait() + require.Equal(t, process.Wait(), errWaitAlreadyCalled) +} diff --git a/private/pkg/command/process.go b/private/pkg/execext/process.go similarity index 66% rename from private/pkg/command/process.go rename to private/pkg/execext/process.go index 02ccd8016d..4b72533e99 100644 --- a/private/pkg/command/process.go +++ b/private/pkg/execext/process.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package command +package execext import ( "context" @@ -25,36 +25,20 @@ import ( var errWaitAlreadyCalled = errors.New("wait already called on process") type process struct { + ctx context.Context cmd *exec.Cmd - done func() waitC chan error } -// newProcess wraps an *exec.Cmd and monitors it for exiting. -// When the process exits, done will be called. -// -// This implements the Process interface. -// -// The process is expected to have been started by the caller. -func newProcess(cmd *exec.Cmd, done func()) *process { +func newProcess(ctx context.Context, cmd *exec.Cmd) *process { return &process{ + ctx: ctx, cmd: cmd, - done: done, waitC: make(chan error, 1), } } -// Monitor starts monitoring of the *exec.Cmd. -func (p *process) Monitor() { - go func() { - p.waitC <- p.cmd.Wait() - close(p.waitC) - p.done() - }() -} - -// Wait waits for the process to exit. -func (p *process) Wait(ctx context.Context) error { +func (p *process) Wait() error { select { case err, ok := <-p.waitC: // Process exited @@ -62,8 +46,17 @@ func (p *process) Wait(ctx context.Context) error { return err } return errWaitAlreadyCalled - case <-ctx.Done(): + case <-p.ctx.Done(): // Timed out. Send a kill signal and release our handle to it. - return multierr.Combine(ctx.Err(), p.cmd.Process.Kill()) + return multierr.Combine(p.ctx.Err(), p.cmd.Process.Kill()) } } + +func (p *process) monitor() { + go func() { + p.waitC <- p.cmd.Wait() + close(p.waitC) + }() +} + +func (*process) isProcess() {} diff --git a/private/pkg/command/usage.gen.go b/private/pkg/execext/usage.gen.go similarity index 97% rename from private/pkg/command/usage.gen.go rename to private/pkg/execext/usage.gen.go index b84237f458..4c3ce0da85 100644 --- a/private/pkg/command/usage.gen.go +++ b/private/pkg/execext/usage.gen.go @@ -14,6 +14,6 @@ // Generated. DO NOT EDIT. -package command +package execext import _ "github.com/bufbuild/buf/private/usage" diff --git a/private/pkg/git/cloner.go b/private/pkg/git/cloner.go index f891af6ca6..d79b28e242 100644 --- a/private/pkg/git/cloner.go +++ b/private/pkg/git/cloner.go @@ -24,7 +24,7 @@ import ( "strings" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/slogext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" @@ -35,20 +35,17 @@ import ( type cloner struct { logger *slog.Logger storageosProvider storageos.Provider - runner command.Runner options ClonerOptions } func newCloner( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, options ClonerOptions, ) *cloner { return &cloner{ logger: logger, storageosProvider: storageosProvider, - runner: runner, options: options, } } @@ -89,25 +86,25 @@ func (c *cloner) CloneToBucket( }() buffer := bytes.NewBuffer(nil) - if err := c.runner.Run( + if err := execext.Run( ctx, "git", - command.RunWithArgs("init"), - command.RunWithEnv(app.EnvironMap(envContainer)), - command.RunWithStderr(buffer), - command.RunWithDir(baseDir.Path()), + execext.WithArgs("init"), + execext.WithEnv(app.Environ(envContainer)), + execext.WithStderr(buffer), + execext.WithDir(baseDir.Path()), ); err != nil { return newGitCommandError(err, buffer) } buffer.Reset() - if err := c.runner.Run( + if err := execext.Run( ctx, "git", - command.RunWithArgs("remote", "add", "origin", url), - command.RunWithEnv(app.EnvironMap(envContainer)), - command.RunWithStderr(buffer), - command.RunWithDir(baseDir.Path()), + execext.WithArgs("remote", "add", "origin", url), + execext.WithEnv(app.Environ(envContainer)), + execext.WithStderr(buffer), + execext.WithDir(baseDir.Path()), ); err != nil { return newGitCommandError(err, buffer) } @@ -135,10 +132,10 @@ func (c *cloner) CloneToBucket( var usedFallback bool fetchRef, fallbackRef, checkoutRef := getRefspecsForName(options.Name) buffer.Reset() - if err := c.runner.Run( + if err := execext.Run( ctx, "git", - command.RunWithArgs(append( + execext.WithArgs(append( gitConfigAuthArgs, "fetch", "--depth", depthArg, @@ -146,9 +143,9 @@ func (c *cloner) CloneToBucket( "origin", fetchRef, )...), - command.RunWithEnv(app.EnvironMap(envContainer)), - command.RunWithStderr(buffer), - command.RunWithDir(baseDir.Path()), + execext.WithEnv(app.Environ(envContainer)), + execext.WithStderr(buffer), + execext.WithDir(baseDir.Path()), ); err != nil { // If the ref fetch failed, without a fallback, return the error. if fallbackRef == "" { @@ -157,10 +154,10 @@ func (c *cloner) CloneToBucket( // Failed to fetch the ref directly, try to fetch the fallback ref. usedFallback = true buffer.Reset() - if err := c.runner.Run( + if err := execext.Run( ctx, "git", - command.RunWithArgs(append( + execext.WithArgs(append( gitConfigAuthArgs, "fetch", "--depth", depthArg, @@ -168,9 +165,9 @@ func (c *cloner) CloneToBucket( "origin", fallbackRef, )...), - command.RunWithEnv(app.EnvironMap(envContainer)), - command.RunWithStderr(buffer), - command.RunWithDir(baseDir.Path()), + execext.WithEnv(app.Environ(envContainer)), + execext.WithStderr(buffer), + execext.WithDir(baseDir.Path()), ); err != nil { return newGitCommandError(err, buffer) } @@ -179,13 +176,13 @@ func (c *cloner) CloneToBucket( // Always checkout the FETCH_HEAD to populate the working directory. // This allows for referencing HEAD in checkouts. buffer.Reset() - if err := c.runner.Run( + if err := execext.Run( ctx, "git", - command.RunWithArgs("checkout", "--force", "FETCH_HEAD"), - command.RunWithEnv(app.EnvironMap(envContainer)), - command.RunWithStderr(buffer), - command.RunWithDir(baseDir.Path()), + execext.WithArgs("checkout", "--force", "FETCH_HEAD"), + execext.WithEnv(app.Environ(envContainer)), + execext.WithStderr(buffer), + execext.WithDir(baseDir.Path()), ); err != nil { return newGitCommandError(err, buffer) } @@ -193,13 +190,13 @@ func (c *cloner) CloneToBucket( // from the fetch ref. if checkoutRef != "" && (usedFallback || checkoutRef != fetchRef) { buffer.Reset() - if err := c.runner.Run( + if err := execext.Run( ctx, "git", - command.RunWithArgs("checkout", "--force", checkoutRef), - command.RunWithEnv(app.EnvironMap(envContainer)), - command.RunWithStderr(buffer), - command.RunWithDir(baseDir.Path()), + execext.WithArgs("checkout", "--force", checkoutRef), + execext.WithEnv(app.Environ(envContainer)), + execext.WithStderr(buffer), + execext.WithDir(baseDir.Path()), ); err != nil { return newGitCommandError(err, buffer) } @@ -207,10 +204,10 @@ func (c *cloner) CloneToBucket( if options.RecurseSubmodules { buffer.Reset() - if err := c.runner.Run( + if err := execext.Run( ctx, "git", - command.RunWithArgs(append( + execext.WithArgs(append( gitConfigAuthArgs, "submodule", "update", @@ -220,9 +217,9 @@ func (c *cloner) CloneToBucket( "--depth", depthArg, )...), - command.RunWithEnv(app.EnvironMap(envContainer)), - command.RunWithStderr(buffer), - command.RunWithDir(baseDir.Path()), + execext.WithEnv(app.Environ(envContainer)), + execext.WithStderr(buffer), + execext.WithDir(baseDir.Path()), ); err != nil { return newGitCommandError(err, buffer) } diff --git a/private/pkg/git/cmd/git-ls-files-unstaged/main.go b/private/pkg/git/cmd/git-ls-files-unstaged/main.go index 25c1415e1a..2d66038c9f 100644 --- a/private/pkg/git/cmd/git-ls-files-unstaged/main.go +++ b/private/pkg/git/cmd/git-ls-files-unstaged/main.go @@ -22,7 +22,6 @@ import ( "strings" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/git" ) @@ -31,7 +30,7 @@ func main() { } func run(ctx context.Context, container app.Container) error { - files, err := git.NewLister(command.NewRunner()).ListFilesAndUnstagedFiles( + files, err := git.NewLister().ListFilesAndUnstagedFiles( ctx, container, git.ListFilesAndUnstagedFilesOptions{}, diff --git a/private/pkg/git/git.go b/private/pkg/git/git.go index 04c950d6af..339fad2bd4 100644 --- a/private/pkg/git/git.go +++ b/private/pkg/git/git.go @@ -26,7 +26,7 @@ import ( "strings" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" ) @@ -103,10 +103,9 @@ type CloneToBucketOptions struct { func NewCloner( logger *slog.Logger, storageosProvider storageos.Provider, - runner command.Runner, options ClonerOptions, ) Cloner { - return newCloner(logger, storageosProvider, runner, options) + return newCloner(logger, storageosProvider, options) } // ClonerOptions are options for a new Cloner. @@ -144,8 +143,8 @@ type Lister interface { } // NewLister returns a new Lister. -func NewLister(runner command.Runner) Lister { - return newLister(runner) +func NewLister() Lister { + return newLister() } // ListFilesAndUnstagedFilesOptions are options for ListFilesAndUnstagedFiles. @@ -200,12 +199,11 @@ type Remote interface { // permissions. func GetRemote( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, name string, ) (Remote, error) { - return getRemote(ctx, runner, envContainer, dir, name) + return getRemote(ctx, envContainer, dir, name) } // CheckDirectoryIsValidGitCheckout runs a simple git rev-parse. In the case where the @@ -214,20 +212,19 @@ func GetRemote( // ErrInvalidGitCheckout to the user. func CheckDirectoryIsValidGitCheckout( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, ) error { stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, - command.RunWithArgs("rev-parse"), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(app.EnvironMap(envContainer)), + execext.WithArgs("rev-parse"), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), ); err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) { @@ -244,23 +241,22 @@ func CheckDirectoryIsValidGitCheckout( // changes from git based on the given directory. func CheckForUncommittedGitChanges( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, ) ([]string, error) { stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) var modifiedFiles []string - envMap := app.EnvironMap(envContainer) + environ := app.Environ(envContainer) // Unstaged changes - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, - command.RunWithArgs("diff", "--name-only"), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(envMap), + execext.WithArgs("diff", "--name-only"), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(environ), ); err != nil { return nil, fmt.Errorf("failed to get unstaged changes: %w: %s", err, stderr.String()) } @@ -269,14 +265,14 @@ func CheckForUncommittedGitChanges( stdout = bytes.NewBuffer(nil) stderr = bytes.NewBuffer(nil) // Staged changes - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, - command.RunWithArgs("diff", "--name-only", "--cached"), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(envMap), + execext.WithArgs("diff", "--name-only", "--cached"), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(environ), ); err != nil { return nil, fmt.Errorf("failed to get staged changes: %w: %s", err, stderr.String()) } @@ -288,20 +284,19 @@ func CheckForUncommittedGitChanges( // GetCurrentHEADGitCommit returns the current HEAD commit based on the given directory. func GetCurrentHEADGitCommit( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, ) (string, error) { stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, - command.RunWithArgs("rev-parse", "HEAD"), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(app.EnvironMap(envContainer)), + execext.WithArgs("rev-parse", "HEAD"), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), ); err != nil { return "", fmt.Errorf("failed to get current HEAD commit: %w: %s", err, stderr.String()) } @@ -313,7 +308,6 @@ func GetCurrentHEADGitCommit( // passing the environment for permissions. func GetRefsForGitCommitAndRemote( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, remote string, @@ -321,14 +315,14 @@ func GetRefsForGitCommitAndRemote( ) ([]string, error) { stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, - command.RunWithArgs("ls-remote", "--heads", "--tags", remote), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(app.EnvironMap(envContainer)), + execext.WithArgs("ls-remote", "--heads", "--tags", remote), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), ); err != nil { return nil, fmt.Errorf("failed to get refs for remote %s: %w: %s", remote, err, stderr.String()) } diff --git a/private/pkg/git/git_test.go b/private/pkg/git/git_test.go index b9091c5b15..3bd15af640 100644 --- a/private/pkg/git/git_test.go +++ b/private/pkg/git/git_test.go @@ -28,7 +28,6 @@ import ( "testing" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slogtestext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storagemem" @@ -42,12 +41,11 @@ func TestGitCloner(t *testing.T) { ctx := context.Background() container, err := app.NewContainerForOS() require.NoError(t, err) - runner := command.NewRunner() - originDir, workDir := createGitDirs(ctx, t, container, runner) + originDir, workDir := createGitDirs(ctx, t, container) t.Run("default", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, nil, false) + readBucket := readBucketForName(ctx, t, workDir, 1, nil, false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -60,7 +58,7 @@ func TestGitCloner(t *testing.T) { t.Run("default_submodule", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, nil, true) + readBucket := readBucketForName(ctx, t, workDir, 1, nil, true) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -74,7 +72,7 @@ func TestGitCloner(t *testing.T) { t.Run("main", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewBranchName("main"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewBranchName("main"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -84,7 +82,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=main", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewRefName("main"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewRefName("main"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -95,7 +93,7 @@ func TestGitCloner(t *testing.T) { t.Run("origin/main", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewBranchName("origin/main"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewBranchName("origin/main"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -105,7 +103,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=origin/main", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewRefName("origin/main"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewRefName("origin/main"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -116,7 +114,7 @@ func TestGitCloner(t *testing.T) { t.Run("origin/remote-branch", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewBranchName("origin/remote-branch"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewBranchName("origin/remote-branch"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -127,7 +125,7 @@ func TestGitCloner(t *testing.T) { t.Run("remote-tag", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewTagName("remote-tag"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewTagName("remote-tag"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -137,7 +135,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=remote-tag", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewRefName("remote-tag"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewRefName("remote-tag"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -147,7 +145,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("tag=remote-annotated-tag", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewTagName("remote-annotated-tag"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewTagName("remote-annotated-tag"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -157,7 +155,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=remote-annotated-tag", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewRefName("remote-annotated-tag"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewRefName("remote-annotated-tag"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -168,7 +166,7 @@ func TestGitCloner(t *testing.T) { t.Run("branch_and_main_ref", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefNameWithBranch("HEAD~", "main"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefNameWithBranch("HEAD~", "main"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -178,7 +176,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("branch=main,ref=main~1", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefNameWithBranch("main~", "main"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefNameWithBranch("main~", "main"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -189,7 +187,7 @@ func TestGitCloner(t *testing.T) { t.Run("branch_and_ref", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefNameWithBranch("local-branch~", "local-branch"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefNameWithBranch("local-branch~", "local-branch"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -200,7 +198,7 @@ func TestGitCloner(t *testing.T) { t.Run("ref=HEAD", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 1, NewRefName("HEAD"), false) + readBucket := readBucketForName(ctx, t, workDir, 1, NewRefName("HEAD"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -210,7 +208,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=HEAD~", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefName("HEAD~"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefName("HEAD~"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -220,7 +218,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=HEAD~1", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefName("HEAD~1"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefName("HEAD~1"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -230,7 +228,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=HEAD^", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefName("HEAD^"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefName("HEAD^"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -240,7 +238,7 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=HEAD^1", func(t *testing.T) { t.Parallel() - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefName("HEAD^1"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefName("HEAD^1"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -251,9 +249,9 @@ func TestGitCloner(t *testing.T) { t.Run("ref=", func(t *testing.T) { t.Parallel() - revParseBytes, err := command.RunStdout(ctx, container, runner, "git", "-C", workDir, "rev-parse", "HEAD~") + revParseBytes, err := runStdout(ctx, container, "git", "-C", workDir, "rev-parse", "HEAD~") require.NoError(t, err) - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefName(strings.TrimSpace(string(revParseBytes))), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefName(strings.TrimSpace(string(revParseBytes))), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -263,10 +261,10 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=", func(t *testing.T) { t.Parallel() - revParseBytes, err := command.RunStdout(ctx, container, runner, "git", "-C", workDir, "rev-parse", "HEAD~") + revParseBytes, err := runStdout(ctx, container, "git", "-C", workDir, "rev-parse", "HEAD~") require.NoError(t, err) partialRef := NewRefName(strings.TrimSpace(string(revParseBytes))[:8]) - readBucket := readBucketForName(ctx, t, runner, workDir, 8, partialRef, false) + readBucket := readBucketForName(ctx, t, workDir, 8, partialRef, false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -277,9 +275,9 @@ func TestGitCloner(t *testing.T) { t.Run("ref=,branch=origin/remote-branch", func(t *testing.T) { t.Parallel() - revParseBytes, err := command.RunStdout(ctx, container, runner, "git", "-C", originDir, "rev-parse", "remote-branch~") + revParseBytes, err := runStdout(ctx, container, "git", "-C", originDir, "rev-parse", "remote-branch~") require.NoError(t, err) - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefNameWithBranch(strings.TrimSpace(string(revParseBytes)), "origin/remote-branch"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefNameWithBranch(strings.TrimSpace(string(revParseBytes)), "origin/remote-branch"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -289,10 +287,10 @@ func TestGitCloner(t *testing.T) { }) t.Run("ref=,branch=origin/remote-branch", func(t *testing.T) { t.Parallel() - revParseBytes, err := command.RunStdout(ctx, container, runner, "git", "-C", originDir, "rev-parse", "remote-branch~") + revParseBytes, err := runStdout(ctx, container, "git", "-C", originDir, "rev-parse", "remote-branch~") require.NoError(t, err) partialRef := strings.TrimSpace(string(revParseBytes))[:8] - readBucket := readBucketForName(ctx, t, runner, workDir, 2, NewRefNameWithBranch(partialRef, "origin/remote-branch"), false) + readBucket := readBucketForName(ctx, t, workDir, 2, NewRefNameWithBranch(partialRef, "origin/remote-branch"), false) content, err := storage.ReadPath(ctx, readBucket, "test.proto") require.NoError(t, err) @@ -302,10 +300,10 @@ func TestGitCloner(t *testing.T) { }) } -func readBucketForName(ctx context.Context, t *testing.T, runner command.Runner, path string, depth uint32, name Name, recurseSubmodules bool) storage.ReadBucket { +func readBucketForName(ctx context.Context, t *testing.T, path string, depth uint32, name Name, recurseSubmodules bool) storage.ReadBucket { t.Helper() storageosProvider := storageos.NewProvider(storageos.ProviderWithSymlinks()) - cloner := NewCloner(slogtestext.NewLogger(t), storageosProvider, runner, ClonerOptions{}) + cloner := NewCloner(slogtestext.NewLogger(t), storageosProvider, ClonerOptions{}) envContainer, err := app.NewEnvContainerForOS() require.NoError(t, err) @@ -330,21 +328,20 @@ func createGitDirs( ctx context.Context, t *testing.T, container app.EnvStdioContainer, - runner command.Runner, ) (string, string) { tmpDir := t.TempDir() submodulePath := filepath.Join(tmpDir, "submodule") require.NoError(t, os.MkdirAll(submodulePath, os.ModePerm)) - runCommand(ctx, t, container, runner, "git", "-C", submodulePath, "init") - runCommand(ctx, t, container, runner, "git", "-C", submodulePath, "config", "user.email", "tests@buf.build") - runCommand(ctx, t, container, runner, "git", "-C", submodulePath, "config", "user.name", "Buf go tests") - runCommand(ctx, t, container, runner, "git", "-C", submodulePath, "checkout", "-b", "main") + runCommand(ctx, t, container, "git", "-C", submodulePath, "init") + runCommand(ctx, t, container, "git", "-C", submodulePath, "config", "user.email", "tests@buf.build") + runCommand(ctx, t, container, "git", "-C", submodulePath, "config", "user.name", "Buf go tests") + runCommand(ctx, t, container, "git", "-C", submodulePath, "checkout", "-b", "main") require.NoError(t, os.WriteFile(filepath.Join(submodulePath, "test.proto"), []byte("// submodule"), 0600)) - runCommand(ctx, t, container, runner, "git", "-C", submodulePath, "add", "test.proto") - runCommand(ctx, t, container, runner, "git", "-C", submodulePath, "commit", "-m", "commit 0") + runCommand(ctx, t, container, "git", "-C", submodulePath, "add", "test.proto") + runCommand(ctx, t, container, "git", "-C", submodulePath, "commit", "-m", "commit 0") - gitExecPathBytes, err := command.RunStdout(ctx, container, runner, "git", "--exec-path") + gitExecPathBytes, err := runStdout(ctx, container, "git", "--exec-path") require.NoError(t, err) gitExecPath := strings.TrimSpace(string(gitExecPathBytes)) // In Golang 1.22, the behavior of "os/exec" was changed so that LookPath is no longer called @@ -374,38 +371,38 @@ func createGitDirs( originPath := filepath.Join(tmpDir, "origin") require.NoError(t, os.MkdirAll(originPath, 0777)) - runCommand(ctx, t, container, runner, "git", "-C", originPath, "init") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "config", "user.email", "tests@buf.build") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "config", "user.name", "Buf go tests") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "checkout", "-b", "main") + runCommand(ctx, t, container, "git", "-C", originPath, "init") + runCommand(ctx, t, container, "git", "-C", originPath, "config", "user.email", "tests@buf.build") + runCommand(ctx, t, container, "git", "-C", originPath, "config", "user.name", "Buf go tests") + runCommand(ctx, t, container, "git", "-C", originPath, "checkout", "-b", "main") require.NoError(t, os.WriteFile(filepath.Join(originPath, "test.proto"), []byte("// commit 0"), 0600)) - runCommand(ctx, t, container, runner, "git", "-C", originPath, "add", "test.proto") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "commit", "-m", "commit 0") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "submodule", "add", submodulePath, "submodule") + runCommand(ctx, t, container, "git", "-C", originPath, "add", "test.proto") + runCommand(ctx, t, container, "git", "-C", originPath, "commit", "-m", "commit 0") + runCommand(ctx, t, container, "git", "-C", originPath, "submodule", "add", submodulePath, "submodule") require.NoError(t, os.WriteFile(filepath.Join(originPath, "test.proto"), []byte("// commit 1"), 0600)) - runCommand(ctx, t, container, runner, "git", "-C", originPath, "add", "test.proto") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "commit", "-m", "commit 1") + runCommand(ctx, t, container, "git", "-C", originPath, "add", "test.proto") + runCommand(ctx, t, container, "git", "-C", originPath, "commit", "-m", "commit 1") workPath := filepath.Join(tmpDir, "workdir") - runCommand(ctx, t, container, runner, "git", "clone", originPath, workPath) - runCommand(ctx, t, container, runner, "git", "-C", workPath, "config", "user.email", "tests@buf.build") - runCommand(ctx, t, container, runner, "git", "-C", workPath, "config", "user.name", "Buf go tests") - runCommand(ctx, t, container, runner, "git", "-C", workPath, "checkout", "-b", "local-branch") + runCommand(ctx, t, container, "git", "clone", originPath, workPath) + runCommand(ctx, t, container, "git", "-C", workPath, "config", "user.email", "tests@buf.build") + runCommand(ctx, t, container, "git", "-C", workPath, "config", "user.name", "Buf go tests") + runCommand(ctx, t, container, "git", "-C", workPath, "checkout", "-b", "local-branch") require.NoError(t, os.WriteFile(filepath.Join(workPath, "test.proto"), []byte("// commit 2"), 0600)) - runCommand(ctx, t, container, runner, "git", "-C", workPath, "commit", "-a", "-m", "commit 2") + runCommand(ctx, t, container, "git", "-C", workPath, "commit", "-a", "-m", "commit 2") require.NoError(t, os.WriteFile(filepath.Join(originPath, "test.proto"), []byte("// commit 3"), 0600)) - runCommand(ctx, t, container, runner, "git", "-C", originPath, "add", "test.proto") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "commit", "-m", "commit 3") + runCommand(ctx, t, container, "git", "-C", originPath, "add", "test.proto") + runCommand(ctx, t, container, "git", "-C", originPath, "commit", "-m", "commit 3") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "checkout", "-b", "remote-branch") + runCommand(ctx, t, container, "git", "-C", originPath, "checkout", "-b", "remote-branch") require.NoError(t, os.WriteFile(filepath.Join(originPath, "test.proto"), []byte("// commit 4"), 0600)) - runCommand(ctx, t, container, runner, "git", "-C", originPath, "add", "test.proto") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "commit", "-m", "commit 4") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "tag", "remote-tag") - runCommand(ctx, t, container, runner, "git", "-C", originPath, "tag", "-a", "remote-annotated-tag", "-m", "annotated tag") + runCommand(ctx, t, container, "git", "-C", originPath, "add", "test.proto") + runCommand(ctx, t, container, "git", "-C", originPath, "commit", "-m", "commit 4") + runCommand(ctx, t, container, "git", "-C", originPath, "tag", "remote-tag") + runCommand(ctx, t, container, "git", "-C", originPath, "tag", "-a", "remote-annotated-tag", "-m", "annotated tag") - runCommand(ctx, t, container, runner, "git", "-C", workPath, "fetch", "origin") + runCommand(ctx, t, container, "git", "-C", workPath, "fetch", "origin") return originPath, workPath } @@ -413,12 +410,11 @@ func runCommand( ctx context.Context, t *testing.T, container app.EnvStdioContainer, - runner command.Runner, name string, args ...string, ) { t.Helper() - output, err := command.RunStdout(ctx, container, runner, name, args...) + output, err := runStdout(ctx, container, name, args...) if err != nil { var exitErr *exec.ExitError var stdErr []byte diff --git a/private/pkg/git/lister.go b/private/pkg/git/lister.go index 3736cbc532..d861549f9c 100644 --- a/private/pkg/git/lister.go +++ b/private/pkg/git/lister.go @@ -15,24 +15,21 @@ package git import ( + "bytes" "context" "os" "regexp" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" ) -type lister struct { - runner command.Runner -} +type lister struct{} -func newLister(runner command.Runner) *lister { - return &lister{ - runner: runner, - } +func newLister() *lister { + return &lister{} } func (l *lister) ListFilesAndUnstagedFiles( @@ -40,10 +37,9 @@ func (l *lister) ListFilesAndUnstagedFiles( container app.EnvStdioContainer, options ListFilesAndUnstagedFilesOptions, ) ([]string, error) { - allFilesOutput, err := command.RunStdout( + allFilesOutput, err := runStdout( ctx, container, - l.runner, "git", "ls-files", "--cached", @@ -54,10 +50,9 @@ func (l *lister) ListFilesAndUnstagedFiles( if err != nil { return nil, err } - deletedFilesOutput, err := command.RunStdout( + deletedFilesOutput, err := runStdout( ctx, container, - l.runner, "git", "ls-files", "--deleted", @@ -130,3 +125,19 @@ func filterNonRegularFiles(files []string) []string { } return filteredFiles } + +func runStdout(ctx context.Context, container app.EnvStdioContainer, name string, args ...string) ([]byte, error) { + buffer := bytes.NewBuffer(nil) + if err := execext.Run( + ctx, + name, + execext.WithArgs(args...), + execext.WithEnv(app.Environ(container)), + execext.WithStdin(container.Stdin()), + execext.WithStdout(buffer), + execext.WithStderr(container.Stderr()), + ); err != nil { + return nil, err + } + return buffer.Bytes(), nil +} diff --git a/private/pkg/git/remote.go b/private/pkg/git/remote.go index 822017f755..52f2c78941 100644 --- a/private/pkg/git/remote.go +++ b/private/pkg/git/remote.go @@ -26,7 +26,7 @@ import ( "strings" "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" ) const ( @@ -123,17 +123,15 @@ func newRemote( func getRemote( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, name string, ) (*remote, error) { - if err := validateRemoteExists(ctx, runner, envContainer, dir, name); err != nil { + if err := validateRemoteExists(ctx, envContainer, dir, name); err != nil { return nil, err } hostname, repositoryPath, err := getRemoteURLMetadata( ctx, - runner, envContainer, dir, name, @@ -141,7 +139,7 @@ func getRemote( if err != nil { return nil, fmt.Errorf("failed to get remote URL metadata: %w", err) } - headBranch, err := getRemoteHEADBranch(ctx, runner, envContainer, dir, name) + headBranch, err := getRemoteHEADBranch(ctx, envContainer, dir, name) if err != nil { return nil, fmt.Errorf("failed to get remote HEAD branch: %w", err) } @@ -156,21 +154,20 @@ func getRemote( func validateRemoteExists( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, name string, ) error { stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, - command.RunWithArgs("ls-remote", "--exit-code", name), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(app.EnvironMap(envContainer)), + execext.WithArgs("ls-remote", "--exit-code", name), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), ); err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) { @@ -185,23 +182,22 @@ func validateRemoteExists( func getRemoteURLMetadata( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, remote string, ) (string, string, error) { stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, // We use `git config --get remote..url` instead of `git remote get-url // since it is more specific to the checkout. - command.RunWithArgs("config", "--get", fmt.Sprintf("remote.%s.url", remote)), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(app.EnvironMap(envContainer)), + execext.WithArgs("config", "--get", fmt.Sprintf("remote.%s.url", remote)), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), ); err != nil { return "", "", err } @@ -246,21 +242,20 @@ func parseSCPLikeURL(rawURL string) *url.URL { // environment for permissions. func getRemoteHEADBranch( ctx context.Context, - runner command.Runner, envContainer app.EnvContainer, dir string, remote string, ) (string, error) { stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) - if err := runner.Run( + if err := execext.Run( ctx, gitCommand, - command.RunWithArgs("remote", "show", remote), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), - command.RunWithDir(dir), - command.RunWithEnv(app.EnvironMap(envContainer)), + execext.WithArgs("remote", "show", remote), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), ); err != nil { return "", err } diff --git a/private/pkg/licenseheader/cmd/license-header/main.go b/private/pkg/licenseheader/cmd/license-header/main.go index 40e571044b..040a58fd22 100644 --- a/private/pkg/licenseheader/cmd/license-header/main.go +++ b/private/pkg/licenseheader/cmd/license-header/main.go @@ -23,7 +23,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/app/appcmd" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/diff" "github.com/bufbuild/buf/private/pkg/git" "github.com/bufbuild/buf/private/pkg/licenseheader" @@ -130,8 +129,7 @@ func run(ctx context.Context, container app.Container, flags *flags) error { return newRequiredFlagError(yearRangeFlagName) } } - runner := command.NewRunner() - filenames, err := getFilenames(ctx, container, runner, flags.Ignore) + filenames, err := getFilenames(ctx, container, flags.Ignore) if err != nil { return err } @@ -154,7 +152,6 @@ func run(ctx context.Context, container app.Container, flags *flags) error { if flags.Diff { diffData, err := diff.Diff( ctx, - runner, data, modifiedData, filename, @@ -188,7 +185,6 @@ func run(ctx context.Context, container app.Container, flags *flags) error { func getFilenames( ctx context.Context, container app.Container, - runner command.Runner, ignores []string, ) ([]string, error) { if container.NumArgs() > 0 { @@ -205,7 +201,7 @@ func getFilenames( } ignoreRegexps[i] = ignoreRegexp } - return git.NewLister(runner).ListFilesAndUnstagedFiles( + return git.NewLister().ListFilesAndUnstagedFiles( ctx, container, git.ListFilesAndUnstagedFilesOptions{ diff --git a/private/pkg/pluginrpcutil/pluginrpcutil.go b/private/pkg/pluginrpcutil/pluginrpcutil.go index ef50a20146..3164e316fa 100644 --- a/private/pkg/pluginrpcutil/pluginrpcutil.go +++ b/private/pkg/pluginrpcutil/pluginrpcutil.go @@ -15,14 +15,13 @@ package pluginrpcutil import ( - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" ) -// NewRunner returns a new pluginrpc.Runner for the command.Runner and program name. -func NewRunner(delegate command.Runner, programName string, programArgs ...string) pluginrpc.Runner { - return newRunner(delegate, programName, programArgs...) +// NewRunner returns a new pluginrpc.Runner for the program name. +func NewRunner(programName string, programArgs ...string) pluginrpc.Runner { + return newRunner(programName, programArgs...) } // NewWasmRunner returns a new pluginrpc.Runner for the wasm.Runtime and program name. diff --git a/private/pkg/pluginrpcutil/runner.go b/private/pkg/pluginrpcutil/runner.go index af29b6af70..258d18a6e2 100644 --- a/private/pkg/pluginrpcutil/runner.go +++ b/private/pkg/pluginrpcutil/runner.go @@ -20,23 +20,20 @@ import ( "os/exec" "slices" - "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/execext" "pluginrpc.com/pluginrpc" ) type runner struct { - delegate command.Runner programName string programArgs []string } func newRunner( - delegate command.Runner, programName string, programArgs ...string, ) *runner { return &runner{ - delegate: delegate, programName: programName, programArgs: programArgs, } @@ -47,13 +44,13 @@ func (r *runner) Run(ctx context.Context, env pluginrpc.Env) error { if len(r.programArgs) > 0 { args = append(slices.Clone(r.programArgs), env.Args...) } - if err := r.delegate.Run( + if err := execext.Run( ctx, r.programName, - command.RunWithArgs(args...), - command.RunWithStdin(env.Stdin), - command.RunWithStdout(env.Stdout), - command.RunWithStderr(env.Stderr), + execext.WithArgs(args...), + execext.WithStdin(env.Stdin), + execext.WithStdout(env.Stdout), + execext.WithStderr(env.Stderr), ); err != nil { execExitError := &exec.ExitError{} if errors.As(err, &execExitError) { diff --git a/private/pkg/prototesting/prototesting.go b/private/pkg/prototesting/prototesting.go index b1b25f201a..9a8570d4b2 100644 --- a/private/pkg/prototesting/prototesting.go +++ b/private/pkg/prototesting/prototesting.go @@ -25,8 +25,8 @@ import ( "strings" "testing" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/diff" + "github.com/bufbuild/buf/private/pkg/execext" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" @@ -41,7 +41,6 @@ import ( // Only use for testing. func GetProtocFileDescriptorSet( ctx context.Context, - runner command.Runner, roots []string, realFilePaths []string, includeImports bool, @@ -63,7 +62,6 @@ func GetProtocFileDescriptorSet( if err := RunProtoc( ctx, - runner, roots, realFilePaths, includeImports, @@ -99,8 +97,7 @@ func GetProtocFileDescriptorSet( // GetProtocVersion runs protoc --version with RunProtoc and returns the output sans the "libprotoc" prefix func GetProtocVersion(ctx context.Context) (string, error) { var stdout bytes.Buffer - runner := command.NewRunner() - if err := RunProtoc(ctx, runner, nil, nil, false, false, nil, &stdout, "--version"); err != nil { + if err := RunProtoc(ctx, nil, nil, false, false, nil, &stdout, "--version"); err != nil { return "", err } return strings.TrimSpace(strings.TrimPrefix(stdout.String(), "libprotoc")), nil @@ -109,7 +106,6 @@ func GetProtocVersion(ctx context.Context) (string, error) { // RunProtoc runs protoc. func RunProtoc( ctx context.Context, - runner command.Runner, roots []string, realFilePaths []string, includeImports bool, @@ -146,13 +142,17 @@ func RunProtoc( if stdout == nil { stdout = stderr } - if err := runner.Run( + environ := make([]string, 0, len(env)) + for key, value := range env { + environ = append(environ, key+"="+value) + } + if err := execext.Run( ctx, protocBinPath, - command.RunWithArgs(args...), - command.RunWithEnv(env), - command.RunWithStdout(stdout), - command.RunWithStderr(stderr), + execext.WithArgs(args...), + execext.WithEnv(environ), + execext.WithStdout(stdout), + execext.WithStderr(stderr), ); err != nil { return fmt.Errorf("%s returned error: %v %v", protocBinPath, err, stderr.String()) } @@ -162,7 +162,6 @@ func RunProtoc( // DiffFileDescriptorSetsWire diffs the two FileDescriptorSets using proto.MarshalWire. func DiffFileDescriptorSetsWire( ctx context.Context, - runner command.Runner, one *descriptorpb.FileDescriptorSet, two *descriptorpb.FileDescriptorSet, oneName string, @@ -176,7 +175,7 @@ func DiffFileDescriptorSetsWire( if err != nil { return "", err } - output, err := diff.Diff(ctx, runner, oneData, twoData, oneName, twoName) + output, err := diff.Diff(ctx, oneData, twoData, oneName, twoName) if err != nil { return "", err } @@ -186,7 +185,6 @@ func DiffFileDescriptorSetsWire( // DiffFileDescriptorSetsJSON diffs the two FileDescriptorSets using JSON. func DiffFileDescriptorSetsJSON( ctx context.Context, - runner command.Runner, one *descriptorpb.FileDescriptorSet, two *descriptorpb.FileDescriptorSet, oneName string, @@ -208,7 +206,7 @@ func DiffFileDescriptorSetsJSON( if err != nil { return "", err } - output, err := diff.Diff(ctx, runner, oneData, twoData, oneName, twoName) + output, err := diff.Diff(ctx, oneData, twoData, oneName, twoName) if err != nil { return "", err } @@ -227,11 +225,10 @@ func DiffFileDescriptorSetsCompare( // JSON and compare. func AssertFileDescriptorSetsEqual( t *testing.T, - runner command.Runner, one *descriptorpb.FileDescriptorSet, two *descriptorpb.FileDescriptorSet, ) { - diff, err := DiffFileDescriptorSetsJSON(context.Background(), runner, one, two, "buf", "protoc") + diff, err := DiffFileDescriptorSetsJSON(context.Background(), one, two, "buf", "protoc") assert.NoError(t, err) assert.Empty(t, diff) diff = DiffFileDescriptorSetsCompare(one, two) diff --git a/private/pkg/storage/cmd/ddiff/main.go b/private/pkg/storage/cmd/ddiff/main.go index 62df8b663c..1ade10b088 100644 --- a/private/pkg/storage/cmd/ddiff/main.go +++ b/private/pkg/storage/cmd/ddiff/main.go @@ -22,7 +22,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/app/appcmd" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" ) @@ -64,7 +63,6 @@ func run(ctx context.Context, container app.Container) error { } return storage.Diff( ctx, - command.NewRunner(), container.Stdout(), oneReadWriteBucket, twoReadWriteBucket, diff --git a/private/pkg/storage/diff.go b/private/pkg/storage/diff.go index f61d51735e..8879b4b8e4 100644 --- a/private/pkg/storage/diff.go +++ b/private/pkg/storage/diff.go @@ -22,7 +22,6 @@ import ( "sort" "strings" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/diff" ) @@ -101,13 +100,12 @@ func DiffWithTransform( // DiffBytes does a diff of the ReadBuckets. func DiffBytes( ctx context.Context, - runner command.Runner, one ReadBucket, two ReadBucket, options ...DiffOption, ) ([]byte, error) { buffer := bytes.NewBuffer(nil) - if err := Diff(ctx, runner, buffer, one, two, options...); err != nil { + if err := Diff(ctx, buffer, one, two, options...); err != nil { return nil, err } return buffer.Bytes(), nil @@ -116,13 +114,12 @@ func DiffBytes( // Diff writes a diff of the ReadBuckets to the Writer. func Diff( ctx context.Context, - runner command.Runner, writer io.Writer, one ReadBucket, two ReadBucket, options ...DiffOption, ) error { - _, err := DiffWithFilenames(ctx, runner, writer, one, two, options...) + _, err := DiffWithFilenames(ctx, writer, one, two, options...) return err } @@ -136,7 +133,6 @@ func Diff( // to change. func DiffWithFilenames( ctx context.Context, - runner command.Runner, writer io.Writer, one ReadBucket, two ReadBucket, @@ -214,7 +210,6 @@ func DiffWithFilenames( } diffData, err := diff.Diff( ctx, - runner, oneData, twoData, oneDiffPath, @@ -257,7 +252,6 @@ func DiffWithFilenames( } diffData, err := diff.Diff( ctx, - runner, nil, twoData, oneDiffPath, diff --git a/private/pkg/storage/storagetesting/storagetesting.go b/private/pkg/storage/storagetesting/storagetesting.go index 6a65ccb307..15d4d291d5 100644 --- a/private/pkg/storage/storagetesting/storagetesting.go +++ b/private/pkg/storage/storagetesting/storagetesting.go @@ -30,7 +30,6 @@ import ( "sync" "testing" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storagearchive" @@ -178,7 +177,6 @@ func RunTestSuite( symlinkLoopDirPath := filepath.Join(storagetestingDirPath, "testdata", "symlink_loop") overlayDirPath := filepath.Join(storagetestingDirPath, "testdata", "overlay") defaultProvider := storageos.NewProvider() - runner := command.NewRunner() for _, prefix := range []string{ "", @@ -1042,7 +1040,6 @@ func RunTestSuite( diff, err := storage.DiffBytes( context.Background(), - runner, readBucketA, readBucketB, storage.DiffWithSuppressTimestamps(),