Remove command package and command.Runner in favor of simpler and sta…

…teless execext package (#3439)

.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

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
 }

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,

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))
 					})

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

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

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

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
 		}

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
 	}

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...,