refactor: address fourth round of PR review feedback

- Rename helpers: gitCmd -> cmd, gitRun -> exec, gitPipeline -> pipe
- Remove stderr parameter from pipe (stderr is already in the error)
- Remove extractStderr (no longer used after stderr removal)
- Remove Blob.Pipeline stderr parameter and update all callers
- Rename CreateArchive -> Archive and add --end-of-options

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: unknwon

blob.go

@@ -25,14 +25,14 @@ func (b *Blob) Bytes(ctx context.Context) ([]byte, error) {
 		stdout.Grow(int(size))
 	}
 
-	if err := b.Pipeline(ctx, stdout, nil); err != nil {
+	if err := b.Pipeline(ctx, stdout); err != nil {
 		return nil, err
 	}
 	return stdout.Bytes(), nil
 }
 
-// Pipeline reads the content of the blob and pipes stdout and stderr to
-// supplied io.Writer.
-func (b *Blob) Pipeline(ctx context.Context, stdout, stderr io.Writer) error {
-	return gitPipeline(ctx, b.parent.repo.path, []string{"show", b.id.String()}, nil, stdout, stderr)
+// Pipeline reads the content of the blob and pipes stdout to the supplied
+// io.Writer.
+func (b *Blob) Pipeline(ctx context.Context, stdout io.Writer) error {
+	return pipe(ctx, b.parent.repo.path, []string{"show", b.id.String()}, nil, stdout)
 }

blob_test.go

@@ -50,7 +50,7 @@ This demo also includes an image with changes on a branch for examination of ima
 
 	t.Run("get data with pipeline", func(t *testing.T) {
 		stdout := new(bytes.Buffer)
-		err := blob.Pipeline(ctx, stdout, nil)
+		err := blob.Pipeline(ctx, stdout)
 		assert.Nil(t, err)
 		assert.Equal(t, expOutput, stdout.String())
 	})

command.go

@@ -7,7 +7,6 @@ package git
 import (
 	"bytes"
 	"context"
-	"fmt"
 	"io"
 	"os"
 	"strconv"
@@ -27,10 +26,10 @@ type CommandOptions struct {
 // applied when the context does not already have a deadline.
 const DefaultTimeout = time.Minute
 
-// gitCmd builds a *run.Command for "git" with the given arguments, environment
+// cmd builds a *run.Command for "git" with the given arguments, environment
 // variables and working directory. If the context does not already have a
 // deadline, DefaultTimeout will be applied automatically.
-func gitCmd(ctx context.Context, dir string, args []string, envs []string) (*run.Command, context.CancelFunc) {
+func cmd(ctx context.Context, dir string, args []string, envs []string) (*run.Command, context.CancelFunc) {
 	cancel := func() {}
 
 	// Apply default timeout if the context doesn't already have a deadline.
@@ -49,22 +48,22 @@ func gitCmd(ctx context.Context, dir string, args []string, envs []string) (*run
 		parts = append(parts, run.Arg(arg))
 	}
 
-	cmd := run.Cmd(ctx, parts...)
+	c := run.Cmd(ctx, parts...)
 	if dir != "" {
-		cmd = cmd.Dir(dir)
+		c = c.Dir(dir)
 	}
 	if len(envs) > 0 {
-		cmd = cmd.Environ(append(os.Environ(), envs...))
+		c = c.Environ(append(os.Environ(), envs...))
 	}
-	return cmd, cancel
+	return c, cancel
 }
 
-// gitRun executes a git command in the given directory and returns stdout as
+// exec executes a git command in the given directory and returns stdout as
 // bytes. Stderr is included in the error message on failure. If the command's
 // context does not have a deadline, DefaultTimeout will be applied
 // automatically. It returns an ErrExecTimeout if the execution was timed out.
-func gitRun(ctx context.Context, dir string, args []string, envs []string) ([]byte, error) {
-	cmd, cancel := gitCmd(ctx, dir, args, envs)
+func exec(ctx context.Context, dir string, args []string, envs []string) ([]byte, error) {
+	c, cancel := cmd(ctx, dir, args, envs)
 	defer cancel()
 
 	var logBuf *bytes.Buffer
@@ -80,7 +79,7 @@ func gitRun(ctx context.Context, dir string, args []string, envs []string) ([]by
 	// commands like "ls-tree -z"). The String/Lines methods process output
 	// line-by-line which corrupts binary-ish output.
 	stdout := new(bytes.Buffer)
-	err := cmd.StdOut().Run().Stream(stdout)
+	err := c.StdOut().Run().Stream(stdout)
 
 	// Capture (partial) stdout for logging even on error, so failed commands
 	// produce a useful log entry rather than an empty one.
@@ -102,11 +101,10 @@ func gitRun(ctx context.Context, dir string, args []string, envs []string) ([]by
 	return stdout.Bytes(), nil
 }
 
-// gitPipeline executes a git command in the given directory, streaming stdout
-// to the given writer. If stderr writer is provided and the command fails,
-// stderr content extracted from the error is written to it.
-func gitPipeline(ctx context.Context, dir string, args []string, envs []string, stdout, stderr io.Writer) error {
-	cmd, cancel := gitCmd(ctx, dir, args, envs)
+// pipe executes a git command in the given directory, streaming stdout to the
+// given writer.
+func pipe(ctx context.Context, dir string, args []string, envs []string, stdout io.Writer) error {
+	c, cancel := cmd(ctx, dir, args, envs)
 	defer cancel()
 
 	var buf *bytes.Buffer
@@ -125,11 +123,8 @@ func gitPipeline(ctx context.Context, dir string, args []string, envs []string,
 		}()
 	}
 
-	streamErr := cmd.StdOut().Run().Stream(w)
+	streamErr := c.StdOut().Run().Stream(w)
 	if streamErr != nil {
-		if stderr != nil {
-			_, _ = fmt.Fprint(stderr, extractStderr(streamErr))
-		}
 		return mapContextError(streamErr, ctx)
 	}
 	return nil
@@ -213,17 +208,3 @@ func isExitStatus(err error, code int) bool {
 	exitCoder, ok := err.(run.ExitCoder)
 	return ok && exitCoder.ExitCode() == code
 }
-
-// extractStderr attempts to extract the stderr portion from a sourcegraph/run
-// error. The error format is typically "exit status N: <stderr content>".
-func extractStderr(err error) string {
-	if err == nil {
-		return ""
-	}
-	msg := err.Error()
-	// sourcegraph/run error format: "exit status N: <stderr>"
-	if idx := strings.Index(msg, ": "); idx >= 0 && strings.HasPrefix(msg, "exit status") {
-		return msg[idx+2:]
-	}
-	return msg
-}

command_test.go

@@ -13,33 +13,33 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestGitRun_ContextTimeout(t *testing.T) {
+func TestExec_ContextTimeout(t *testing.T) {
 	t.Run("context already expired before start", func(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
 		defer cancel()
 		time.Sleep(time.Millisecond) // ensure deadline has passed
-		_, err := gitRun(ctx, "", []string{"version"}, nil)
+		_, err := exec(ctx, "", []string{"version"}, nil)
 		assert.Equal(t, ErrExecTimeout, err)
 	})
 
 	t.Run("context deadline fires mid-execution", func(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
 		defer cancel()
 
-		// Use gitCmd directly with a blocking stdin so the command starts
+		// Use cmd directly with a blocking stdin so the command starts
 		// successfully and blocks reading until the context deadline fires.
-		cmd, timeoutCancel := gitCmd(ctx, "", []string{"hash-object", "--stdin"}, nil)
+		c, timeoutCancel := cmd(ctx, "", []string{"hash-object", "--stdin"}, nil)
 		defer timeoutCancel()
 
-		err := cmd.Input(blockingReader{cancel: ctx.Done()}).StdOut().Run().Stream(io.Discard)
+		err := c.Input(blockingReader{cancel: ctx.Done()}).StdOut().Run().Stream(io.Discard)
 		err = mapContextError(err, ctx)
 		assert.Equal(t, ErrExecTimeout, err)
 	})
 }
 
 // blockingReader is an io.Reader that blocks until its cancel channel is
 // closed, simulating a stdin that never provides data. When cancelled it
-// returns io.EOF so that exec's stdin copy goroutine can exit cleanly,
+// returns io.EOF so that the stdin copy goroutine can exit cleanly,
 // allowing cmd.Wait() to return.
 type blockingReader struct {
 	cancel <-chan struct{}
@@ -50,7 +50,7 @@ func (r blockingReader) Read(p []byte) (int, error) {
 	return 0, io.EOF
 }
 
-func TestGitCmd_ContextCancellation(t *testing.T) {
+func TestCmd_ContextCancellation(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 
 	// Cancel in the background after a short delay so the command is already
@@ -62,58 +62,22 @@ func TestGitCmd_ContextCancellation(t *testing.T) {
 		close(done)
 	}()
 
-	cmd, timeoutCancel := gitCmd(ctx, "", []string{"hash-object", "--stdin"}, nil)
+	c, timeoutCancel := cmd(ctx, "", []string{"hash-object", "--stdin"}, nil)
 	defer timeoutCancel()
 
-	err := cmd.Input(blockingReader{cancel: done}).StdOut().Run().Stream(io.Discard)
+	err := c.Input(blockingReader{cancel: done}).StdOut().Run().Stream(io.Discard)
 	err = mapContextError(err, ctx)
 	assert.ErrorIs(t, err, context.Canceled)
 	// Must NOT be ErrExecTimeout — cancellation is distinct from deadline.
 	assert.NotEqual(t, ErrExecTimeout, err)
 }
 
-func TestGitRun_DefaultTimeoutApplied(t *testing.T) {
+func TestExec_DefaultTimeoutApplied(t *testing.T) {
 	// A plain context.Background() has no deadline. The command should still
 	// succeed because DefaultTimeout (1 min) is applied automatically and
 	// "git version" completes well within that.
 	ctx := context.Background()
-	stdout, err := gitRun(ctx, "", []string{"version"}, nil)
+	stdout, err := exec(ctx, "", []string{"version"}, nil)
 	assert.NoError(t, err)
 	assert.Contains(t, string(stdout), "git version")
 }
-
-func TestExtractStderr(t *testing.T) {
-	tests := []struct {
-		name string
-		err  error
-		want string
-	}{
-		{
-			name: "nil error",
-			err:  nil,
-			want: "",
-		},
-		{
-			name: "exit status with stderr",
-			err:  &exitStatusError{msg: "exit status 1: fatal: not a git repository"},
-			want: "fatal: not a git repository",
-		},
-		{
-			name: "other error",
-			err:  io.EOF,
-			want: "EOF",
-		},
-	}
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			assert.Equal(t, test.want, extractStderr(test.err))
-		})
-	}
-}
-
-// exitStatusError is a simple error type for testing extractStderr.
-type exitStatusError struct {
-	msg string
-}
-
-func (e *exitStatusError) Error() string { return e.msg }

commit.go

@@ -154,7 +154,7 @@ func (c *Commit) isImageFile(ctx context.Context, blob *Blob, err error) (bool,
 		N: int64(buf.Cap()),
 	}
 
-	err = blob.Pipeline(ctx, stdout, io.Discard)
+	err = blob.Pipeline(ctx, stdout)
 	if err != nil {
 		return false, err
 	}

commit_archive.go

@@ -19,14 +19,15 @@ const (
 	ArchiveTarGz ArchiveFormat = "tar.gz"
 )
 
-// CreateArchive creates given format of archive to the destination.
-func (c *Commit) CreateArchive(ctx context.Context, format ArchiveFormat, dst string) error {
+// Archive creates given format of archive to the destination.
+func (c *Commit) Archive(ctx context.Context, format ArchiveFormat, dst string) error {
 	prefix := filepath.Base(strings.TrimSuffix(c.repo.path, ".git")) + "/"
-	_, err := gitRun(ctx, c.repo.path, []string{
+	_, err := exec(ctx, c.repo.path, []string{
 		"archive",
 		"--prefix=" + prefix,
 		"--format=" + string(format),
 		"-o", dst,
+		"--end-of-options",
 		c.ID.String(),
 	}, nil)
 	return err

commit_archive_test.go

@@ -19,7 +19,7 @@ func tempPath() string {
 	return filepath.Join(os.TempDir(), strconv.Itoa(int(time.Now().UnixNano())))
 }
 
-func TestCommit_CreateArchive(t *testing.T) {
+func TestCommit_Archive(t *testing.T) {
 	ctx := context.Background()
 	for _, format := range []ArchiveFormat{
 		ArchiveZip,
@@ -36,7 +36,7 @@ func TestCommit_CreateArchive(t *testing.T) {
 				_ = os.Remove(dst)
 			}()
 
-			assert.Nil(t, c.CreateArchive(ctx, format, dst))
+			assert.Nil(t, c.Archive(ctx, format, dst))
 		})
 	}
 }

git.go

@@ -58,7 +58,7 @@ func BinVersion(ctx context.Context) (string, error) {
 		return gitVersion, nil
 	}
 
-	stdout, err := gitRun(ctx, "", []string{"version"}, nil)
+	stdout, err := exec(ctx, "", []string{"version"}, nil)
 	if err != nil {
 		return "", err
 	}