Replace shell.Run with functional equivalent

internal/job/artifacts.go

@@ -69,7 +69,7 @@ func (e *Executor) uploadArtifacts(ctx context.Context) error {
 		args = append(args, e.ArtifactUploadDestination)
 	}
 
-	if err = e.shell.Run(ctx, "buildkite-agent", args...); err != nil {
+	if err = e.shell.Command("buildkite-agent", args...).Run(ctx); err != nil {
 		return err
 	}
 

internal/job/checkout.go

@@ -382,12 +382,14 @@ func (e *Executor) updateGitMirror(ctx context.Context, repository string) (stri
 			e.shell.Commentf("Fetch and mirror pull request head from GitHub")
 			refspec := fmt.Sprintf("refs/pull/%s/head", e.PullRequest)
 			// Fetch the PR head from the upstream repository into the mirror.
-			if err := e.shell.Run(ctx, "git", "--git-dir", mirrorDir, "fetch", "origin", refspec); err != nil {
+			cmd := e.shell.Command("git", "--git-dir", mirrorDir, "fetch", "origin", refspec)
+			if err := cmd.Run(ctx); err != nil {
 				return "", err
 			}
 		} else {
 			// Fetch the build branch from the upstream repository into the mirror.
-			if err := e.shell.Run(ctx, "git", "--git-dir", mirrorDir, "fetch", "origin", e.Branch); err != nil {
+			cmd := e.shell.Command("git", "--git-dir", mirrorDir, "fetch", "origin", e.Branch)
+			if err := cmd.Run(ctx); err != nil {
 				return "", err
 			}
 		}
@@ -400,7 +402,8 @@ func (e *Executor) updateGitMirror(ctx context.Context, repository string) (stri
 		// a clean host or with a clean checkout.)
 		// TODO: Investigate getting the ref from the main repo and passing
 		// that in here.
-		if err := e.shell.Run(ctx, "git", "--git-dir", mirrorDir, "fetch", "origin"); err != nil {
+		cmd := e.shell.Command("git", "--git-dir", mirrorDir, "fetch", "origin")
+		if err := cmd.Run(ctx); err != nil {
 			return "", err
 		}
 	}
@@ -409,10 +412,10 @@ func (e *Executor) updateGitMirror(ctx context.Context, repository string) (stri
 		// Let's opportunistically fsck and gc.
 		// 1. In case of remote URL confusion (bug introduced in #1959), and
 		// 2. There's possibly some object churn when remotes are renamed.
-		if err := e.shell.Run(ctx, "git", "--git-dir", mirrorDir, "fsck"); err != nil {
+		if err := e.shell.Command("git", "--git-dir", mirrorDir, "fsck").Run(ctx); err != nil {
 			e.shell.Logger.Warningf("Couldn't run git fsck: %v", err)
 		}
-		if err := e.shell.Run(ctx, "git", "--git-dir", mirrorDir, "gc"); err != nil {
+		if err := e.shell.Command("git", "--git-dir", mirrorDir, "gc").Run(ctx); err != nil {
 			e.shell.Logger.Warningf("Couldn't run git gc: %v", err)
 		}
 	}
@@ -468,7 +471,7 @@ func (e *Executor) updateRemoteURL(ctx context.Context, gitDir, repository strin
 	if gitDir != "" {
 		args = append([]string{"--git-dir", gitDir}, args...)
 	}
-	return true, e.shell.Run(ctx, "git", args...)
+	return true, e.shell.Command("git", args...).Run(ctx)
 }
 
 func (e *Executor) getOrUpdateMirrorDir(ctx context.Context, repository string) (string, error) {
@@ -631,7 +634,7 @@ func (e *Executor) defaultCheckoutPhase(ctx context.Context) error {
 		// is only available in git version 1.8.1, so
 		// if the call fails, continue the job
 		// script, and show an informative error.
-		if err := e.shell.Run(ctx, "git", "submodule", "sync", "--recursive"); err != nil {
+		if err := e.shell.Command("git", "submodule", "sync", "--recursive").Run(ctx); err != nil {
 			gitVersionOutput, _ := e.shell.Command("git", "--version").RunAndCaptureStdout(ctx)
 			e.shell.Warningf("Failed to recursively sync git submodules. This is most likely because you have an older version of git installed (" + gitVersionOutput + ") and you need version 1.8.1 and above. If you're using submodules, it's highly recommended you upgrade if you can.")
 		}
@@ -684,19 +687,20 @@ func (e *Executor) defaultCheckoutPhase(ctx context.Context) error {
 					submoduleArgs = append(submoduleArgs, "submodule", "update", "--init", "--recursive", "--force")
 				}
 
-				if err := e.shell.Run(ctx, "git", submoduleArgs...); err != nil {
+				if err := e.shell.Command("git", submoduleArgs...).Run(ctx); err != nil {
 					return fmt.Errorf("updating submodules: %w", err)
 				}
 			}
 
 			if !mirrorSubmodules {
 				args = append(args, "submodule", "update", "--init", "--recursive", "--force")
-				if err := e.shell.Run(ctx, "git", args...); err != nil {
+				if err := e.shell.Command("git", args...).Run(ctx); err != nil {
 					return fmt.Errorf("updating submodules: %w", err)
 				}
 			}
 
-			if err := e.shell.Run(ctx, "git", "submodule", "foreach", "--recursive", "git reset --hard"); err != nil {
+			cmd := e.shell.Command("git", "submodule", "foreach", "--recursive", "git reset --hard")
+			if err := cmd.Run(ctx); err != nil {
 				return fmt.Errorf("resetting submodules: %w", err)
 			}
 		}
@@ -777,7 +781,8 @@ const CommitMetadataKey = "buildkite:git:commit"
 // note that we bail early if the key already exists, as we don't want to overwrite it
 func (e *Executor) sendCommitToBuildkite(ctx context.Context) error {
 	e.shell.Commentf("Checking to see if git commit information needs to be sent to Buildkite...")
-	if err := e.shell.Run(ctx, "buildkite-agent", "meta-data", "exists", CommitMetadataKey); err == nil {
+	cmd := e.shell.Command("buildkite-agent", "meta-data", "exists", CommitMetadataKey)
+	if err := cmd.Run(ctx); err == nil {
 		// Command exited 0, ie the key exists, so we don't need to send it again
 		e.shell.Commentf("Git commit information has already been sent to Buildkite")
 		return nil
@@ -809,7 +814,8 @@ func (e *Executor) sendCommitToBuildkite(ctx context.Context) error {
 	}
 
 	stdin := strings.NewReader(out)
-	if err := e.shell.CloneWithStdin(stdin).Run(ctx, "buildkite-agent", "meta-data", "set", CommitMetadataKey); err != nil {
+	cmd = e.shell.CloneWithStdin(stdin).Command("buildkite-agent", "meta-data", "set", CommitMetadataKey)
+	if err := cmd.Run(ctx); err != nil {
 		return fmt.Errorf("sending git commit information to Buildkite: %w", err)
 	}
 

internal/job/docker.go

@@ -61,7 +61,7 @@ func tearDownDeprecatedDockerIntegration(ctx context.Context, sh *shell.Shell) e
 	if container, ok := sh.Env.Get("DOCKER_CONTAINER"); ok {
 		sh.Printf("~~~ Cleaning up Docker containers")
 
-		if err := sh.Run(ctx, "docker", "rm", "-f", "-v", container); err != nil {
+		if err := sh.Command("docker", "rm", "-f", "-v", container).Run(ctx); err != nil {
 			return err
 		}
 	} else if projectName, ok := sh.Env.Get("COMPOSE_PROJ_NAME"); ok {
@@ -98,12 +98,14 @@ func runDockerCommand(ctx context.Context, sh *shell.Shell, cmd []string) error
 	sh.Env.Set("DOCKER_IMAGE", dockerImage)
 
 	sh.Printf("~~~ :docker: Building Docker image %s", dockerImage)
-	if err := sh.Run(ctx, "docker", "build", "-f", dockerFile, "-t", dockerImage, "."); err != nil {
+	shCmd := sh.Command("docker", "build", "-f", dockerFile, "-t", dockerImage, ".")
+	if err := shCmd.Run(ctx); err != nil {
 		return err
 	}
 
 	sh.Headerf(":docker: Running command (in Docker container)")
-	if err := sh.Run(ctx, "docker", append([]string{"run", "--name", dockerContainer, dockerImage}, cmd...)...); err != nil {
+	shCmd = sh.Command("docker", append([]string{"run", "--name", dockerContainer, dockerImage}, cmd...)...)
+	if err := shCmd.Run(ctx); err != nil {
 		return err
 	}
 
@@ -159,5 +161,5 @@ func runDockerCompose(ctx context.Context, sh *shell.Shell, projectName string,
 	}
 
 	args = append(args, commandArgs...)
-	return sh.Run(ctx, "docker-compose", args...)
+	return sh.Command("docker-compose", args...).Run(ctx)
 }

internal/job/plugin.go

@@ -389,7 +389,7 @@ func (e *Executor) checkoutPlugin(ctx context.Context, p *plugin.Plugin) (*plugi
 		roko.WithMaxAttempts(3),
 		roko.WithStrategy(roko.Constant(2*time.Second)),
 	).DoWithContext(ctx, func(r *roko.Retrier) error {
-		return e.shell.Run(ctx, "git", args...)
+		return e.shell.Command("git", args...).Run(ctx)
 	})
 	if err != nil {
 		return nil, err
@@ -398,7 +398,7 @@ func (e *Executor) checkoutPlugin(ctx context.Context, p *plugin.Plugin) (*plugi
 	// Switch to the version if we need to
 	if p.Version != "" {
 		e.shell.Commentf("Checking out `%s`", p.Version)
-		if err = e.shell.Run(ctx, "git", "checkout", "-f", p.Version); err != nil {
+		if err = e.shell.Command("git", "checkout", "-f", p.Version).Run(ctx); err != nil {
 			return nil, err
 		}
 	}

internal/shell/shell.go

@@ -495,12 +495,6 @@ func WithExtraEnv(e *env.Environment) RunCommandOpt { return func(c *runConfig)
 // ones were observed.
 func WithStringSearch(m map[string]bool) RunCommandOpt { return func(c *runConfig) { c.smells = m } }
 
-// Run runs a command, write stdout and stderr to the logger and return an error
-// if it fails.
-func (s *Shell) Run(ctx context.Context, command string, arg ...string) error {
-	return s.Command(command, arg...).Run(ctx)
-}
-
 // injectTraceCtx adds tracing information to the given env vars to support
 // distributed tracing across jobs/builds.
 func (s *Shell) injectTraceCtx(ctx context.Context, env *env.Environment) {

internal/shell/shell_test.go

@@ -81,6 +81,7 @@ func TestRunAndCaptureWithExitCode(t *testing.T) {
 
 func TestRun(t *testing.T) {
 	t.Parallel()
+	ctx := context.Background()
 
 	sshKeygen, err := bintest.CompileProxy("ssh-keygen")
 	if err != nil {
@@ -102,7 +103,8 @@ func TestRun(t *testing.T) {
 		call.Exit(0)
 	}()
 
-	if err := sh.Run(context.Background(), sshKeygen.Path, "-f", "my_hosts", "-F", "llamas.com"); err != nil {
+	cmd := sh.Command(sshKeygen.Path, "-f", "my_hosts", "-F", "llamas.com")
+	if err := cmd.Run(ctx); err != nil {
 		t.Errorf(`sh.Run(ssh-keygen, "-f", "my_hosts", "-F", "llamas.com") error = %v`, err)
 	}
 
@@ -119,11 +121,12 @@ func TestRun(t *testing.T) {
 
 func TestRunWithStdin(t *testing.T) {
 	t.Parallel()
+	ctx := context.Background()
 
 	out := &bytes.Buffer{}
 	sh := newShellForTest(t, shell.WithStdout(out), shell.WithPTY(false))
-
-	if err := sh.CloneWithStdin(strings.NewReader("hello stdin")).Run(context.Background(), "tr", "hs", "HS"); err != nil {
+	cmd := sh.CloneWithStdin(strings.NewReader("hello stdin")).Command("tr", "hs", "HS")
+	if err := cmd.Run(ctx); err != nil {
 		t.Fatalf(`sh.WithStdin("hello stdin").Run("tr", "hs", "HS") error = %v`, err)
 	}
 	if got, want := out.String(), "Hello Stdin"; want != got {
@@ -133,6 +136,7 @@ func TestRunWithStdin(t *testing.T) {
 
 func TestContextCancelTerminates(t *testing.T) {
 	t.Parallel()
+	ctx := context.Background()
 
 	if runtime.GOOS == "windows" {
 		t.Skip("Not supported in windows")
@@ -144,7 +148,7 @@ func TestContextCancelTerminates(t *testing.T) {
 	}
 	defer sleepCmd.Close()
 
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	sh, err := shell.New()
@@ -162,13 +166,14 @@ func TestContextCancelTerminates(t *testing.T) {
 
 	cancel()
 
-	if err := sh.Run(ctx, sleepCmd.Path); !shell.IsExitSignaled(err) {
+	if err := sh.Command(sleepCmd.Path).Run(ctx); !shell.IsExitSignaled(err) {
 		t.Errorf("sh.Run(ctx, sleep) error = %v, want shell.IsExitSignaled(err) = true", err)
 	}
 }
 
 func TestInterrupt(t *testing.T) {
 	t.Parallel()
+	ctx := context.Background()
 
 	if runtime.GOOS == "windows" {
 		t.Skip("Not supported in windows")
@@ -180,7 +185,7 @@ func TestInterrupt(t *testing.T) {
 	}
 	defer sleepCmd.Close()
 
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	sh, err := shell.New()
@@ -202,7 +207,7 @@ func TestInterrupt(t *testing.T) {
 		sh.Interrupt()
 	}()
 
-	if err := sh.Run(ctx, sleepCmd.Path); err == nil {
+	if err := sh.Command(sleepCmd.Path).Run(ctx); err == nil {
 		t.Errorf("sh.Run(ctx, sleep) = %v, want non-nil error", err)
 	}
 }
@@ -306,6 +311,7 @@ func TestWorkingDir(t *testing.T) {
 
 func TestLockFileRetriesAndTimesOut(t *testing.T) {
 	t.Parallel()
+	ctx := context.Background()
 
 	if runtime.GOOS == "windows" {
 		t.Skip("Flakey on windows")
@@ -327,7 +333,7 @@ func TestLockFileRetriesAndTimesOut(t *testing.T) {
 	cmd := acquireLockInOtherProcess(t, lockPath)
 	defer func() { assert.NilError(t, cmd.Process.Kill()) }()
 
-	ctx, canc := context.WithTimeout(context.Background(), 2*time.Second)
+	ctx, canc := context.WithTimeout(ctx, 2*time.Second)
 	defer canc()
 
 	lock, err := sh.LockFile(ctx, lockPath)
@@ -443,6 +449,7 @@ func TestRunWithOlfactor(t *testing.T) {
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
+			ctx := context.Background()
 
 			out := &bytes.Buffer{}
 			sh, err := shell.New(shell.WithStdout(out))
@@ -454,7 +461,7 @@ func TestRunWithOlfactor(t *testing.T) {
 			}
 
 			err = sh.Command(test.command[0], test.command[1:]...).Run(
-				context.Background(),
+				ctx,
 				shell.WithStringSearch(smelled),
 			)
 			if eerr := new(exec.ExitError); !errors.As(err, &eerr) {