From 80933ab11333fe0954058c1a1d788db58b806000 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 12:18:41 -0500 Subject: [PATCH 01/14] refactor: use sourcegraph/run for command execution Replace custom os/exec-based command execution with github.com/sourcegraph/run. The new implementation delegates process lifecycle management to the library while preserving the existing public API and all behavioral contracts (timeout, cancellation, stderr handling, debug logging). Key changes in command.go: - Replace exec.CommandContext with run.Cmd, using run.Arg to quote arguments for the shell-based parser - Add newRunCmd helper to centralize run.Command construction - Simplify RunInDirWithOptions by streaming stdout via Output.Stream and extracting stderr from the error - Optimize RunInDir to stream directly to a bytes.Buffer, preserving raw bytes (important for binary-ish output like ls-tree -z) --- command.go | 198 +++++++++++++++++++++++++++++++++++++++-------------- go.mod | 15 +++- go.sum | 60 ++++++++++++++++ 3 files changed, 220 insertions(+), 53 deletions(-) diff --git a/command.go b/command.go index 54e0094f..3e07dd7e 100644 --- a/command.go +++ b/command.go @@ -10,9 +10,10 @@ import ( "fmt" "io" "os" - "os/exec" "strings" "time" + + "github.com/sourcegraph/run" ) // Command contains the name, arguments and environment variables of a command. @@ -121,6 +122,45 @@ type RunInDirOptions struct { Stderr io.Writer } +// newRunCmd builds a *run.Command from the Command, applying the directory, +// environment variables and default timeout. +func (c *Command) newRunCmd(dir string) *run.Command { + ctx := c.ctx + if ctx == nil { + ctx = context.Background() + } + + // Apply default timeout if the context doesn't already have a deadline. + if _, ok := ctx.Deadline(); !ok { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, DefaultTimeout) + // We cannot defer cancel here because the command hasn't run yet. + // The caller is responsible for the context lifecycle. In practice the + // timeout context will be collected when it expires or when the parent + // is cancelled. We attach the cancel func to the context so the caller + // could cancel it, but for this fire-and-forget usage the GC handles it. + _ = cancel + } + + // run.Cmd joins all parts into a single string and then shell-parses it. + // We must quote each argument so that special characters (spaces, quotes, + // angle brackets, etc.) are preserved correctly. + parts := make([]string, 0, 1+len(c.args)) + parts = append(parts, c.name) + for _, arg := range c.args { + parts = append(parts, run.Arg(arg)) + } + + cmd := run.Cmd(ctx, parts...) + if dir != "" { + cmd = cmd.Dir(dir) + } + if len(c.envs) > 0 { + cmd = cmd.Environ(append(os.Environ(), c.envs...)) + } + return cmd +} + // RunInDirWithOptions executes the command in given directory and options. It // pipes stdin from supplied io.Reader, and pipes stdout and stderr to supplied // io.Writer. If the command's context does not have a deadline, DefaultTimeout @@ -133,10 +173,10 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err } buf := new(bytes.Buffer) - w := opt.Stdout + stdout := opt.Stdout if logOutput != nil { buf.Grow(512) - w = &limitDualWriter{ + stdout = &limitDualWriter{ W: buf, N: int64(buf.Cap()), w: opt.Stdout, @@ -151,65 +191,88 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err } }() - ctx := c.ctx - if ctx == nil { - ctx = context.Background() + cmd := c.newRunCmd(dir) + if opt.Stdin != nil { + cmd = cmd.Input(opt.Stdin) } - // Apply default timeout if the context doesn't already have a deadline. - if _, ok := ctx.Deadline(); !ok { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, DefaultTimeout) - defer cancel() - } + // Build a combined writer for the command output. We need to capture + // both stdout and stderr separately. sourcegraph/run's default Output + // is combined output, but we need to split them. + // + // We use StdOut() to get only stdout on the output stream and handle + // stderr via a pipe. + // + // However, sourcegraph/run doesn't have a direct way to wire both stdout + // and stderr to separate writers in a single Run call. Instead, we use + // the approach of streaming stdout and collecting stderr from the error. + // + // For the streaming case, we stream stdout to the writer and if there's + // an error, stderr is included in the error message by default. - cmd := exec.CommandContext(ctx, c.name, c.args...) - if len(c.envs) > 0 { - cmd.Env = append(os.Environ(), c.envs...) - } - cmd.Dir = dir - cmd.Stdin = opt.Stdin - cmd.Stdout = w - cmd.Stderr = opt.Stderr - if err = cmd.Start(); err != nil { - if ctx.Err() == context.DeadlineExceeded { - return ErrExecTimeout - } else if ctx.Err() != nil { - return ctx.Err() + if stdout != nil && opt.Stderr != nil { + // When both stdout and stderr writers are provided, we need to stream + // stdout and capture stderr. We use StdOut() to only get stdout. + output := cmd.StdOut().Run() + streamErr := output.Stream(stdout) + if streamErr != nil { + // Extract stderr from the error and write it to the stderr writer. + // sourcegraph/run includes stderr in the error by default. + _, _ = fmt.Fprint(opt.Stderr, extractStderr(streamErr)) + return mapContextError(streamErr, c.ctx) } - return err + return nil + } else if stdout != nil { + // Only stdout writer provided + output := cmd.StdOut().Run() + streamErr := output.Stream(stdout) + if streamErr != nil { + return mapContextError(streamErr, c.ctx) + } + return nil } - result := make(chan error) - go func() { - result <- cmd.Wait() - }() + // No writers - just wait for completion + waitErr := cmd.Run().Wait() + if waitErr != nil { + return mapContextError(waitErr, c.ctx) + } + return nil +} - select { - case <-ctx.Done(): - // Kill the process before waiting so cancellation is enforced promptly. - if cmd.Process != nil { - _ = cmd.Process.Kill() +// mapContextError maps context errors to the appropriate sentinel errors used +// by this package. +func mapContextError(err error, ctx context.Context) error { + if ctx == nil { + return err + } + if ctxErr := ctx.Err(); ctxErr != nil { + if ctxErr == context.DeadlineExceeded { + return ErrExecTimeout } - <-result - + return ctxErr + } + // Also check if the error itself wraps a context error + if strings.Contains(err.Error(), "signal: killed") || strings.Contains(err.Error(), context.DeadlineExceeded.Error()) { if ctx.Err() == context.DeadlineExceeded { return ErrExecTimeout } - return ctx.Err() - case err = <-result: - // Normalize errors when the context may have expired around the same time. - if err != nil { - if ctxErr := ctx.Err(); ctxErr != nil { - if ctxErr == context.DeadlineExceeded { - return ErrExecTimeout - } - return ctxErr - } - } - return err } + return err +} +// extractStderr attempts to extract the stderr portion from a sourcegraph/run +// error. The error format is typically "exit status N: ". +func extractStderr(err error) string { + if err == nil { + return "" + } + msg := err.Error() + // sourcegraph/run error format: "exit status N: " + if idx := strings.Index(msg, ": "); idx >= 0 && strings.HasPrefix(msg, "exit status") { + return msg[idx+2:] + } + return msg } // RunInDirPipeline executes the command in given directory. It pipes stdout and @@ -225,11 +288,42 @@ func (c *Command) RunInDirPipeline(stdout, stderr io.Writer, dir string) error { // RunInDir executes the command in given directory. It returns stdout and error // (combined with stderr). func (c *Command) RunInDir(dir string) ([]byte, error) { + cmd := c.newRunCmd(dir) + + logBuf := new(bytes.Buffer) + if logOutput != nil { + logBuf.Grow(512) + } + + defer func() { + if len(dir) == 0 { + log("%s\n%s", c, logBuf.Bytes()) + } else { + log("%s: %s\n%s", dir, c, logBuf.Bytes()) + } + }() + + // Use Stream to a buffer to preserve raw bytes (including NUL bytes from + // commands like "ls-tree -z"). The String/Lines methods process output + // line-by-line which corrupts binary-ish output. stdout := new(bytes.Buffer) - stderr := new(bytes.Buffer) - if err := c.RunInDirPipeline(stdout, stderr, dir); err != nil { - return nil, concatenateError(err, stderr.String()) + err := cmd.StdOut().Run().Stream(stdout) + if err != nil { + return nil, mapContextError(err, c.ctx) + } + + if logOutput != nil { + data := stdout.Bytes() + limit := len(data) + if limit > 512 { + limit = 512 + } + logBuf.Write(data[:limit]) + if len(data) > 512 { + logBuf.WriteString("... (more omitted)") + } } + return stdout.Bytes(), nil } diff --git a/go.mod b/go.mod index 0753589a..52cd9060 100644 --- a/go.mod +++ b/go.mod @@ -2,10 +2,23 @@ module github.com/gogs/git-module/v2 go 1.26.0 -require github.com/stretchr/testify v1.11.1 +require ( + github.com/sourcegraph/run v0.12.0 + github.com/stretchr/testify v1.11.1 +) require ( + bitbucket.org/creachadair/shell v0.0.7 // indirect github.com/davecgh/go-spew v1.1.1 // indirect + github.com/djherbis/buffer v1.2.0 // indirect + github.com/djherbis/nio/v3 v3.0.1 // indirect + github.com/go-logr/logr v1.2.3 // indirect + github.com/go-logr/stdr v1.2.2 // indirect + github.com/itchyny/gojq v0.12.11 // indirect + github.com/itchyny/timefmt-go v0.1.5 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + go.bobheadxi.dev/streamline v1.2.1 // indirect + go.opentelemetry.io/otel v1.11.0 // indirect + go.opentelemetry.io/otel/trace v1.11.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index c4c1710c..2957c960 100644 --- a/go.sum +++ b/go.sum @@ -1,10 +1,70 @@ +bitbucket.org/creachadair/shell v0.0.7 h1:Z96pB6DkSb7F3Y3BBnJeOZH2gazyMTWlvecSD4vDqfk= +bitbucket.org/creachadair/shell v0.0.7/go.mod h1:oqtXSSvSYr4624lnnabXHaBsYW6RD80caLi2b3hJk0U= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/djherbis/buffer v1.1.0/go.mod h1:VwN8VdFkMY0DCALdY8o00d3IZ6Amz/UNVMWcSaJT44o= +github.com/djherbis/buffer v1.2.0 h1:PH5Dd2ss0C7CRRhQCZ2u7MssF+No9ide8Ye71nPHcrQ= +github.com/djherbis/buffer v1.2.0/go.mod h1:fjnebbZjCUpPinBRD+TDwXSOeNQ7fPQWLfGQqiAiUyE= +github.com/djherbis/nio/v3 v3.0.1 h1:6wxhnuppteMa6RHA4L81Dq7ThkZH8SwnDzXDYy95vB4= +github.com/djherbis/nio/v3 v3.0.1/go.mod h1:Ng4h80pbZFMla1yKzm61cF0tqqilXZYrogmWgZxOcmg= +github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= +github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= +github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE= +github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps= +github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= +github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= +github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/hexops/autogold v1.3.1 h1:YgxF9OHWbEIUjhDbpnLhgVsjUDsiHDTyDfy2lrfdlzo= +github.com/hexops/autogold v1.3.1/go.mod h1:sQO+mQUCVfxOKPht+ipDSkJ2SCJ7BNJVHZexsXqWMx4= +github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= +github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= +github.com/hexops/valast v1.4.3 h1:oBoGERMJh6UZdRc6cduE1CTPK+VAdXA59Y1HFgu3sm0= +github.com/hexops/valast v1.4.3/go.mod h1:Iqx2kLj3Jn47wuXpj3wX40xn6F93QNFBHuiKBerkTGA= +github.com/itchyny/gojq v0.12.11 h1:YhLueoHhHiN4mkfM+3AyJV6EPcCxKZsOnYf+aVSwaQw= +github.com/itchyny/gojq v0.12.11/go.mod h1:o3FT8Gkbg/geT4pLI0tF3hvip5F3Y/uskjRz9OYa38g= +github.com/itchyny/timefmt-go v0.1.5 h1:G0INE2la8S6ru/ZI5JecgyzbbJNs5lG1RcBqa7Jm6GE= +github.com/itchyny/timefmt-go v0.1.5/go.mod h1:nEP7L+2YmAbT2kZ2HfSs1d8Xtw9LY8D2stDBckWakZ8= +github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= +github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= +github.com/mattn/go-isatty v0.0.16 h1:bq3VjFmv/sOjHtdEhmkEV4x1AJtvUvOJ2PFAZ5+peKQ= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/nightlyone/lockfile v1.0.0 h1:RHep2cFKK4PonZJDdEl4GmkabuhbsRMgk/k3uAmxBiA= +github.com/nightlyone/lockfile v1.0.0/go.mod h1:rywoIealpdNse2r832aiD9jRk8ErCatROs6LzC841CI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= +github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= +github.com/sourcegraph/run v0.12.0 h1:3A8w5e8HIYPfafHekvmdmmh42RHKGVhmiTZAPJclg7I= +github.com/sourcegraph/run v0.12.0/go.mod h1:PwaP936BTnAJC1cqR5rSbG5kOs/EWStTK3lqvMX5GUA= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +go.bobheadxi.dev/streamline v1.2.1 h1:IqKSA1TbeuDqCzYNAwtlh8sqf3tsQus8XgJdkCWFT8c= +go.bobheadxi.dev/streamline v1.2.1/go.mod h1:yJsVXOSBFLgAKvsnf6WmIzmB2A65nWqkR/sRNxJPa74= +go.opentelemetry.io/otel v1.11.0 h1:kfToEGMDq6TrVrJ9Vht84Y8y9enykSZzDDZglV0kIEk= +go.opentelemetry.io/otel v1.11.0/go.mod h1:H2KtuEphyMvlhZ+F7tg9GRhAOe60moNx61Ex+WmiKkk= +go.opentelemetry.io/otel/sdk v1.11.0 h1:ZnKIL9V9Ztaq+ME43IUi/eo22mNsb6a7tGfzaOWB5fo= +go.opentelemetry.io/otel/sdk v1.11.0/go.mod h1:REusa8RsyKaq0OlyangWXaw97t2VogoO4SSEeKkSTAk= +go.opentelemetry.io/otel/trace v1.11.0 h1:20U/Vj42SX+mASlXLmSGBg6jpI1jQtv682lZtTAOVFI= +go.opentelemetry.io/otel/trace v1.11.0/go.mod h1:nyYjis9jy0gytE9LXGU+/m1sHTKbRY0fX0hulNNDP1U= +golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA= +golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= +golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/tools v0.4.0 h1:7mTAgkunk3fr4GAloyyCasadO6h9zSsQZbwvcaIciV4= +golang.org/x/tools v0.4.0/go.mod h1:UE5sM2OK9E/d67R0ANs2xJizIymRP5gJU295PvKXxjQ= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +mvdan.cc/gofumpt v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM= +mvdan.cc/gofumpt v0.4.0/go.mod h1:PljLOHDeZqgS8opHRKLzp2It2VBuSdteAgqUfzMTxlQ= From 78b219b916c1b139a1dcd3501446598db92c9fb5 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 13:49:36 -0500 Subject: [PATCH 02/14] refactor: convert remaining call sites and rewrite tests Convert repo_tree.go LsTree to use gitRun with local args, removing the last NewCommand call site. Rewrite command_test.go to test the new gitRun/gitPipeline helpers directly, removing tests for the deleted Command struct. --- blob.go | 2 +- command.go | 341 +++++++++++++++------------------------------- command_test.go | 109 ++++++--------- commit_archive.go | 9 +- git.go | 2 +- repo.go | 131 ++++++++++-------- repo_blame.go | 9 +- repo_commit.go | 101 +++++++------- repo_diff.go | 59 ++++---- repo_grep.go | 24 ++-- repo_pull.go | 12 +- repo_reference.go | 39 +++--- repo_remote.go | 90 ++++++------ repo_tag.go | 43 +++--- repo_tree.go | 12 +- server.go | 36 +++-- tree_entry.go | 2 +- 17 files changed, 455 insertions(+), 566 deletions(-) diff --git a/blob.go b/blob.go index 399171b4..d23bcdea 100644 --- a/blob.go +++ b/blob.go @@ -35,5 +35,5 @@ func (b *Blob) Bytes(ctx context.Context) ([]byte, error) { // 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 NewCommand(ctx, "show", b.id.String()).RunInDirPipeline(stdout, stderr, b.parent.repo.path) + return gitPipeline(ctx, b.parent.repo.path, []string{"show", b.id.String()}, nil, stdout, stderr, nil) } diff --git a/command.go b/command.go index 3e07dd7e..930ade14 100644 --- a/command.go +++ b/command.go @@ -16,116 +16,20 @@ import ( "github.com/sourcegraph/run" ) -// Command contains the name, arguments and environment variables of a command. -type Command struct { - name string - args []string - envs []string - ctx context.Context -} - // CommandOptions contains options for running a command. type CommandOptions struct { Args []string Envs []string } -// String returns the string representation of the command. -func (c *Command) String() string { - if len(c.args) == 0 { - return c.name - } - return fmt.Sprintf("%s %s", c.name, strings.Join(c.args, " ")) -} - -// NewCommand creates and returns a new Command with given arguments for "git". -func NewCommand(ctx context.Context, args ...string) *Command { - return &Command{ - name: "git", - args: args, - ctx: ctx, - } -} - -// AddArgs appends given arguments to the command. -func (c *Command) AddArgs(args ...string) *Command { - c.args = append(c.args, args...) - return c -} - -// AddEnvs appends given environment variables to the command. -func (c *Command) AddEnvs(envs ...string) *Command { - c.envs = append(c.envs, envs...) - return c -} - -// WithContext returns a new Command with the given context. -func (c Command) WithContext(ctx context.Context) *Command { - c.ctx = ctx - return &c -} - -// AddOptions adds options to the command. -func (c *Command) AddOptions(opts ...CommandOptions) *Command { - for _, opt := range opts { - c.AddArgs(opt.Args...) - c.AddEnvs(opt.Envs...) - } - return c -} - -// AddCommitter appends given committer to the command. -func (c *Command) AddCommitter(committer *Signature) *Command { - c.AddEnvs("GIT_COMMITTER_NAME="+committer.Name, "GIT_COMMITTER_EMAIL="+committer.Email) - return c -} - // DefaultTimeout is the default timeout duration for all commands. It is // applied when the context does not already have a deadline. const DefaultTimeout = time.Minute -// A limitDualWriter writes to W but limits the amount of data written to just N -// bytes. On the other hand, it passes everything to w. -type limitDualWriter struct { - W io.Writer // underlying writer - N int64 // max bytes remaining - prompted bool - - w io.Writer -} - -func (w *limitDualWriter) Write(p []byte) (int, error) { - if w.N > 0 { - limit := int64(len(p)) - if limit > w.N { - limit = w.N - } - n, _ := w.W.Write(p[:limit]) - w.N -= int64(n) - } - - if !w.prompted && w.N <= 0 { - w.prompted = true - _, _ = w.W.Write([]byte("... (more omitted)")) - } - - return w.w.Write(p) -} - -// RunInDirOptions contains options for running a command in a directory. -type RunInDirOptions struct { - // Stdin is the input to the command. - Stdin io.Reader - // Stdout is the outputs from the command. - Stdout io.Writer - // Stderr is the error output from the command. - Stderr io.Writer -} - -// newRunCmd builds a *run.Command from the Command, applying the directory, -// environment variables and default timeout. -func (c *Command) newRunCmd(dir string) *run.Command { - ctx := c.ctx +// gitCmd 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 { if ctx == nil { ctx = context.Background() } @@ -134,20 +38,15 @@ func (c *Command) newRunCmd(dir string) *run.Command { if _, ok := ctx.Deadline(); !ok { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, DefaultTimeout) - // We cannot defer cancel here because the command hasn't run yet. - // The caller is responsible for the context lifecycle. In practice the - // timeout context will be collected when it expires or when the parent - // is cancelled. We attach the cancel func to the context so the caller - // could cancel it, but for this fire-and-forget usage the GC handles it. _ = cancel } // run.Cmd joins all parts into a single string and then shell-parses it. // We must quote each argument so that special characters (spaces, quotes, // angle brackets, etc.) are preserved correctly. - parts := make([]string, 0, 1+len(c.args)) - parts = append(parts, c.name) - for _, arg := range c.args { + parts := make([]string, 0, 1+len(args)) + parts = append(parts, "git") + for _, arg := range args { parts = append(parts, run.Arg(arg)) } @@ -155,89 +54,133 @@ func (c *Command) newRunCmd(dir string) *run.Command { if dir != "" { cmd = cmd.Dir(dir) } - if len(c.envs) > 0 { - cmd = cmd.Environ(append(os.Environ(), c.envs...)) + if len(envs) > 0 { + cmd = cmd.Environ(append(os.Environ(), envs...)) } return cmd } -// RunInDirWithOptions executes the command in given directory and options. It -// pipes stdin from supplied io.Reader, and pipes stdout and stderr to supplied -// io.Writer. 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 (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err error) { - var opt RunInDirOptions - if len(opts) > 0 { - opt = opts[0] +// gitRun 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 := gitCmd(ctx, dir, args, envs) + + logBuf := new(bytes.Buffer) + if logOutput != nil { + logBuf.Grow(512) + } + + defer func() { + logf(dir, args, logBuf.Bytes()) + }() + + // Use Stream to a buffer to preserve raw bytes (including NUL bytes from + // 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) + if err != nil { + return nil, mapContextError(err, ctx) + } + + if logOutput != nil { + data := stdout.Bytes() + limit := len(data) + if limit > 512 { + limit = 512 + } + logBuf.Write(data[:limit]) + if len(data) > 512 { + logBuf.WriteString("... (more omitted)") + } + } + + 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. stdin is optional. +func gitPipeline(ctx context.Context, dir string, args []string, envs []string, stdout, stderr io.Writer, stdin io.Reader) error { + cmd := gitCmd(ctx, dir, args, envs) + if stdin != nil { + cmd = cmd.Input(stdin) } buf := new(bytes.Buffer) - stdout := opt.Stdout + w := stdout if logOutput != nil { buf.Grow(512) - stdout = &limitDualWriter{ + w = &limitDualWriter{ W: buf, N: int64(buf.Cap()), - w: opt.Stdout, + w: stdout, } } defer func() { - if len(dir) == 0 { - log("%s\n%s", c, buf.Bytes()) - } else { - log("%s: %s\n%s", dir, c, buf.Bytes()) - } + logf(dir, args, buf.Bytes()) }() - cmd := c.newRunCmd(dir) - if opt.Stdin != nil { - cmd = cmd.Input(opt.Stdin) + streamErr := cmd.StdOut().Run().Stream(w) + if streamErr != nil { + if stderr != nil { + _, _ = fmt.Fprint(stderr, extractStderr(streamErr)) + } + return mapContextError(streamErr, ctx) + } + return nil +} + +// committerEnvs returns environment variables for setting the Git committer. +func committerEnvs(committer *Signature) []string { + return []string{ + "GIT_COMMITTER_NAME=" + committer.Name, + "GIT_COMMITTER_EMAIL=" + committer.Email, } +} - // Build a combined writer for the command output. We need to capture - // both stdout and stderr separately. sourcegraph/run's default Output - // is combined output, but we need to split them. - // - // We use StdOut() to get only stdout on the output stream and handle - // stderr via a pipe. - // - // However, sourcegraph/run doesn't have a direct way to wire both stdout - // and stderr to separate writers in a single Run call. Instead, we use - // the approach of streaming stdout and collecting stderr from the error. - // - // For the streaming case, we stream stdout to the writer and if there's - // an error, stderr is included in the error message by default. +// logf logs a git command execution with optional output. +func logf(dir string, args []string, output []byte) { + cmdStr := "git" + if len(args) > 0 { + cmdStr = "git " + strings.Join(args, " ") + } + if len(dir) == 0 { + log("%s\n%s", cmdStr, output) + } else { + log("%s: %s\n%s", dir, cmdStr, output) + } +} - if stdout != nil && opt.Stderr != nil { - // When both stdout and stderr writers are provided, we need to stream - // stdout and capture stderr. We use StdOut() to only get stdout. - output := cmd.StdOut().Run() - streamErr := output.Stream(stdout) - if streamErr != nil { - // Extract stderr from the error and write it to the stderr writer. - // sourcegraph/run includes stderr in the error by default. - _, _ = fmt.Fprint(opt.Stderr, extractStderr(streamErr)) - return mapContextError(streamErr, c.ctx) - } - return nil - } else if stdout != nil { - // Only stdout writer provided - output := cmd.StdOut().Run() - streamErr := output.Stream(stdout) - if streamErr != nil { - return mapContextError(streamErr, c.ctx) +// A limitDualWriter writes to W but limits the amount of data written to just N +// bytes. On the other hand, it passes everything to w. +type limitDualWriter struct { + W io.Writer // underlying writer + N int64 // max bytes remaining + prompted bool + + w io.Writer +} + +func (w *limitDualWriter) Write(p []byte) (int, error) { + if w.N > 0 { + limit := int64(len(p)) + if limit > w.N { + limit = w.N } - return nil + n, _ := w.W.Write(p[:limit]) + w.N -= int64(n) } - // No writers - just wait for completion - waitErr := cmd.Run().Wait() - if waitErr != nil { - return mapContextError(waitErr, c.ctx) + if !w.prompted && w.N <= 0 { + w.prompted = true + _, _ = w.W.Write([]byte("... (more omitted)")) } - return nil + + return w.w.Write(p) } // mapContextError maps context errors to the appropriate sentinel errors used @@ -252,7 +195,7 @@ func mapContextError(err error, ctx context.Context) error { } return ctxErr } - // Also check if the error itself wraps a context error + // Also check if the error itself wraps a context error. if strings.Contains(err.Error(), "signal: killed") || strings.Contains(err.Error(), context.DeadlineExceeded.Error()) { if ctx.Err() == context.DeadlineExceeded { return ErrExecTimeout @@ -274,65 +217,3 @@ func extractStderr(err error) string { } return msg } - -// RunInDirPipeline executes the command in given directory. It pipes stdout and -// stderr to supplied io.Writer. -func (c *Command) RunInDirPipeline(stdout, stderr io.Writer, dir string) error { - return c.RunInDirWithOptions(dir, RunInDirOptions{ - Stdin: nil, - Stdout: stdout, - Stderr: stderr, - }) -} - -// RunInDir executes the command in given directory. It returns stdout and error -// (combined with stderr). -func (c *Command) RunInDir(dir string) ([]byte, error) { - cmd := c.newRunCmd(dir) - - logBuf := new(bytes.Buffer) - if logOutput != nil { - logBuf.Grow(512) - } - - defer func() { - if len(dir) == 0 { - log("%s\n%s", c, logBuf.Bytes()) - } else { - log("%s: %s\n%s", dir, c, logBuf.Bytes()) - } - }() - - // Use Stream to a buffer to preserve raw bytes (including NUL bytes from - // 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) - if err != nil { - return nil, mapContextError(err, c.ctx) - } - - if logOutput != nil { - data := stdout.Bytes() - limit := len(data) - if limit > 512 { - limit = 512 - } - logBuf.Write(data[:limit]) - if len(data) > 512 { - logBuf.WriteString("... (more omitted)") - } - } - - return stdout.Bytes(), nil -} - -// Run executes the command in working directory. It returns stdout and -// error (combined with stderr). -func (c *Command) Run() ([]byte, error) { - stdout, err := c.RunInDir("") - if err != nil { - return nil, err - } - return stdout, nil -} diff --git a/command_test.go b/command_test.go index e2fb26f5..e1aa9b68 100644 --- a/command_test.go +++ b/command_test.go @@ -13,63 +13,12 @@ import ( "github.com/stretchr/testify/assert" ) -func TestCommand_String(t *testing.T) { - ctx := context.Background() - tests := []struct { - name string - args []string - expStr string - }{ - { - name: "no args", - args: nil, - expStr: "git", - }, - { - name: "has one arg", - args: []string{"version"}, - expStr: "git version", - }, - { - name: "has more args", - args: []string{"config", "--global", "http.proxy", "http://localhost:8080"}, - expStr: "git config --global http.proxy http://localhost:8080", - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - cmd := NewCommand(ctx, test.args...) - assert.Equal(t, test.expStr, cmd.String()) - }) - } -} - -func TestCommand_AddArgs(t *testing.T) { - ctx := context.Background() - cmd := NewCommand(ctx) - assert.Equal(t, []string(nil), cmd.args) - - cmd.AddArgs("push") - cmd.AddArgs("origin", "master") - assert.Equal(t, []string{"push", "origin", "master"}, cmd.args) -} - -func TestCommand_AddEnvs(t *testing.T) { - ctx := context.Background() - cmd := NewCommand(ctx) - assert.Equal(t, []string(nil), cmd.envs) - - cmd.AddEnvs("GIT_DIR=/tmp") - cmd.AddEnvs("HOME=/Users/unknwon", "GIT_EDITOR=code") - assert.Equal(t, []string{"GIT_DIR=/tmp", "HOME=/Users/unknwon", "GIT_EDITOR=code"}, cmd.envs) -} - -func TestCommand_RunWithContextTimeout(t *testing.T) { +func TestGitRun_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 := NewCommand(ctx, "version").Run() + _, err := gitRun(ctx, "", []string{"version"}, nil) assert.Equal(t, ErrExecTimeout, err) }) @@ -79,11 +28,7 @@ func TestCommand_RunWithContextTimeout(t *testing.T) { // Use a blocking reader so the command starts successfully and blocks // reading stdin until the context deadline fires. - err := NewCommand(ctx, "hash-object", "--stdin").RunInDirWithOptions("", RunInDirOptions{ - Stdin: blockingReader{cancel: ctx.Done()}, - Stdout: io.Discard, - Stderr: io.Discard, - }) + err := gitPipeline(ctx, "", []string{"hash-object", "--stdin"}, nil, io.Discard, io.Discard, blockingReader{cancel: ctx.Done()}) assert.Equal(t, ErrExecTimeout, err) }) } @@ -101,7 +46,7 @@ func (r blockingReader) Read(p []byte) (int, error) { return 0, io.EOF } -func TestCommand_RunWithContextCancellation(t *testing.T) { +func TestGitPipeline_ContextCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) // Cancel in the background after a short delay so the command is already @@ -113,22 +58,54 @@ func TestCommand_RunWithContextCancellation(t *testing.T) { close(done) }() - err := NewCommand(ctx, "hash-object", "--stdin").RunInDirWithOptions("", RunInDirOptions{ - Stdin: blockingReader{cancel: done}, - Stdout: io.Discard, - Stderr: io.Discard, - }) + err := gitPipeline(ctx, "", []string{"hash-object", "--stdin"}, nil, io.Discard, io.Discard, blockingReader{cancel: done}) assert.ErrorIs(t, err, context.Canceled) // Must NOT be ErrExecTimeout — cancellation is distinct from deadline. assert.NotEqual(t, ErrExecTimeout, err) } -func TestCommand_DefaultTimeoutApplied(t *testing.T) { +func TestGitRun_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 := NewCommand(ctx, "version").Run() + stdout, err := gitRun(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 } diff --git a/commit_archive.go b/commit_archive.go index 535f6f82..65393f9f 100644 --- a/commit_archive.go +++ b/commit_archive.go @@ -22,11 +22,12 @@ const ( // CreateArchive creates given format of archive to the destination. func (c *Commit) CreateArchive(ctx context.Context, format ArchiveFormat, dst string) error { prefix := filepath.Base(strings.TrimSuffix(c.repo.path, ".git")) + "/" - _, err := NewCommand(ctx, "archive", - "--prefix="+prefix, - "--format="+string(format), + _, err := gitRun(ctx, c.repo.path, []string{ + "archive", + "--prefix=" + prefix, + "--format=" + string(format), "-o", dst, c.ID.String(), - ).RunInDir(c.repo.path) + }, nil) return err } diff --git a/git.go b/git.go index ebbfdfd7..0b4d27bb 100644 --- a/git.go +++ b/git.go @@ -58,7 +58,7 @@ func BinVersion(ctx context.Context) (string, error) { return gitVersion, nil } - stdout, err := NewCommand(ctx, "version").Run() + stdout, err := gitRun(ctx, "", []string{"version"}, nil) if err != nil { return "", err } diff --git a/repo.go b/repo.go index 4b82c477..7cba48ae 100644 --- a/repo.go +++ b/repo.go @@ -74,12 +74,13 @@ func Init(ctx context.Context, path string, opts ...InitOptions) error { return err } - cmd := NewCommand(ctx, "init").AddOptions(opt.CommandOptions) + args := []string{"init"} + args = append(args, opt.CommandOptions.Args...) if opt.Bare { - cmd.AddArgs("--bare") + args = append(args, "--bare") } - cmd.AddArgs("--end-of-options") - _, err = cmd.RunInDir(path) + args = append(args, "--end-of-options") + _, err = gitRun(ctx, path, args, opt.CommandOptions.Envs) return err } @@ -131,25 +132,26 @@ func Clone(ctx context.Context, url, dst string, opts ...CloneOptions) error { return err } - cmd := NewCommand(ctx, "clone").AddOptions(opt.CommandOptions) + args := []string{"clone"} + args = append(args, opt.CommandOptions.Args...) if opt.Mirror { - cmd.AddArgs("--mirror") + args = append(args, "--mirror") } if opt.Bare { - cmd.AddArgs("--bare") + args = append(args, "--bare") } if opt.Quiet { - cmd.AddArgs("--quiet") + args = append(args, "--quiet") } if !opt.Bare && opt.Branch != "" { - cmd.AddArgs("-b", opt.Branch) + args = append(args, "-b", opt.Branch) } if opt.Depth > 0 { - cmd.AddArgs("--depth", strconv.FormatUint(opt.Depth, 10)) + args = append(args, "--depth", strconv.FormatUint(opt.Depth, 10)) } - cmd.AddArgs("--end-of-options") - _, err = cmd.AddArgs(url, dst).Run() + args = append(args, "--end-of-options", url, dst) + _, err = gitRun(ctx, "", args, opt.CommandOptions.Envs) return err } @@ -170,12 +172,13 @@ func (r *Repository) Fetch(ctx context.Context, opts ...FetchOptions) error { opt = opts[0] } - cmd := NewCommand(ctx, "fetch").AddOptions(opt.CommandOptions) + args := []string{"fetch"} + args = append(args, opt.CommandOptions.Args...) if opt.Prune { - cmd.AddArgs("--prune") + args = append(args, "--prune") } - _, err := cmd.RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -202,21 +205,22 @@ func (r *Repository) Pull(ctx context.Context, opts ...PullOptions) error { opt = opts[0] } - cmd := NewCommand(ctx, "pull").AddOptions(opt.CommandOptions) + args := []string{"pull"} + args = append(args, opt.CommandOptions.Args...) if opt.Rebase { - cmd.AddArgs("--rebase") + args = append(args, "--rebase") } if opt.All { - cmd.AddArgs("--all") + args = append(args, "--all") } if !opt.All && opt.Remote != "" { - cmd.AddArgs(opt.Remote) + args = append(args, opt.Remote) if opt.Branch != "" { - cmd.AddArgs(opt.Branch) + args = append(args, opt.Branch) } } - _, err := cmd.RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -235,8 +239,10 @@ func (r *Repository) Push(ctx context.Context, remote, branch string, opts ...Pu opt = opts[0] } - cmd := NewCommand(ctx, "push").AddOptions(opt.CommandOptions).AddArgs("--end-of-options", remote, branch) - _, err := cmd.RunInDir(r.path) + args := []string{"push"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--end-of-options", remote, branch) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -257,16 +263,17 @@ func (r *Repository) Checkout(ctx context.Context, branch string, opts ...Checko opt = opts[0] } - cmd := NewCommand(ctx, "checkout").AddOptions(opt.CommandOptions) + args := []string{"checkout"} + args = append(args, opt.CommandOptions.Args...) if opt.BaseBranch != "" { - cmd.AddArgs("-b") + args = append(args, "-b") } - cmd.AddArgs(branch) + args = append(args, branch) if opt.BaseBranch != "" { - cmd.AddArgs(opt.BaseBranch) + args = append(args, opt.BaseBranch) } - _, err := cmd.RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -287,12 +294,14 @@ func (r *Repository) Reset(ctx context.Context, rev string, opts ...ResetOptions opt = opts[0] } - cmd := NewCommand(ctx, "reset") + args := []string{"reset"} if opt.Hard { - cmd.AddArgs("--hard") + args = append(args, "--hard") } + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--end-of-options", rev) - _, err := cmd.AddOptions(opt.CommandOptions).AddArgs("--end-of-options", rev).RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -313,7 +322,10 @@ func (r *Repository) Move(ctx context.Context, src, dst string, opts ...MoveOpti opt = opts[0] } - _, err := NewCommand(ctx, "mv").AddOptions(opt.CommandOptions).AddArgs("--end-of-options", src, dst).RunInDir(r.path) + args := []string{"mv"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--end-of-options", src, dst) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -336,15 +348,16 @@ func (r *Repository) Add(ctx context.Context, opts ...AddOptions) error { opt = opts[0] } - cmd := NewCommand(ctx, "add").AddOptions(opt.CommandOptions) + args := []string{"add"} + args = append(args, opt.CommandOptions.Args...) if opt.All { - cmd.AddArgs("--all") + args = append(args, "--all") } if len(opt.Pathspecs) > 0 { - cmd.AddArgs("--") - cmd.AddArgs(opt.Pathspecs...) + args = append(args, "--") + args = append(args, opt.Pathspecs...) } - _, err := cmd.RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -366,17 +379,19 @@ func (r *Repository) Commit(ctx context.Context, committer *Signature, message s opt = opts[0] } - cmd := NewCommand(ctx, "commit") - cmd.AddCommitter(committer) + envs := committerEnvs(committer) + envs = append(envs, opt.CommandOptions.Envs...) if opt.Author == nil { opt.Author = committer } - cmd = cmd.AddArgs(fmt.Sprintf("--author='%s <%s>'", opt.Author.Name, opt.Author.Email)). - AddArgs("-m", message). - AddOptions(opt.CommandOptions) - _, err := cmd.RunInDir(r.path) + args := []string{"commit"} + args = append(args, fmt.Sprintf("--author='%s <%s>'", opt.Author.Name, opt.Author.Email)) + args = append(args, "-m", message) + args = append(args, opt.CommandOptions.Args...) + + _, err := gitRun(ctx, r.path, args, envs) // No stderr but exit status 1 means nothing to commit. if err != nil && err.Error() == "exit status 1" { return nil @@ -429,11 +444,12 @@ func (r *Repository) ShowNameStatus(ctx context.Context, rev string, opts ...Sho done <- struct{}{} }() + args := []string{"show", "--name-status", "--pretty=format:''"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--end-of-options", rev) + stderr := new(bytes.Buffer) - cmd := NewCommand(ctx, "show", "--name-status", "--pretty=format:''"). - AddOptions(opt.CommandOptions). - AddArgs("--end-of-options", rev) - err := cmd.RunInDirPipeline(w, stderr, r.path) + err := gitPipeline(ctx, r.path, args, opt.CommandOptions.Envs, w, stderr, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, concatenateError(err, stderr.String()) @@ -459,10 +475,11 @@ func (r *Repository) RevParse(ctx context.Context, rev string, opts ...RevParseO opt = opts[0] } - commitID, err := NewCommand(ctx, "rev-parse"). - AddOptions(opt.CommandOptions). - AddArgs(rev). - RunInDir(r.path) + args := []string{"rev-parse"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, rev) + + commitID, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { if strings.Contains(err.Error(), "exit status 128") { return "", ErrRevisionNotExist @@ -499,9 +516,10 @@ func (r *Repository) CountObjects(ctx context.Context, opts ...CountObjectsOptio opt = opts[0] } - stdout, err := NewCommand(ctx, "count-objects", "-v"). - AddOptions(opt.CommandOptions). - RunInDir(r.path) + args := []string{"count-objects", "-v"} + args = append(args, opt.CommandOptions.Args...) + + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -552,7 +570,8 @@ func (r *Repository) Fsck(ctx context.Context, opts ...FsckOptions) error { opt = opts[0] } - cmd := NewCommand(ctx, "fsck").AddOptions(opt.CommandOptions) - _, err := cmd.RunInDir(r.path) + args := []string{"fsck"} + args = append(args, opt.CommandOptions.Args...) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } diff --git a/repo_blame.go b/repo_blame.go index d2ffc062..9c3d1719 100644 --- a/repo_blame.go +++ b/repo_blame.go @@ -24,10 +24,11 @@ func (r *Repository) Blame(ctx context.Context, rev, file string, opts ...BlameO opt = opts[0] } - stdout, err := NewCommand(ctx, "blame"). - AddOptions(opt.CommandOptions). - AddArgs("-l", "-s", rev, "--", file). - RunInDir(r.path) + args := []string{"blame"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "-l", "-s", rev, "--", file) + + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } diff --git a/repo_commit.go b/repo_commit.go index b2425200..3e29a06d 100644 --- a/repo_commit.go +++ b/repo_commit.go @@ -94,10 +94,11 @@ func (r *Repository) CatFileCommit(ctx context.Context, rev string, opts ...CatF return nil, err } - stdout, err := NewCommand(ctx, "cat-file"). - AddOptions(opt.CommandOptions). - AddArgs("commit", commitID). - RunInDir(r.path) + args := []string{"cat-file"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "commit", commitID) + + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -128,10 +129,11 @@ func (r *Repository) CatFileType(ctx context.Context, rev string, opts ...CatFil opt = opts[0] } - typ, err := NewCommand(ctx, "cat-file"). - AddOptions(opt.CommandOptions). - AddArgs("-t", rev). - RunInDir(r.path) + args := []string{"cat-file"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "-t", rev) + + typ, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return "", err } @@ -191,30 +193,30 @@ func (r *Repository) Log(ctx context.Context, rev string, opts ...LogOptions) ([ opt = opts[0] } - cmd := NewCommand(ctx, "log"). - AddOptions(opt.CommandOptions). - AddArgs("--pretty=" + LogFormatHashOnly) + args := []string{"log"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--pretty="+LogFormatHashOnly) if opt.MaxCount > 0 { - cmd.AddArgs("--max-count=" + strconv.Itoa(opt.MaxCount)) + args = append(args, "--max-count="+strconv.Itoa(opt.MaxCount)) } if opt.Skip > 0 { - cmd.AddArgs("--skip=" + strconv.Itoa(opt.Skip)) + args = append(args, "--skip="+strconv.Itoa(opt.Skip)) } if !opt.Since.IsZero() { - cmd.AddArgs("--since=" + opt.Since.Format(time.RFC3339)) + args = append(args, "--since="+opt.Since.Format(time.RFC3339)) } if opt.GrepPattern != "" { - cmd.AddArgs("--grep=" + opt.GrepPattern) + args = append(args, "--grep="+opt.GrepPattern) } if opt.RegexpIgnoreCase { - cmd.AddArgs("--regexp-ignore-case") + args = append(args, "--regexp-ignore-case") } - cmd.AddArgs("--end-of-options", rev, "--") + args = append(args, "--end-of-options", rev, "--") if opt.Path != "" { - cmd.AddArgs(escapePath(opt.Path)) + args = append(args, escapePath(opt.Path)) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -356,21 +358,20 @@ func (r *Repository) DiffNameOnly(ctx context.Context, base, head string, opts . opt = opts[0] } - cmd := NewCommand(ctx, "diff"). - AddOptions(opt.CommandOptions). - AddArgs("--name-only") - cmd.AddArgs("--end-of-options") + args := []string{"diff"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--name-only", "--end-of-options") if opt.NeedsMergeBase { - cmd.AddArgs(base + "..." + head) + args = append(args, base+"..."+head) } else { - cmd.AddArgs(base, head) + args = append(args, base, head) } - cmd.AddArgs("--") + args = append(args, "--") if opt.Path != "" { - cmd.AddArgs(escapePath(opt.Path)) + args = append(args, escapePath(opt.Path)) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -409,19 +410,16 @@ func (r *Repository) RevListCount(ctx context.Context, refspecs []string, opts . return 0, errors.New("must have at least one refspec") } - cmd := NewCommand(ctx, "rev-list"). - AddOptions(opt.CommandOptions). - AddArgs( - "--count", - "--end-of-options", - ) - cmd.AddArgs(refspecs...) - cmd.AddArgs("--") + args := []string{"rev-list"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--count", "--end-of-options") + args = append(args, refspecs...) + args = append(args, "--") if opt.Path != "" { - cmd.AddArgs(escapePath(opt.Path)) + args = append(args, escapePath(opt.Path)) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return 0, err } @@ -451,15 +449,16 @@ func (r *Repository) RevList(ctx context.Context, refspecs []string, opts ...Rev return nil, errors.New("must have at least one refspec") } - cmd := NewCommand(ctx, "rev-list").AddOptions(opt.CommandOptions) - cmd.AddArgs("--end-of-options") - cmd.AddArgs(refspecs...) - cmd.AddArgs("--") + args := []string{"rev-list"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--end-of-options") + args = append(args, refspecs...) + args = append(args, "--") if opt.Path != "" { - cmd.AddArgs(escapePath(opt.Path)) + args = append(args, escapePath(opt.Path)) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -482,18 +481,14 @@ func (r *Repository) LatestCommitTime(ctx context.Context, opts ...LatestCommitT opt = opts[0] } - cmd := NewCommand(ctx, "for-each-ref"). - AddOptions(opt.CommandOptions). - AddArgs( - "--count=1", - "--sort=-committerdate", - "--format=%(committerdate:iso8601)", - ) + args := []string{"for-each-ref"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--count=1", "--sort=-committerdate", "--format=%(committerdate:iso8601)") if opt.Branch != "" { - cmd.AddArgs(RefsHeads + opt.Branch) + args = append(args, RefsHeads+opt.Branch) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return time.Time{}, err } diff --git a/repo_diff.go b/repo_diff.go index 0ea4fdf5..e94810d5 100644 --- a/repo_diff.go +++ b/repo_diff.go @@ -34,26 +34,26 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine return nil, err } - cmd := NewCommand(ctx) + var args []string if opt.Base == "" { // First commit of repository if commit.ParentsCount() == 0 { - cmd = cmd.AddArgs("show"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "--end-of-options", rev) + args = []string{"show"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "--end-of-options", rev) } else { c, err := commit.Parent(ctx, 0) if err != nil { return nil, err } - cmd = cmd.AddArgs("diff"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "-M", c.ID.String(), "--end-of-options", rev) + args = []string{"diff"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "-M", c.ID.String(), "--end-of-options", rev) } } else { - cmd = cmd.AddArgs("diff"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "-M", opt.Base, "--end-of-options", rev) + args = []string{"diff"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "-M", opt.Base, "--end-of-options", rev) } stdout, w := io.Pipe() @@ -61,7 +61,7 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine go StreamParseDiff(stdout, done, maxFiles, maxFileLines, maxLineChars) stderr := new(bytes.Buffer) - err = cmd.RunInDirPipeline(w, stderr, r.path) + err = gitPipeline(ctx, r.path, args, opt.CommandOptions.Envs, w, stderr, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, concatenateError(err, stderr.String()) @@ -100,42 +100,42 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo return err } - cmd := NewCommand(ctx) + var args []string switch diffType { case RawDiffNormal: if commit.ParentsCount() == 0 { - cmd = cmd.AddArgs("show"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "--end-of-options", rev) + args = []string{"show"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "--end-of-options", rev) } else { c, err := commit.Parent(ctx, 0) if err != nil { return err } - cmd = cmd.AddArgs("diff"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "-M", c.ID.String(), "--end-of-options", rev) + args = []string{"diff"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "-M", c.ID.String(), "--end-of-options", rev) } case RawDiffPatch: if commit.ParentsCount() == 0 { - cmd = cmd.AddArgs("format-patch"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "--no-signoff", "--no-signature", "--stdout", "--root", "--end-of-options", rev) + args = []string{"format-patch"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "--no-signoff", "--no-signature", "--stdout", "--root", "--end-of-options", rev) } else { c, err := commit.Parent(ctx, 0) if err != nil { return err } - cmd = cmd.AddArgs("format-patch"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "--no-signoff", "--no-signature", "--stdout", "--end-of-options", rev+"..."+c.ID.String()) + args = []string{"format-patch"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "--no-signoff", "--no-signature", "--stdout", "--end-of-options", rev+"..."+c.ID.String()) } default: return fmt.Errorf("invalid diffType: %s", diffType) } stderr := new(bytes.Buffer) - if err = cmd.RunInDirPipeline(w, stderr, r.path); err != nil { + if err = gitPipeline(ctx, r.path, args, opt.CommandOptions.Envs, w, stderr, nil); err != nil { return concatenateError(err, stderr.String()) } return nil @@ -155,8 +155,9 @@ func (r *Repository) DiffBinary(ctx context.Context, base, head string, opts ... opt = opts[0] } - return NewCommand(ctx, "diff"). - AddOptions(opt.CommandOptions). - AddArgs("--full-index", "--binary", base, head). - RunInDir(r.path) + args := []string{"diff"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--full-index", "--binary", base, head) + + return gitRun(ctx, r.path, args, opt.CommandOptions.Envs) } diff --git a/repo_grep.go b/repo_grep.go index dd05a64f..d6ffb812 100644 --- a/repo_grep.go +++ b/repo_grep.go @@ -78,29 +78,25 @@ func (r *Repository) Grep(ctx context.Context, pattern string, opts ...GrepOptio opt.Tree = "HEAD" } - cmd := NewCommand(ctx, "grep"). - AddOptions(opt.CommandOptions). - // Display full-name, line number and column number - AddArgs("--full-name", "--line-number", "--column") + args := []string{"grep"} + args = append(args, opt.CommandOptions.Args...) + // Display full-name, line number and column number + args = append(args, "--full-name", "--line-number", "--column") if opt.IgnoreCase { - cmd.AddArgs("--ignore-case") + args = append(args, "--ignore-case") } if opt.WordRegexp { - cmd.AddArgs("--word-regexp") + args = append(args, "--word-regexp") } if opt.ExtendedRegexp { - cmd.AddArgs("--extended-regexp") + args = append(args, "--extended-regexp") } - cmd.AddArgs( - "--end-of-options", - pattern, - opt.Tree, - ) + args = append(args, "--end-of-options", pattern, opt.Tree) if opt.Pathspec != "" { - cmd.AddArgs("--", opt.Pathspec) + args = append(args, "--", opt.Pathspec) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil } diff --git a/repo_pull.go b/repo_pull.go index 0c9e7775..8cb4b2fd 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -25,13 +25,11 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M opt = opts[0] } - stdout, err := NewCommand(ctx, "merge-base"). - AddOptions(opt.CommandOptions). - AddArgs( - "--end-of-options", - base, - head, - ).RunInDir(r.path) + args := []string{"merge-base"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--end-of-options", base, head) + + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { if strings.Contains(err.Error(), "exit status 1") { return "", ErrNoMergeBase diff --git a/repo_reference.go b/repo_reference.go index eec27159..771d0d7f 100644 --- a/repo_reference.go +++ b/repo_reference.go @@ -51,8 +51,10 @@ func (r *Repository) ShowRefVerify(ctx context.Context, ref string, opts ...Show opt = opts[0] } - cmd := NewCommand(ctx, "show-ref", "--verify", "--end-of-options", ref).AddOptions(opt.CommandOptions) - stdout, err := cmd.RunInDir(r.path) + args := []string{"show-ref", "--verify", "--end-of-options", ref} + args = append(args, opt.CommandOptions.Args...) + + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { if strings.Contains(err.Error(), "not a valid ref") { return "", ErrReferenceNotExist @@ -113,16 +115,17 @@ func (r *Repository) SymbolicRef(ctx context.Context, opts ...SymbolicRefOptions opt = opts[0] } - cmd := NewCommand(ctx, "symbolic-ref").AddOptions(opt.CommandOptions) + args := []string{"symbolic-ref"} + args = append(args, opt.CommandOptions.Args...) if opt.Name == "" { opt.Name = "HEAD" } - cmd.AddArgs("--end-of-options", opt.Name) + args = append(args, "--end-of-options", opt.Name) if opt.Ref != "" { - cmd.AddArgs(opt.Ref) + args = append(args, opt.Ref) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return "", err } @@ -150,19 +153,20 @@ func (r *Repository) ShowRef(ctx context.Context, opts ...ShowRefOptions) ([]*Re opt = opts[0] } - cmd := NewCommand(ctx, "show-ref").AddOptions(opt.CommandOptions) + args := []string{"show-ref"} + args = append(args, opt.CommandOptions.Args...) if opt.Heads { - cmd.AddArgs("--heads") + args = append(args, "--heads") } if opt.Tags { - cmd.AddArgs("--tags") + args = append(args, "--tags") } - cmd.AddArgs("--") + args = append(args, "--") if len(opt.Patterns) > 0 { - cmd.AddArgs(opt.Patterns...) + args = append(args, opt.Patterns...) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -213,12 +217,15 @@ func (r *Repository) DeleteBranch(ctx context.Context, name string, opts ...Dele opt = opts[0] } - cmd := NewCommand(ctx, "branch").AddOptions(opt.CommandOptions) + args := []string{"branch"} + args = append(args, opt.CommandOptions.Args...) if opt.Force { - cmd.AddArgs("-D") + args = append(args, "-D") } else { - cmd.AddArgs("-d") + args = append(args, "-d") } - _, err := cmd.AddArgs("--end-of-options", name).RunInDir(r.path) + args = append(args, "--end-of-options", name) + + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } diff --git a/repo_remote.go b/repo_remote.go index 3f13755d..877b39b4 100644 --- a/repo_remote.go +++ b/repo_remote.go @@ -34,22 +34,23 @@ func LsRemote(ctx context.Context, url string, opts ...LsRemoteOptions) ([]*Refe opt = opts[0] } - cmd := NewCommand(ctx, "ls-remote", "--quiet").AddOptions(opt.CommandOptions) + args := []string{"ls-remote", "--quiet"} + args = append(args, opt.CommandOptions.Args...) if opt.Heads { - cmd.AddArgs("--heads") + args = append(args, "--heads") } if opt.Tags { - cmd.AddArgs("--tags") + args = append(args, "--tags") } if opt.Refs { - cmd.AddArgs("--refs") + args = append(args, "--refs") } - cmd.AddArgs("--end-of-options", url) + args = append(args, "--end-of-options", url) if len(opt.Patterns) > 0 { - cmd.AddArgs(opt.Patterns...) + args = append(args, opt.Patterns...) } - stdout, err := cmd.Run() + stdout, err := gitRun(ctx, "", args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -99,15 +100,17 @@ func (r *Repository) RemoteAdd(ctx context.Context, name, url string, opts ...Re opt = opts[0] } - cmd := NewCommand(ctx, "remote", "add").AddOptions(opt.CommandOptions) + args := []string{"remote", "add"} + args = append(args, opt.CommandOptions.Args...) if opt.Fetch { - cmd.AddArgs("-f") + args = append(args, "-f") } if opt.MirrorFetch { - cmd.AddArgs("--mirror=fetch") + args = append(args, "--mirror=fetch") } + args = append(args, "--end-of-options", name, url) - _, err := cmd.AddArgs("--end-of-options", name, url).RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } @@ -127,10 +130,11 @@ func (r *Repository) RemoteRemove(ctx context.Context, name string, opts ...Remo opt = opts[0] } - _, err := NewCommand(ctx, "remote", "remove"). - AddOptions(opt.CommandOptions). - AddArgs("--end-of-options", name). - RunInDir(r.path) + args := []string{"remote", "remove"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--end-of-options", name) + + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { // the error status may differ from git clients if strings.Contains(err.Error(), "error: No such remote") || @@ -157,9 +161,10 @@ func (r *Repository) Remotes(ctx context.Context, opts ...RemotesOptions) ([]str opt = opts[0] } - stdout, err := NewCommand(ctx, "remote"). - AddOptions(opt.CommandOptions). - RunInDir(r.path) + args := []string{"remote"} + args = append(args, opt.CommandOptions.Args...) + + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -188,15 +193,17 @@ func (r *Repository) RemoteGetURL(ctx context.Context, name string, opts ...Remo opt = opts[0] } - cmd := NewCommand(ctx, "remote", "get-url").AddOptions(opt.CommandOptions) + args := []string{"remote", "get-url"} + args = append(args, opt.CommandOptions.Args...) if opt.Push { - cmd.AddArgs("--push") + args = append(args, "--push") } if opt.All { - cmd.AddArgs("--all") + args = append(args, "--all") } + args = append(args, "--end-of-options", name) - stdout, err := cmd.AddArgs("--end-of-options", name).RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -224,18 +231,17 @@ func (r *Repository) RemoteSetURL(ctx context.Context, name, newurl string, opts opt = opts[0] } - cmd := NewCommand(ctx, "remote", "set-url").AddOptions(opt.CommandOptions) + args := []string{"remote", "set-url"} + args = append(args, opt.CommandOptions.Args...) if opt.Push { - cmd.AddArgs("--push") + args = append(args, "--push") } - - cmd.AddArgs("--end-of-options", name, newurl) - + args = append(args, "--end-of-options", name, newurl) if opt.Regex != "" { - cmd.AddArgs(opt.Regex) + args = append(args, opt.Regex) } - _, err := cmd.RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { if strings.Contains(err.Error(), "No such URL found") { return ErrURLNotExist @@ -266,16 +272,15 @@ func (r *Repository) RemoteSetURLAdd(ctx context.Context, name, newurl string, o opt = opts[0] } - cmd := NewCommand(ctx, "remote", "set-url"). - AddOptions(opt.CommandOptions). - AddArgs("--add") + args := []string{"remote", "set-url"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--add") if opt.Push { - cmd.AddArgs("--push") + args = append(args, "--push") } + args = append(args, "--end-of-options", name, newurl) - cmd.AddArgs("--end-of-options", name, newurl) - - _, err := cmd.RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") { return ErrNotDeleteNonPushURLs } @@ -301,16 +306,15 @@ func (r *Repository) RemoteSetURLDelete(ctx context.Context, name, regex string, opt = opts[0] } - cmd := NewCommand(ctx, "remote", "set-url"). - AddOptions(opt.CommandOptions). - AddArgs("--delete") + args := []string{"remote", "set-url"} + args = append(args, opt.CommandOptions.Args...) + args = append(args, "--delete") if opt.Push { - cmd.AddArgs("--push") + args = append(args, "--push") } + args = append(args, "--end-of-options", name, regex) - cmd.AddArgs("--end-of-options", name, regex) - - _, err := cmd.RunInDir(r.path) + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") { return ErrNotDeleteNonPushURLs } diff --git a/repo_tag.go b/repo_tag.go index df3b883a..e76cbc95 100644 --- a/repo_tag.go +++ b/repo_tag.go @@ -76,7 +76,7 @@ func (r *Repository) getTag(ctx context.Context, id *SHA1) (*Tag, error) { } case ObjectTag: // Tag is an annotation - data, err := NewCommand(ctx, "cat-file", "-p", id.String()).RunInDir(r.path) + data, err := gitRun(ctx, r.path, []string{"cat-file", "-p", id.String()}, nil) if err != nil { return nil, err } @@ -155,19 +155,18 @@ func (r *Repository) Tags(ctx context.Context, opts ...TagsOptions) ([]string, e opt = opts[0] } - cmd := NewCommand(ctx, "tag", "--list").AddOptions(opt.CommandOptions) - + args := []string{"tag", "--list"} + args = append(args, opt.CommandOptions.Args...) if opt.SortKey != "" { - cmd.AddArgs("--sort=" + opt.SortKey) + args = append(args, "--sort="+opt.SortKey) } else { - cmd.AddArgs("--sort=-creatordate") + args = append(args, "--sort=-creatordate") } - if opt.Pattern != "" { - cmd.AddArgs(opt.Pattern) + args = append(args, opt.Pattern) } - stdout, err := cmd.RunInDir(r.path) + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } @@ -199,21 +198,24 @@ func (r *Repository) CreateTag(ctx context.Context, name, rev string, opts ...Cr opt = opts[0] } - cmd := NewCommand(ctx, "tag").AddOptions(opt.CommandOptions) + args := []string{"tag"} + args = append(args, opt.CommandOptions.Args...) + + var envs []string if opt.Annotated { - cmd.AddArgs("-a", name) - cmd.AddArgs("--message", opt.Message) + args = append(args, "-a", name) + args = append(args, "--message", opt.Message) if opt.Author != nil { - cmd.AddCommitter(opt.Author) + envs = committerEnvs(opt.Author) } - cmd.AddArgs("--end-of-options") + args = append(args, "--end-of-options") } else { - cmd.AddArgs("--end-of-options", name) + args = append(args, "--end-of-options", name) } + args = append(args, rev) - cmd.AddArgs(rev) - - _, err := cmd.RunInDir(r.path) + envs = append(envs, opt.CommandOptions.Envs...) + _, err := gitRun(ctx, r.path, args, envs) return err } @@ -232,8 +234,9 @@ func (r *Repository) DeleteTag(ctx context.Context, name string, opts ...DeleteT opt = opts[0] } - _, err := NewCommand(ctx, "tag", "--delete", "--end-of-options", name). - AddOptions(opt.CommandOptions). - RunInDir(r.path) + args := []string{"tag", "--delete", "--end-of-options", name} + args = append(args, opt.CommandOptions.Args...) + + _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) return err } diff --git a/repo_tree.go b/repo_tree.go index 8fb2e663..3675c9ff 100644 --- a/repo_tree.go +++ b/repo_tree.go @@ -131,14 +131,14 @@ func (r *Repository) LsTree(ctx context.Context, treeID string, opts ...LsTreeOp repo: r, } - cmd := NewCommand(ctx, "ls-tree") + args := []string{"ls-tree"} + args = append(args, opt.CommandOptions.Args...) if opt.Verbatim { - cmd.AddArgs("-z") + args = append(args, "-z") } - stdout, err := cmd. - AddOptions(opt.CommandOptions). - AddArgs(treeID). - RunInDir(r.path) + args = append(args, treeID) + + stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) if err != nil { return nil, err } diff --git a/server.go b/server.go index 1b4d2356..e2776933 100755 --- a/server.go +++ b/server.go @@ -27,11 +27,13 @@ func UpdateServerInfo(ctx context.Context, path string, opts ...UpdateServerInfo if len(opts) > 0 { opt = opts[0] } - cmd := NewCommand(ctx, "update-server-info").AddOptions(opt.CommandOptions) + + args := []string{"update-server-info"} + args = append(args, opt.CommandOptions.Args...) if opt.Force { - cmd.AddArgs("--force") + args = append(args, "--force") } - _, err := cmd.RunInDir(path) + _, err := gitRun(ctx, path, args, opt.CommandOptions.Envs) return err } @@ -54,15 +56,17 @@ func ReceivePack(ctx context.Context, path string, opts ...ReceivePackOptions) ( if len(opts) > 0 { opt = opts[0] } - cmd := NewCommand(ctx, "receive-pack").AddOptions(opt.CommandOptions) + + args := []string{"receive-pack"} + args = append(args, opt.CommandOptions.Args...) if opt.Quiet { - cmd.AddArgs("--quiet") + args = append(args, "--quiet") } if opt.HTTPBackendInfoRefs { - cmd.AddArgs("--http-backend-info-refs") + args = append(args, "--http-backend-info-refs") } - cmd.AddArgs(".") - return cmd.RunInDir(path) + args = append(args, ".") + return gitRun(ctx, path, args, opt.CommandOptions.Envs) } // UploadPackOptions contains optional arguments for sending the packfile to the @@ -91,19 +95,21 @@ func UploadPack(ctx context.Context, path string, opts ...UploadPackOptions) ([] if len(opts) > 0 { opt = opts[0] } - cmd := NewCommand(ctx, "upload-pack").AddOptions(opt.CommandOptions) + + args := []string{"upload-pack"} + args = append(args, opt.CommandOptions.Args...) if opt.StatelessRPC { - cmd.AddArgs("--stateless-rpc") + args = append(args, "--stateless-rpc") } if opt.Strict { - cmd.AddArgs("--strict") + args = append(args, "--strict") } if opt.InactivityTimeout > 0 { - cmd.AddArgs("--timeout", opt.InactivityTimeout.String()) + args = append(args, "--timeout", opt.InactivityTimeout.String()) } if opt.HTTPBackendInfoRefs { - cmd.AddArgs("--http-backend-info-refs") + args = append(args, "--http-backend-info-refs") } - cmd.AddArgs(".") - return cmd.RunInDir(path) + args = append(args, ".") + return gitRun(ctx, path, args, opt.CommandOptions.Envs) } diff --git a/tree_entry.go b/tree_entry.go index 30f99641..f3d68e1f 100644 --- a/tree_entry.go +++ b/tree_entry.go @@ -98,7 +98,7 @@ func (e *TreeEntry) Size(ctx context.Context) int64 { return e.size } - stdout, err := NewCommand(ctx, "cat-file", "-s", e.id.String()).RunInDir(e.parent.repo.path) + stdout, err := gitRun(ctx, e.parent.repo.path, []string{"cat-file", "-s", e.id.String()}, nil) if err != nil { return 0 } From 3edb1de804ac75ef5c9280b0386b8bed8ef76837 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 13:52:01 -0500 Subject: [PATCH 03/14] style: simplify embedded field selectors to fix QF1008 lint Use opt.Args/opt.Envs instead of opt.CommandOptions.Args/opt.CommandOptions.Envs since CommandOptions is an embedded field. --- repo.go | 56 +++++++++++++++++++++++------------------------ repo_blame.go | 4 ++-- repo_commit.go | 28 ++++++++++++------------ repo_diff.go | 22 +++++++++---------- repo_grep.go | 4 ++-- repo_pull.go | 4 ++-- repo_reference.go | 16 +++++++------- repo_remote.go | 32 +++++++++++++-------------- repo_tag.go | 12 +++++----- repo_tree.go | 4 ++-- server.go | 12 +++++----- 11 files changed, 97 insertions(+), 97 deletions(-) diff --git a/repo.go b/repo.go index 7cba48ae..8e7410ca 100644 --- a/repo.go +++ b/repo.go @@ -75,12 +75,12 @@ func Init(ctx context.Context, path string, opts ...InitOptions) error { } args := []string{"init"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Bare { args = append(args, "--bare") } args = append(args, "--end-of-options") - _, err = gitRun(ctx, path, args, opt.CommandOptions.Envs) + _, err = gitRun(ctx, path, args, opt.Envs) return err } @@ -133,7 +133,7 @@ func Clone(ctx context.Context, url, dst string, opts ...CloneOptions) error { } args := []string{"clone"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Mirror { args = append(args, "--mirror") } @@ -151,7 +151,7 @@ func Clone(ctx context.Context, url, dst string, opts ...CloneOptions) error { } args = append(args, "--end-of-options", url, dst) - _, err = gitRun(ctx, "", args, opt.CommandOptions.Envs) + _, err = gitRun(ctx, "", args, opt.Envs) return err } @@ -173,12 +173,12 @@ func (r *Repository) Fetch(ctx context.Context, opts ...FetchOptions) error { } args := []string{"fetch"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Prune { args = append(args, "--prune") } - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -206,7 +206,7 @@ func (r *Repository) Pull(ctx context.Context, opts ...PullOptions) error { } args := []string{"pull"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Rebase { args = append(args, "--rebase") } @@ -220,7 +220,7 @@ func (r *Repository) Pull(ctx context.Context, opts ...PullOptions) error { } } - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -240,9 +240,9 @@ func (r *Repository) Push(ctx context.Context, remote, branch string, opts ...Pu } args := []string{"push"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--end-of-options", remote, branch) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -264,7 +264,7 @@ func (r *Repository) Checkout(ctx context.Context, branch string, opts ...Checko } args := []string{"checkout"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.BaseBranch != "" { args = append(args, "-b") } @@ -273,7 +273,7 @@ func (r *Repository) Checkout(ctx context.Context, branch string, opts ...Checko args = append(args, opt.BaseBranch) } - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -298,10 +298,10 @@ func (r *Repository) Reset(ctx context.Context, rev string, opts ...ResetOptions if opt.Hard { args = append(args, "--hard") } - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--end-of-options", rev) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -323,9 +323,9 @@ func (r *Repository) Move(ctx context.Context, src, dst string, opts ...MoveOpti } args := []string{"mv"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--end-of-options", src, dst) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -349,7 +349,7 @@ func (r *Repository) Add(ctx context.Context, opts ...AddOptions) error { } args := []string{"add"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.All { args = append(args, "--all") } @@ -357,7 +357,7 @@ func (r *Repository) Add(ctx context.Context, opts ...AddOptions) error { args = append(args, "--") args = append(args, opt.Pathspecs...) } - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -380,7 +380,7 @@ func (r *Repository) Commit(ctx context.Context, committer *Signature, message s } envs := committerEnvs(committer) - envs = append(envs, opt.CommandOptions.Envs...) + envs = append(envs, opt.Envs...) if opt.Author == nil { opt.Author = committer @@ -389,7 +389,7 @@ func (r *Repository) Commit(ctx context.Context, committer *Signature, message s args := []string{"commit"} args = append(args, fmt.Sprintf("--author='%s <%s>'", opt.Author.Name, opt.Author.Email)) args = append(args, "-m", message) - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) _, err := gitRun(ctx, r.path, args, envs) // No stderr but exit status 1 means nothing to commit. @@ -445,11 +445,11 @@ func (r *Repository) ShowNameStatus(ctx context.Context, rev string, opts ...Sho }() args := []string{"show", "--name-status", "--pretty=format:''"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--end-of-options", rev) stderr := new(bytes.Buffer) - err := gitPipeline(ctx, r.path, args, opt.CommandOptions.Envs, w, stderr, nil) + err := gitPipeline(ctx, r.path, args, opt.Envs, w, stderr, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, concatenateError(err, stderr.String()) @@ -476,10 +476,10 @@ func (r *Repository) RevParse(ctx context.Context, rev string, opts ...RevParseO } args := []string{"rev-parse"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, rev) - commitID, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + commitID, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "exit status 128") { return "", ErrRevisionNotExist @@ -517,9 +517,9 @@ func (r *Repository) CountObjects(ctx context.Context, opts ...CountObjectsOptio } args := []string{"count-objects", "-v"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -571,7 +571,7 @@ func (r *Repository) Fsck(ctx context.Context, opts ...FsckOptions) error { } args := []string{"fsck"} - args = append(args, opt.CommandOptions.Args...) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + args = append(args, opt.Args...) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } diff --git a/repo_blame.go b/repo_blame.go index 9c3d1719..af480fcc 100644 --- a/repo_blame.go +++ b/repo_blame.go @@ -25,10 +25,10 @@ func (r *Repository) Blame(ctx context.Context, rev, file string, opts ...BlameO } args := []string{"blame"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "-l", "-s", rev, "--", file) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } diff --git a/repo_commit.go b/repo_commit.go index 3e29a06d..eb40b3da 100644 --- a/repo_commit.go +++ b/repo_commit.go @@ -95,10 +95,10 @@ func (r *Repository) CatFileCommit(ctx context.Context, rev string, opts ...CatF } args := []string{"cat-file"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "commit", commitID) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -130,10 +130,10 @@ func (r *Repository) CatFileType(ctx context.Context, rev string, opts ...CatFil } args := []string{"cat-file"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "-t", rev) - typ, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + typ, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return "", err } @@ -194,7 +194,7 @@ func (r *Repository) Log(ctx context.Context, rev string, opts ...LogOptions) ([ } args := []string{"log"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--pretty="+LogFormatHashOnly) if opt.MaxCount > 0 { args = append(args, "--max-count="+strconv.Itoa(opt.MaxCount)) @@ -216,7 +216,7 @@ func (r *Repository) Log(ctx context.Context, rev string, opts ...LogOptions) ([ args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -359,7 +359,7 @@ func (r *Repository) DiffNameOnly(ctx context.Context, base, head string, opts . } args := []string{"diff"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--name-only", "--end-of-options") if opt.NeedsMergeBase { args = append(args, base+"..."+head) @@ -371,7 +371,7 @@ func (r *Repository) DiffNameOnly(ctx context.Context, base, head string, opts . args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -411,7 +411,7 @@ func (r *Repository) RevListCount(ctx context.Context, refspecs []string, opts . } args := []string{"rev-list"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--count", "--end-of-options") args = append(args, refspecs...) args = append(args, "--") @@ -419,7 +419,7 @@ func (r *Repository) RevListCount(ctx context.Context, refspecs []string, opts . args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return 0, err } @@ -450,7 +450,7 @@ func (r *Repository) RevList(ctx context.Context, refspecs []string, opts ...Rev } args := []string{"rev-list"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--end-of-options") args = append(args, refspecs...) args = append(args, "--") @@ -458,7 +458,7 @@ func (r *Repository) RevList(ctx context.Context, refspecs []string, opts ...Rev args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -482,13 +482,13 @@ func (r *Repository) LatestCommitTime(ctx context.Context, opts ...LatestCommitT } args := []string{"for-each-ref"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--count=1", "--sort=-committerdate", "--format=%(committerdate:iso8601)") if opt.Branch != "" { args = append(args, RefsHeads+opt.Branch) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return time.Time{}, err } diff --git a/repo_diff.go b/repo_diff.go index e94810d5..69dcdbbd 100644 --- a/repo_diff.go +++ b/repo_diff.go @@ -39,7 +39,7 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine // First commit of repository if commit.ParentsCount() == 0 { args = []string{"show"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "--end-of-options", rev) } else { c, err := commit.Parent(ctx, 0) @@ -47,12 +47,12 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine return nil, err } args = []string{"diff"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "-M", c.ID.String(), "--end-of-options", rev) } } else { args = []string{"diff"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "-M", opt.Base, "--end-of-options", rev) } @@ -61,7 +61,7 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine go StreamParseDiff(stdout, done, maxFiles, maxFileLines, maxLineChars) stderr := new(bytes.Buffer) - err = gitPipeline(ctx, r.path, args, opt.CommandOptions.Envs, w, stderr, nil) + err = gitPipeline(ctx, r.path, args, opt.Envs, w, stderr, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, concatenateError(err, stderr.String()) @@ -105,7 +105,7 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo case RawDiffNormal: if commit.ParentsCount() == 0 { args = []string{"show"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "--end-of-options", rev) } else { c, err := commit.Parent(ctx, 0) @@ -113,13 +113,13 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo return err } args = []string{"diff"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "-M", c.ID.String(), "--end-of-options", rev) } case RawDiffPatch: if commit.ParentsCount() == 0 { args = []string{"format-patch"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "--no-signoff", "--no-signature", "--stdout", "--root", "--end-of-options", rev) } else { c, err := commit.Parent(ctx, 0) @@ -127,7 +127,7 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo return err } args = []string{"format-patch"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "--no-signoff", "--no-signature", "--stdout", "--end-of-options", rev+"..."+c.ID.String()) } default: @@ -135,7 +135,7 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo } stderr := new(bytes.Buffer) - if err = gitPipeline(ctx, r.path, args, opt.CommandOptions.Envs, w, stderr, nil); err != nil { + if err = gitPipeline(ctx, r.path, args, opt.Envs, w, stderr, nil); err != nil { return concatenateError(err, stderr.String()) } return nil @@ -156,8 +156,8 @@ func (r *Repository) DiffBinary(ctx context.Context, base, head string, opts ... } args := []string{"diff"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--full-index", "--binary", base, head) - return gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + return gitRun(ctx, r.path, args, opt.Envs) } diff --git a/repo_grep.go b/repo_grep.go index d6ffb812..176c71ce 100644 --- a/repo_grep.go +++ b/repo_grep.go @@ -79,7 +79,7 @@ func (r *Repository) Grep(ctx context.Context, pattern string, opts ...GrepOptio } args := []string{"grep"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) // Display full-name, line number and column number args = append(args, "--full-name", "--line-number", "--column") if opt.IgnoreCase { @@ -96,7 +96,7 @@ func (r *Repository) Grep(ctx context.Context, pattern string, opts ...GrepOptio args = append(args, "--", opt.Pathspec) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil } diff --git a/repo_pull.go b/repo_pull.go index 8cb4b2fd..52738e59 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -26,10 +26,10 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M } args := []string{"merge-base"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--end-of-options", base, head) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "exit status 1") { return "", ErrNoMergeBase diff --git a/repo_reference.go b/repo_reference.go index 771d0d7f..b8490b79 100644 --- a/repo_reference.go +++ b/repo_reference.go @@ -52,9 +52,9 @@ func (r *Repository) ShowRefVerify(ctx context.Context, ref string, opts ...Show } args := []string{"show-ref", "--verify", "--end-of-options", ref} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "not a valid ref") { return "", ErrReferenceNotExist @@ -116,7 +116,7 @@ func (r *Repository) SymbolicRef(ctx context.Context, opts ...SymbolicRefOptions } args := []string{"symbolic-ref"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Name == "" { opt.Name = "HEAD" } @@ -125,7 +125,7 @@ func (r *Repository) SymbolicRef(ctx context.Context, opts ...SymbolicRefOptions args = append(args, opt.Ref) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return "", err } @@ -154,7 +154,7 @@ func (r *Repository) ShowRef(ctx context.Context, opts ...ShowRefOptions) ([]*Re } args := []string{"show-ref"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Heads { args = append(args, "--heads") } @@ -166,7 +166,7 @@ func (r *Repository) ShowRef(ctx context.Context, opts ...ShowRefOptions) ([]*Re args = append(args, opt.Patterns...) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -218,7 +218,7 @@ func (r *Repository) DeleteBranch(ctx context.Context, name string, opts ...Dele } args := []string{"branch"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Force { args = append(args, "-D") } else { @@ -226,6 +226,6 @@ func (r *Repository) DeleteBranch(ctx context.Context, name string, opts ...Dele } args = append(args, "--end-of-options", name) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } diff --git a/repo_remote.go b/repo_remote.go index 877b39b4..4d6a4f02 100644 --- a/repo_remote.go +++ b/repo_remote.go @@ -35,7 +35,7 @@ func LsRemote(ctx context.Context, url string, opts ...LsRemoteOptions) ([]*Refe } args := []string{"ls-remote", "--quiet"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Heads { args = append(args, "--heads") } @@ -50,7 +50,7 @@ func LsRemote(ctx context.Context, url string, opts ...LsRemoteOptions) ([]*Refe args = append(args, opt.Patterns...) } - stdout, err := gitRun(ctx, "", args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, "", args, opt.Envs) if err != nil { return nil, err } @@ -101,7 +101,7 @@ func (r *Repository) RemoteAdd(ctx context.Context, name, url string, opts ...Re } args := []string{"remote", "add"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Fetch { args = append(args, "-f") } @@ -110,7 +110,7 @@ func (r *Repository) RemoteAdd(ctx context.Context, name, url string, opts ...Re } args = append(args, "--end-of-options", name, url) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } @@ -131,10 +131,10 @@ func (r *Repository) RemoteRemove(ctx context.Context, name string, opts ...Remo } args := []string{"remote", "remove"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--end-of-options", name) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { // the error status may differ from git clients if strings.Contains(err.Error(), "error: No such remote") || @@ -162,9 +162,9 @@ func (r *Repository) Remotes(ctx context.Context, opts ...RemotesOptions) ([]str } args := []string{"remote"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -194,7 +194,7 @@ func (r *Repository) RemoteGetURL(ctx context.Context, name string, opts ...Remo } args := []string{"remote", "get-url"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Push { args = append(args, "--push") } @@ -203,7 +203,7 @@ func (r *Repository) RemoteGetURL(ctx context.Context, name string, opts ...Remo } args = append(args, "--end-of-options", name) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -232,7 +232,7 @@ func (r *Repository) RemoteSetURL(ctx context.Context, name, newurl string, opts } args := []string{"remote", "set-url"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Push { args = append(args, "--push") } @@ -241,7 +241,7 @@ func (r *Repository) RemoteSetURL(ctx context.Context, name, newurl string, opts args = append(args, opt.Regex) } - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "No such URL found") { return ErrURLNotExist @@ -273,14 +273,14 @@ func (r *Repository) RemoteSetURLAdd(ctx context.Context, name, newurl string, o } args := []string{"remote", "set-url"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--add") if opt.Push { args = append(args, "--push") } args = append(args, "--end-of-options", name, newurl) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") { return ErrNotDeleteNonPushURLs } @@ -307,14 +307,14 @@ func (r *Repository) RemoteSetURLDelete(ctx context.Context, name, regex string, } args := []string{"remote", "set-url"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) args = append(args, "--delete") if opt.Push { args = append(args, "--push") } args = append(args, "--end-of-options", name, regex) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") { return ErrNotDeleteNonPushURLs } diff --git a/repo_tag.go b/repo_tag.go index e76cbc95..a882da7f 100644 --- a/repo_tag.go +++ b/repo_tag.go @@ -156,7 +156,7 @@ func (r *Repository) Tags(ctx context.Context, opts ...TagsOptions) ([]string, e } args := []string{"tag", "--list"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.SortKey != "" { args = append(args, "--sort="+opt.SortKey) } else { @@ -166,7 +166,7 @@ func (r *Repository) Tags(ctx context.Context, opts ...TagsOptions) ([]string, e args = append(args, opt.Pattern) } - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -199,7 +199,7 @@ func (r *Repository) CreateTag(ctx context.Context, name, rev string, opts ...Cr } args := []string{"tag"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) var envs []string if opt.Annotated { @@ -214,7 +214,7 @@ func (r *Repository) CreateTag(ctx context.Context, name, rev string, opts ...Cr } args = append(args, rev) - envs = append(envs, opt.CommandOptions.Envs...) + envs = append(envs, opt.Envs...) _, err := gitRun(ctx, r.path, args, envs) return err } @@ -235,8 +235,8 @@ func (r *Repository) DeleteTag(ctx context.Context, name string, opts ...DeleteT } args := []string{"tag", "--delete", "--end-of-options", name} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) - _, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, r.path, args, opt.Envs) return err } diff --git a/repo_tree.go b/repo_tree.go index 3675c9ff..8cd1d1f8 100644 --- a/repo_tree.go +++ b/repo_tree.go @@ -132,13 +132,13 @@ func (r *Repository) LsTree(ctx context.Context, treeID string, opts ...LsTreeOp } args := []string{"ls-tree"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Verbatim { args = append(args, "-z") } args = append(args, treeID) - stdout, err := gitRun(ctx, r.path, args, opt.CommandOptions.Envs) + stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } diff --git a/server.go b/server.go index e2776933..4eaa402f 100755 --- a/server.go +++ b/server.go @@ -29,11 +29,11 @@ func UpdateServerInfo(ctx context.Context, path string, opts ...UpdateServerInfo } args := []string{"update-server-info"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Force { args = append(args, "--force") } - _, err := gitRun(ctx, path, args, opt.CommandOptions.Envs) + _, err := gitRun(ctx, path, args, opt.Envs) return err } @@ -58,7 +58,7 @@ func ReceivePack(ctx context.Context, path string, opts ...ReceivePackOptions) ( } args := []string{"receive-pack"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.Quiet { args = append(args, "--quiet") } @@ -66,7 +66,7 @@ func ReceivePack(ctx context.Context, path string, opts ...ReceivePackOptions) ( args = append(args, "--http-backend-info-refs") } args = append(args, ".") - return gitRun(ctx, path, args, opt.CommandOptions.Envs) + return gitRun(ctx, path, args, opt.Envs) } // UploadPackOptions contains optional arguments for sending the packfile to the @@ -97,7 +97,7 @@ func UploadPack(ctx context.Context, path string, opts ...UploadPackOptions) ([] } args := []string{"upload-pack"} - args = append(args, opt.CommandOptions.Args...) + args = append(args, opt.Args...) if opt.StatelessRPC { args = append(args, "--stateless-rpc") } @@ -111,5 +111,5 @@ func UploadPack(ctx context.Context, path string, opts ...UploadPackOptions) ([] args = append(args, "--http-backend-info-refs") } args = append(args, ".") - return gitRun(ctx, path, args, opt.CommandOptions.Envs) + return gitRun(ctx, path, args, opt.Envs) } From c7ab9f201cdd90247824fc3345b2f5db140ce920 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 16:56:10 -0500 Subject: [PATCH 04/14] fix: address PR review feedback on run migration Fix timeout context cancel leakage, tighten exit-status matching, remove fragile stderr re-concatenation paths, and harden author/diff argument handling. --- blob.go | 5 ++--- command.go | 50 +++++++++++++++++++++++++++----------------------- repo.go | 11 +++++------ repo_diff.go | 13 +++++-------- repo_pull.go | 2 +- utils.go | 8 -------- 6 files changed, 40 insertions(+), 49 deletions(-) diff --git a/blob.go b/blob.go index d23bcdea..e9a33f83 100644 --- a/blob.go +++ b/blob.go @@ -19,15 +19,14 @@ type Blob struct { // can be very slow and memory consuming for huge content. func (b *Blob) Bytes(ctx context.Context) ([]byte, error) { stdout := new(bytes.Buffer) - stderr := new(bytes.Buffer) // Preallocate memory to save ~50% memory usage on big files. if size := b.Size(ctx); size > 0 && size < int64(^uint(0)>>1) { stdout.Grow(int(size)) } - if err := b.Pipeline(ctx, stdout, stderr); err != nil { - return nil, concatenateError(err, stderr.String()) + if err := b.Pipeline(ctx, stdout, nil); err != nil { + return nil, err } return stdout.Bytes(), nil } diff --git a/command.go b/command.go index 930ade14..35bcd89f 100644 --- a/command.go +++ b/command.go @@ -7,9 +7,11 @@ package git import ( "bytes" "context" + "errors" "fmt" "io" "os" + "os/exec" "strings" "time" @@ -29,16 +31,17 @@ const DefaultTimeout = time.Minute // gitCmd 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 { +func gitCmd(ctx context.Context, dir string, args []string, envs []string) (*run.Command, context.CancelFunc) { if ctx == nil { ctx = context.Background() } + cancel := func() {} // Apply default timeout if the context doesn't already have a deadline. if _, ok := ctx.Deadline(); !ok { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, DefaultTimeout) - _ = cancel + var timeoutCancel context.CancelFunc + ctx, timeoutCancel = context.WithTimeout(ctx, DefaultTimeout) + cancel = timeoutCancel } // run.Cmd joins all parts into a single string and then shell-parses it. @@ -57,7 +60,7 @@ func gitCmd(ctx context.Context, dir string, args []string, envs []string) *run. if len(envs) > 0 { cmd = cmd.Environ(append(os.Environ(), envs...)) } - return cmd + return cmd, cancel } // gitRun executes a git command in the given directory and returns stdout as @@ -65,17 +68,18 @@ func gitCmd(ctx context.Context, dir string, args []string, envs []string) *run. // 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 := gitCmd(ctx, dir, args, envs) + cmd, cancel := gitCmd(ctx, dir, args, envs) + defer cancel() - logBuf := new(bytes.Buffer) + var logBuf *bytes.Buffer if logOutput != nil { + logBuf = new(bytes.Buffer) logBuf.Grow(512) + defer func() { + logf(dir, args, logBuf.Bytes()) + }() } - defer func() { - logf(dir, args, logBuf.Bytes()) - }() - // Use Stream to a buffer to preserve raw bytes (including NUL bytes from // commands like "ls-tree -z"). The String/Lines methods process output // line-by-line which corrupts binary-ish output. @@ -104,25 +108,27 @@ func gitRun(ctx context.Context, dir string, args []string, envs []string) ([]by // to the given writer. If stderr writer is provided and the command fails, // stderr content extracted from the error is written to it. stdin is optional. func gitPipeline(ctx context.Context, dir string, args []string, envs []string, stdout, stderr io.Writer, stdin io.Reader) error { - cmd := gitCmd(ctx, dir, args, envs) + cmd, cancel := gitCmd(ctx, dir, args, envs) + defer cancel() if stdin != nil { cmd = cmd.Input(stdin) } - buf := new(bytes.Buffer) + var buf *bytes.Buffer w := stdout if logOutput != nil { + buf = new(bytes.Buffer) buf.Grow(512) w = &limitDualWriter{ W: buf, N: int64(buf.Cap()), w: stdout, } - } - defer func() { - logf(dir, args, buf.Bytes()) - }() + defer func() { + logf(dir, args, buf.Bytes()) + }() + } streamErr := cmd.StdOut().Run().Stream(w) if streamErr != nil { @@ -195,12 +201,6 @@ func mapContextError(err error, ctx context.Context) error { } return ctxErr } - // Also check if the error itself wraps a context error. - if strings.Contains(err.Error(), "signal: killed") || strings.Contains(err.Error(), context.DeadlineExceeded.Error()) { - if ctx.Err() == context.DeadlineExceeded { - return ErrExecTimeout - } - } return err } @@ -210,6 +210,10 @@ func extractStderr(err error) string { if err == nil { return "" } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 { + return string(exitErr.Stderr) + } msg := err.Error() // sourcegraph/run error format: "exit status N: " if idx := strings.Index(msg, ": "); idx >= 0 && strings.HasPrefix(msg, "exit status") { diff --git a/repo.go b/repo.go index 8e7410ca..70a63d7a 100644 --- a/repo.go +++ b/repo.go @@ -387,13 +387,13 @@ func (r *Repository) Commit(ctx context.Context, committer *Signature, message s } args := []string{"commit"} - args = append(args, fmt.Sprintf("--author='%s <%s>'", opt.Author.Name, opt.Author.Email)) + args = append(args, fmt.Sprintf("--author=%s <%s>", opt.Author.Name, opt.Author.Email)) args = append(args, "-m", message) args = append(args, opt.Args...) _, err := gitRun(ctx, r.path, args, envs) // No stderr but exit status 1 means nothing to commit. - if err != nil && err.Error() == "exit status 1" { + if err != nil && strings.HasPrefix(err.Error(), "exit status 1") { return nil } return err @@ -448,11 +448,10 @@ func (r *Repository) ShowNameStatus(ctx context.Context, rev string, opts ...Sho args = append(args, opt.Args...) args = append(args, "--end-of-options", rev) - stderr := new(bytes.Buffer) - err := gitPipeline(ctx, r.path, args, opt.Envs, w, stderr, nil) + err := gitPipeline(ctx, r.path, args, opt.Envs, w, nil, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { - return nil, concatenateError(err, stderr.String()) + return nil, err } <-done @@ -481,7 +480,7 @@ func (r *Repository) RevParse(ctx context.Context, rev string, opts ...RevParseO commitID, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { - if strings.Contains(err.Error(), "exit status 128") { + if strings.HasPrefix(err.Error(), "exit status 128") { return "", ErrRevisionNotExist } return "", err diff --git a/repo_diff.go b/repo_diff.go index 69dcdbbd..38a80974 100644 --- a/repo_diff.go +++ b/repo_diff.go @@ -5,7 +5,6 @@ package git import ( - "bytes" "context" "fmt" "io" @@ -60,11 +59,10 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine done := make(chan SteamParseDiffResult) go StreamParseDiff(stdout, done, maxFiles, maxFileLines, maxLineChars) - stderr := new(bytes.Buffer) - err = gitPipeline(ctx, r.path, args, opt.Envs, w, stderr, nil) + err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { - return nil, concatenateError(err, stderr.String()) + return nil, err } result := <-done @@ -134,9 +132,8 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo return fmt.Errorf("invalid diffType: %s", diffType) } - stderr := new(bytes.Buffer) - if err = gitPipeline(ctx, r.path, args, opt.Envs, w, stderr, nil); err != nil { - return concatenateError(err, stderr.String()) + if err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil, nil); err != nil { + return err } return nil } @@ -157,7 +154,7 @@ func (r *Repository) DiffBinary(ctx context.Context, base, head string, opts ... args := []string{"diff"} args = append(args, opt.Args...) - args = append(args, "--full-index", "--binary", base, head) + args = append(args, "--full-index", "--binary", "--end-of-options", base, head) return gitRun(ctx, r.path, args, opt.Envs) } diff --git a/repo_pull.go b/repo_pull.go index 52738e59..80d09f89 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -31,7 +31,7 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { - if strings.Contains(err.Error(), "exit status 1") { + if strings.HasPrefix(err.Error(), "exit status 1") { return "", ErrNoMergeBase } return "", err diff --git a/utils.go b/utils.go index 79c0e278..305dfc10 100644 --- a/utils.go +++ b/utils.go @@ -5,7 +5,6 @@ package git import ( - "fmt" "os" "strings" "sync" @@ -66,13 +65,6 @@ func isExist(path string) bool { return err == nil || os.IsExist(err) } -func concatenateError(err error, stderr string) error { - if len(stderr) == 0 { - return err - } - return fmt.Errorf("%v - %s", err, stderr) -} - // bytesToStrings splits given bytes into strings by line separator ("\n"). It // returns empty slice if the given bytes only contains line separators. func bytesToStrings(in []byte) []string { From 762afdfaa994ecc877a0c630dbec07e350ed18ee Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 20:09:10 -0500 Subject: [PATCH 05/14] fix: address second round of PR review feedback - Fix exit status matching: replace loose HasPrefix with isExitStatus helper that won't match "exit status 1" against "exit status 128" - Fix opt.Args ordering in ShowRefVerify and DeleteTag: move before --end-of-options so flag args work correctly - Fix gitRun log capture: capture partial stdout on error instead of logging empty output for failed commands - Quote args with special characters in log output for readability - Fix repo_pull_test: "bad_revision" causes exit 128, not exit 1 Co-Authored-By: Claude Opus 4.6 (1M context) --- command.go | 31 +++++++++++++++++++++++++++---- repo.go | 4 ++-- repo_pull.go | 2 +- repo_pull_test.go | 6 ++++-- repo_reference.go | 3 ++- repo_remote.go | 1 - repo_tag.go | 3 ++- 7 files changed, 38 insertions(+), 12 deletions(-) diff --git a/command.go b/command.go index 35bcd89f..19dc356a 100644 --- a/command.go +++ b/command.go @@ -12,6 +12,7 @@ import ( "io" "os" "os/exec" + "strconv" "strings" "time" @@ -85,10 +86,9 @@ func gitRun(ctx context.Context, dir string, args []string, envs []string) ([]by // line-by-line which corrupts binary-ish output. stdout := new(bytes.Buffer) err := cmd.StdOut().Run().Stream(stdout) - if err != nil { - return nil, mapContextError(err, ctx) - } + // Capture (partial) stdout for logging even on error, so failed commands + // produce a useful log entry rather than an empty one. if logOutput != nil { data := stdout.Bytes() limit := len(data) @@ -101,6 +101,9 @@ func gitRun(ctx context.Context, dir string, args []string, envs []string) ([]by } } + if err != nil { + return nil, mapContextError(err, ctx) + } return stdout.Bytes(), nil } @@ -152,7 +155,15 @@ func committerEnvs(committer *Signature) []string { func logf(dir string, args []string, output []byte) { cmdStr := "git" if len(args) > 0 { - cmdStr = "git " + strings.Join(args, " ") + quoted := make([]string, len(args)) + for i, a := range args { + if strings.ContainsAny(a, " \t\n\"'\\<>") { + quoted[i] = strconv.Quote(a) + } else { + quoted[i] = a + } + } + cmdStr = "git " + strings.Join(quoted, " ") } if len(dir) == 0 { log("%s\n%s", cmdStr, output) @@ -204,6 +215,18 @@ func mapContextError(err error, ctx context.Context) error { return err } +// isExitStatus reports whether err represents a specific process exit status +// code. It handles both the bare "exit status N" format and the +// "exit status N: " format produced by sourcegraph/run. +func isExitStatus(err error, code int) bool { + if err == nil { + return false + } + prefix := fmt.Sprintf("exit status %d", code) + msg := err.Error() + return msg == prefix || strings.HasPrefix(msg, prefix+":") +} + // extractStderr attempts to extract the stderr portion from a sourcegraph/run // error. The error format is typically "exit status N: ". func extractStderr(err error) string { diff --git a/repo.go b/repo.go index 70a63d7a..021ae7d3 100644 --- a/repo.go +++ b/repo.go @@ -393,7 +393,7 @@ func (r *Repository) Commit(ctx context.Context, committer *Signature, message s _, err := gitRun(ctx, r.path, args, envs) // No stderr but exit status 1 means nothing to commit. - if err != nil && strings.HasPrefix(err.Error(), "exit status 1") { + if isExitStatus(err, 1) { return nil } return err @@ -480,7 +480,7 @@ func (r *Repository) RevParse(ctx context.Context, rev string, opts ...RevParseO commitID, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { - if strings.HasPrefix(err.Error(), "exit status 128") { + if isExitStatus(err, 128) { return "", ErrRevisionNotExist } return "", err diff --git a/repo_pull.go b/repo_pull.go index 80d09f89..acc19433 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -31,7 +31,7 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { - if strings.HasPrefix(err.Error(), "exit status 1") { + if isExitStatus(err, 1) { return "", ErrNoMergeBase } return "", err diff --git a/repo_pull_test.go b/repo_pull_test.go index ff231863..27a6a637 100644 --- a/repo_pull_test.go +++ b/repo_pull_test.go @@ -14,9 +14,11 @@ import ( func TestRepository_MergeBase(t *testing.T) { ctx := context.Background() - t.Run("no merge base", func(t *testing.T) { + t.Run("bad revision", func(t *testing.T) { + // "bad_revision" doesn't exist, so git fails with exit status 128 (fatal), + // not exit status 1 (no merge base). mb, err := testrepo.MergeBase(ctx, "0eedd79eba4394bbef888c804e899731644367fe", "bad_revision") - assert.Equal(t, ErrNoMergeBase, err) + assert.Error(t, err) assert.Empty(t, mb) }) diff --git a/repo_reference.go b/repo_reference.go index b8490b79..e09a207a 100644 --- a/repo_reference.go +++ b/repo_reference.go @@ -51,8 +51,9 @@ func (r *Repository) ShowRefVerify(ctx context.Context, ref string, opts ...Show opt = opts[0] } - args := []string{"show-ref", "--verify", "--end-of-options", ref} + args := []string{"show-ref", "--verify"} args = append(args, opt.Args...) + args = append(args, "--end-of-options", ref) stdout, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { diff --git a/repo_remote.go b/repo_remote.go index 4d6a4f02..34248ed2 100644 --- a/repo_remote.go +++ b/repo_remote.go @@ -136,7 +136,6 @@ func (r *Repository) RemoteRemove(ctx context.Context, name string, opts ...Remo _, err := gitRun(ctx, r.path, args, opt.Envs) if err != nil { - // the error status may differ from git clients if strings.Contains(err.Error(), "error: No such remote") || strings.Contains(err.Error(), "fatal: No such remote") { return ErrRemoteNotExist diff --git a/repo_tag.go b/repo_tag.go index a882da7f..d3124a42 100644 --- a/repo_tag.go +++ b/repo_tag.go @@ -234,8 +234,9 @@ func (r *Repository) DeleteTag(ctx context.Context, name string, opts ...DeleteT opt = opts[0] } - args := []string{"tag", "--delete", "--end-of-options", name} + args := []string{"tag", "--delete"} args = append(args, opt.Args...) + args = append(args, "--end-of-options", name) _, err := gitRun(ctx, r.path, args, opt.Envs) return err From b7f7b6fb49ed3537c19e41d86d265a9f8212d116 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 20:37:49 -0500 Subject: [PATCH 06/14] fix: address third round of PR review feedback - Remove nil check on ctx in gitCmd (callers always provide non-nil ctx) - Remove stdin parameter from gitPipeline (no production caller uses it) - Rewrite isExitStatus to use run.ExitCoder interface instead of string parsing, as suggested by reviewer - Remove unused errors and os/exec imports - Update tests to use gitCmd directly for stdin-based scenarios Co-Authored-By: Claude Opus 4.6 (1M context) --- blob.go | 2 +- command.go | 27 +++++---------------------- command_test.go | 18 +++++++++++++----- repo.go | 2 +- repo_diff.go | 4 ++-- 5 files changed, 22 insertions(+), 31 deletions(-) diff --git a/blob.go b/blob.go index e9a33f83..b46f2f8a 100644 --- a/blob.go +++ b/blob.go @@ -34,5 +34,5 @@ func (b *Blob) Bytes(ctx context.Context) ([]byte, error) { // 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, nil) + return gitPipeline(ctx, b.parent.repo.path, []string{"show", b.id.String()}, nil, stdout, stderr) } diff --git a/command.go b/command.go index 19dc356a..f2814def 100644 --- a/command.go +++ b/command.go @@ -7,11 +7,9 @@ package git import ( "bytes" "context" - "errors" "fmt" "io" "os" - "os/exec" "strconv" "strings" "time" @@ -33,9 +31,6 @@ const DefaultTimeout = time.Minute // 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) { - if ctx == nil { - ctx = context.Background() - } cancel := func() {} // Apply default timeout if the context doesn't already have a deadline. @@ -109,13 +104,10 @@ func gitRun(ctx context.Context, dir string, args []string, envs []string) ([]by // 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. stdin is optional. -func gitPipeline(ctx context.Context, dir string, args []string, envs []string, stdout, stderr io.Writer, stdin io.Reader) error { +// 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) defer cancel() - if stdin != nil { - cmd = cmd.Input(stdin) - } var buf *bytes.Buffer w := stdout @@ -216,15 +208,10 @@ func mapContextError(err error, ctx context.Context) error { } // isExitStatus reports whether err represents a specific process exit status -// code. It handles both the bare "exit status N" format and the -// "exit status N: " format produced by sourcegraph/run. +// code, using the run.ExitCoder interface provided by sourcegraph/run. func isExitStatus(err error, code int) bool { - if err == nil { - return false - } - prefix := fmt.Sprintf("exit status %d", code) - msg := err.Error() - return msg == prefix || strings.HasPrefix(msg, prefix+":") + exitCoder, ok := err.(run.ExitCoder) + return ok && exitCoder.ExitCode() == code } // extractStderr attempts to extract the stderr portion from a sourcegraph/run @@ -233,10 +220,6 @@ func extractStderr(err error) string { if err == nil { return "" } - var exitErr *exec.ExitError - if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 { - return string(exitErr.Stderr) - } msg := err.Error() // sourcegraph/run error format: "exit status N: " if idx := strings.Index(msg, ": "); idx >= 0 && strings.HasPrefix(msg, "exit status") { diff --git a/command_test.go b/command_test.go index e1aa9b68..95d7e865 100644 --- a/command_test.go +++ b/command_test.go @@ -26,9 +26,13 @@ func TestGitRun_ContextTimeout(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - // Use a blocking reader so the command starts successfully and blocks - // reading stdin until the context deadline fires. - err := gitPipeline(ctx, "", []string{"hash-object", "--stdin"}, nil, io.Discard, io.Discard, blockingReader{cancel: ctx.Done()}) + // Use gitCmd 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) + defer timeoutCancel() + + err := cmd.Input(blockingReader{cancel: ctx.Done()}).StdOut().Run().Stream(io.Discard) + err = mapContextError(err, ctx) assert.Equal(t, ErrExecTimeout, err) }) } @@ -46,7 +50,7 @@ func (r blockingReader) Read(p []byte) (int, error) { return 0, io.EOF } -func TestGitPipeline_ContextCancellation(t *testing.T) { +func TestGitCmd_ContextCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) // Cancel in the background after a short delay so the command is already @@ -58,7 +62,11 @@ func TestGitPipeline_ContextCancellation(t *testing.T) { close(done) }() - err := gitPipeline(ctx, "", []string{"hash-object", "--stdin"}, nil, io.Discard, io.Discard, blockingReader{cancel: done}) + cmd, timeoutCancel := gitCmd(ctx, "", []string{"hash-object", "--stdin"}, nil) + defer timeoutCancel() + + err := cmd.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) diff --git a/repo.go b/repo.go index 021ae7d3..5abb2d7c 100644 --- a/repo.go +++ b/repo.go @@ -448,7 +448,7 @@ func (r *Repository) ShowNameStatus(ctx context.Context, rev string, opts ...Sho args = append(args, opt.Args...) args = append(args, "--end-of-options", rev) - err := gitPipeline(ctx, r.path, args, opt.Envs, w, nil, nil) + err := gitPipeline(ctx, r.path, args, opt.Envs, w, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, err diff --git a/repo_diff.go b/repo_diff.go index 38a80974..aa1cd5cf 100644 --- a/repo_diff.go +++ b/repo_diff.go @@ -59,7 +59,7 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine done := make(chan SteamParseDiffResult) go StreamParseDiff(stdout, done, maxFiles, maxFileLines, maxLineChars) - err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil, nil) + err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, err @@ -132,7 +132,7 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo return fmt.Errorf("invalid diffType: %s", diffType) } - if err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil, nil); err != nil { + if err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil); err != nil { return err } return nil From ffbfd0a6db174003b43675c4ed5a9cf714769dff Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 20:57:18 -0500 Subject: [PATCH 07/14] 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) --- blob.go | 10 ++++---- blob_test.go | 2 +- command.go | 49 +++++++++++------------------------ command_test.go | 58 ++++++++---------------------------------- commit.go | 2 +- commit_archive.go | 7 ++--- commit_archive_test.go | 4 +-- git.go | 2 +- repo.go | 28 ++++++++++---------- repo_blame.go | 2 +- repo_commit.go | 14 +++++----- repo_diff.go | 6 ++--- repo_grep.go | 2 +- repo_pull.go | 2 +- repo_reference.go | 8 +++--- repo_remote.go | 16 ++++++------ repo_tag.go | 8 +++--- repo_tree.go | 2 +- server.go | 6 ++--- tree_entry.go | 2 +- 20 files changed, 88 insertions(+), 142 deletions(-) diff --git a/blob.go b/blob.go index b46f2f8a..f8a5fb7c 100644 --- a/blob.go +++ b/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) } diff --git a/blob_test.go b/blob_test.go index 6ccc46ee..71753843 100644 --- a/blob_test.go +++ b/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()) }) diff --git a/command.go b/command.go index f2814def..549fbeb9 100644 --- a/command.go +++ b/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: ". -func extractStderr(err error) string { - if err == nil { - return "" - } - msg := err.Error() - // sourcegraph/run error format: "exit status N: " - if idx := strings.Index(msg, ": "); idx >= 0 && strings.HasPrefix(msg, "exit status") { - return msg[idx+2:] - } - return msg -} diff --git a/command_test.go b/command_test.go index 95d7e865..a1cd98ca 100644 --- a/command_test.go +++ b/command_test.go @@ -13,12 +13,12 @@ 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) }) @@ -26,12 +26,12 @@ func TestGitRun_ContextTimeout(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) }) @@ -39,7 +39,7 @@ func TestGitRun_ContextTimeout(t *testing.T) { // 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 } diff --git a/commit.go b/commit.go index 381734e2..ca4c4727 100644 --- a/commit.go +++ b/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 } diff --git a/commit_archive.go b/commit_archive.go index 65393f9f..a0833fc1 100644 --- a/commit_archive.go +++ b/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 diff --git a/commit_archive_test.go b/commit_archive_test.go index 2523c73c..892859c8 100644 --- a/commit_archive_test.go +++ b/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)) }) } } diff --git a/git.go b/git.go index 0b4d27bb..91506f00 100644 --- a/git.go +++ b/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 } diff --git a/repo.go b/repo.go index 5abb2d7c..82bd00c7 100644 --- a/repo.go +++ b/repo.go @@ -80,7 +80,7 @@ func Init(ctx context.Context, path string, opts ...InitOptions) error { args = append(args, "--bare") } args = append(args, "--end-of-options") - _, err = gitRun(ctx, path, args, opt.Envs) + _, err = exec(ctx, path, args, opt.Envs) return err } @@ -151,7 +151,7 @@ func Clone(ctx context.Context, url, dst string, opts ...CloneOptions) error { } args = append(args, "--end-of-options", url, dst) - _, err = gitRun(ctx, "", args, opt.Envs) + _, err = exec(ctx, "", args, opt.Envs) return err } @@ -178,7 +178,7 @@ func (r *Repository) Fetch(ctx context.Context, opts ...FetchOptions) error { args = append(args, "--prune") } - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -220,7 +220,7 @@ func (r *Repository) Pull(ctx context.Context, opts ...PullOptions) error { } } - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -242,7 +242,7 @@ func (r *Repository) Push(ctx context.Context, remote, branch string, opts ...Pu args := []string{"push"} args = append(args, opt.Args...) args = append(args, "--end-of-options", remote, branch) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -273,7 +273,7 @@ func (r *Repository) Checkout(ctx context.Context, branch string, opts ...Checko args = append(args, opt.BaseBranch) } - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -301,7 +301,7 @@ func (r *Repository) Reset(ctx context.Context, rev string, opts ...ResetOptions args = append(args, opt.Args...) args = append(args, "--end-of-options", rev) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -325,7 +325,7 @@ func (r *Repository) Move(ctx context.Context, src, dst string, opts ...MoveOpti args := []string{"mv"} args = append(args, opt.Args...) args = append(args, "--end-of-options", src, dst) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -357,7 +357,7 @@ func (r *Repository) Add(ctx context.Context, opts ...AddOptions) error { args = append(args, "--") args = append(args, opt.Pathspecs...) } - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -391,7 +391,7 @@ func (r *Repository) Commit(ctx context.Context, committer *Signature, message s args = append(args, "-m", message) args = append(args, opt.Args...) - _, err := gitRun(ctx, r.path, args, envs) + _, err := exec(ctx, r.path, args, envs) // No stderr but exit status 1 means nothing to commit. if isExitStatus(err, 1) { return nil @@ -448,7 +448,7 @@ func (r *Repository) ShowNameStatus(ctx context.Context, rev string, opts ...Sho args = append(args, opt.Args...) args = append(args, "--end-of-options", rev) - err := gitPipeline(ctx, r.path, args, opt.Envs, w, nil) + err := pipe(ctx, r.path, args, opt.Envs, w) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, err @@ -478,7 +478,7 @@ func (r *Repository) RevParse(ctx context.Context, rev string, opts ...RevParseO args = append(args, opt.Args...) args = append(args, rev) - commitID, err := gitRun(ctx, r.path, args, opt.Envs) + commitID, err := exec(ctx, r.path, args, opt.Envs) if err != nil { if isExitStatus(err, 128) { return "", ErrRevisionNotExist @@ -518,7 +518,7 @@ func (r *Repository) CountObjects(ctx context.Context, opts ...CountObjectsOptio args := []string{"count-objects", "-v"} args = append(args, opt.Args...) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -571,6 +571,6 @@ func (r *Repository) Fsck(ctx context.Context, opts ...FsckOptions) error { args := []string{"fsck"} args = append(args, opt.Args...) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } diff --git a/repo_blame.go b/repo_blame.go index af480fcc..269279f3 100644 --- a/repo_blame.go +++ b/repo_blame.go @@ -28,7 +28,7 @@ func (r *Repository) Blame(ctx context.Context, rev, file string, opts ...BlameO args = append(args, opt.Args...) args = append(args, "-l", "-s", rev, "--", file) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } diff --git a/repo_commit.go b/repo_commit.go index eb40b3da..da4e52dc 100644 --- a/repo_commit.go +++ b/repo_commit.go @@ -98,7 +98,7 @@ func (r *Repository) CatFileCommit(ctx context.Context, rev string, opts ...CatF args = append(args, opt.Args...) args = append(args, "commit", commitID) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -133,7 +133,7 @@ func (r *Repository) CatFileType(ctx context.Context, rev string, opts ...CatFil args = append(args, opt.Args...) args = append(args, "-t", rev) - typ, err := gitRun(ctx, r.path, args, opt.Envs) + typ, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return "", err } @@ -216,7 +216,7 @@ func (r *Repository) Log(ctx context.Context, rev string, opts ...LogOptions) ([ args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -371,7 +371,7 @@ func (r *Repository) DiffNameOnly(ctx context.Context, base, head string, opts . args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -419,7 +419,7 @@ func (r *Repository) RevListCount(ctx context.Context, refspecs []string, opts . args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return 0, err } @@ -458,7 +458,7 @@ func (r *Repository) RevList(ctx context.Context, refspecs []string, opts ...Rev args = append(args, escapePath(opt.Path)) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -488,7 +488,7 @@ func (r *Repository) LatestCommitTime(ctx context.Context, opts ...LatestCommitT args = append(args, RefsHeads+opt.Branch) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return time.Time{}, err } diff --git a/repo_diff.go b/repo_diff.go index aa1cd5cf..368a861d 100644 --- a/repo_diff.go +++ b/repo_diff.go @@ -59,7 +59,7 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine done := make(chan SteamParseDiffResult) go StreamParseDiff(stdout, done, maxFiles, maxFileLines, maxLineChars) - err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil) + err = pipe(ctx, r.path, args, opt.Envs, w) _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, err @@ -132,7 +132,7 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo return fmt.Errorf("invalid diffType: %s", diffType) } - if err = gitPipeline(ctx, r.path, args, opt.Envs, w, nil); err != nil { + if err = pipe(ctx, r.path, args, opt.Envs, w); err != nil { return err } return nil @@ -156,5 +156,5 @@ func (r *Repository) DiffBinary(ctx context.Context, base, head string, opts ... args = append(args, opt.Args...) args = append(args, "--full-index", "--binary", "--end-of-options", base, head) - return gitRun(ctx, r.path, args, opt.Envs) + return exec(ctx, r.path, args, opt.Envs) } diff --git a/repo_grep.go b/repo_grep.go index 176c71ce..e40a1e4b 100644 --- a/repo_grep.go +++ b/repo_grep.go @@ -96,7 +96,7 @@ func (r *Repository) Grep(ctx context.Context, pattern string, opts ...GrepOptio args = append(args, "--", opt.Pathspec) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil } diff --git a/repo_pull.go b/repo_pull.go index acc19433..33d2be76 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -29,7 +29,7 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M args = append(args, opt.Args...) args = append(args, "--end-of-options", base, head) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { if isExitStatus(err, 1) { return "", ErrNoMergeBase diff --git a/repo_reference.go b/repo_reference.go index e09a207a..67e97b03 100644 --- a/repo_reference.go +++ b/repo_reference.go @@ -55,7 +55,7 @@ func (r *Repository) ShowRefVerify(ctx context.Context, ref string, opts ...Show args = append(args, opt.Args...) args = append(args, "--end-of-options", ref) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "not a valid ref") { return "", ErrReferenceNotExist @@ -126,7 +126,7 @@ func (r *Repository) SymbolicRef(ctx context.Context, opts ...SymbolicRefOptions args = append(args, opt.Ref) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return "", err } @@ -167,7 +167,7 @@ func (r *Repository) ShowRef(ctx context.Context, opts ...ShowRefOptions) ([]*Re args = append(args, opt.Patterns...) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -227,6 +227,6 @@ func (r *Repository) DeleteBranch(ctx context.Context, name string, opts ...Dele } args = append(args, "--end-of-options", name) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } diff --git a/repo_remote.go b/repo_remote.go index 34248ed2..50b401e1 100644 --- a/repo_remote.go +++ b/repo_remote.go @@ -50,7 +50,7 @@ func LsRemote(ctx context.Context, url string, opts ...LsRemoteOptions) ([]*Refe args = append(args, opt.Patterns...) } - stdout, err := gitRun(ctx, "", args, opt.Envs) + stdout, err := exec(ctx, "", args, opt.Envs) if err != nil { return nil, err } @@ -110,7 +110,7 @@ func (r *Repository) RemoteAdd(ctx context.Context, name, url string, opts ...Re } args = append(args, "--end-of-options", name, url) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -134,7 +134,7 @@ func (r *Repository) RemoteRemove(ctx context.Context, name string, opts ...Remo args = append(args, opt.Args...) args = append(args, "--end-of-options", name) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "error: No such remote") || strings.Contains(err.Error(), "fatal: No such remote") { @@ -163,7 +163,7 @@ func (r *Repository) Remotes(ctx context.Context, opts ...RemotesOptions) ([]str args := []string{"remote"} args = append(args, opt.Args...) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -202,7 +202,7 @@ func (r *Repository) RemoteGetURL(ctx context.Context, name string, opts ...Remo } args = append(args, "--end-of-options", name) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -240,7 +240,7 @@ func (r *Repository) RemoteSetURL(ctx context.Context, name, newurl string, opts args = append(args, opt.Regex) } - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "No such URL found") { return ErrURLNotExist @@ -279,7 +279,7 @@ func (r *Repository) RemoteSetURLAdd(ctx context.Context, name, newurl string, o } args = append(args, "--end-of-options", name, newurl) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") { return ErrNotDeleteNonPushURLs } @@ -313,7 +313,7 @@ func (r *Repository) RemoteSetURLDelete(ctx context.Context, name, regex string, } args = append(args, "--end-of-options", name, regex) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") { return ErrNotDeleteNonPushURLs } diff --git a/repo_tag.go b/repo_tag.go index d3124a42..d2c1ce96 100644 --- a/repo_tag.go +++ b/repo_tag.go @@ -76,7 +76,7 @@ func (r *Repository) getTag(ctx context.Context, id *SHA1) (*Tag, error) { } case ObjectTag: // Tag is an annotation - data, err := gitRun(ctx, r.path, []string{"cat-file", "-p", id.String()}, nil) + data, err := exec(ctx, r.path, []string{"cat-file", "-p", id.String()}, nil) if err != nil { return nil, err } @@ -166,7 +166,7 @@ func (r *Repository) Tags(ctx context.Context, opts ...TagsOptions) ([]string, e args = append(args, opt.Pattern) } - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } @@ -215,7 +215,7 @@ func (r *Repository) CreateTag(ctx context.Context, name, rev string, opts ...Cr args = append(args, rev) envs = append(envs, opt.Envs...) - _, err := gitRun(ctx, r.path, args, envs) + _, err := exec(ctx, r.path, args, envs) return err } @@ -238,6 +238,6 @@ func (r *Repository) DeleteTag(ctx context.Context, name string, opts ...DeleteT args = append(args, opt.Args...) args = append(args, "--end-of-options", name) - _, err := gitRun(ctx, r.path, args, opt.Envs) + _, err := exec(ctx, r.path, args, opt.Envs) return err } diff --git a/repo_tree.go b/repo_tree.go index 8cd1d1f8..715a5e00 100644 --- a/repo_tree.go +++ b/repo_tree.go @@ -138,7 +138,7 @@ func (r *Repository) LsTree(ctx context.Context, treeID string, opts ...LsTreeOp } args = append(args, treeID) - stdout, err := gitRun(ctx, r.path, args, opt.Envs) + stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err } diff --git a/server.go b/server.go index 4eaa402f..9bb8f142 100755 --- a/server.go +++ b/server.go @@ -33,7 +33,7 @@ func UpdateServerInfo(ctx context.Context, path string, opts ...UpdateServerInfo if opt.Force { args = append(args, "--force") } - _, err := gitRun(ctx, path, args, opt.Envs) + _, err := exec(ctx, path, args, opt.Envs) return err } @@ -66,7 +66,7 @@ func ReceivePack(ctx context.Context, path string, opts ...ReceivePackOptions) ( args = append(args, "--http-backend-info-refs") } args = append(args, ".") - return gitRun(ctx, path, args, opt.Envs) + return exec(ctx, path, args, opt.Envs) } // UploadPackOptions contains optional arguments for sending the packfile to the @@ -111,5 +111,5 @@ func UploadPack(ctx context.Context, path string, opts ...UploadPackOptions) ([] args = append(args, "--http-backend-info-refs") } args = append(args, ".") - return gitRun(ctx, path, args, opt.Envs) + return exec(ctx, path, args, opt.Envs) } diff --git a/tree_entry.go b/tree_entry.go index f3d68e1f..c854cfc7 100644 --- a/tree_entry.go +++ b/tree_entry.go @@ -98,7 +98,7 @@ func (e *TreeEntry) Size(ctx context.Context) int64 { return e.size } - stdout, err := gitRun(ctx, e.parent.repo.path, []string{"cat-file", "-s", e.id.String()}, nil) + stdout, err := exec(ctx, e.parent.repo.path, []string{"cat-file", "-s", e.id.String()}, nil) if err != nil { return 0 } From d4c8fef29a60f60075f796decc4e0644dd11cdc0 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 21:27:44 -0500 Subject: [PATCH 08/14] 111 --- blob.go | 11 +++++------ blob_test.go | 4 ++-- command.go | 44 ++++++++++++++++++++++---------------------- commit.go | 2 +- git.go | 2 +- git_test.go | 2 +- repo_commit.go | 2 +- repo_tag.go | 2 +- repo_tree.go | 2 +- 9 files changed, 35 insertions(+), 36 deletions(-) diff --git a/blob.go b/blob.go index f8a5fb7c..4cb884c5 100644 --- a/blob.go +++ b/blob.go @@ -15,8 +15,8 @@ type Blob struct { *TreeEntry } -// Bytes reads and returns the content of the blob all at once in bytes. This -// can be very slow and memory consuming for huge content. +// Bytes reads and returns the content of the blob all at once in bytes. This can +// be very slow and memory consuming for huge content. func (b *Blob) Bytes(ctx context.Context) ([]byte, error) { stdout := new(bytes.Buffer) @@ -25,14 +25,13 @@ func (b *Blob) Bytes(ctx context.Context) ([]byte, error) { stdout.Grow(int(size)) } - if err := b.Pipeline(ctx, stdout); err != nil { + if err := b.Pipe(ctx, stdout); err != nil { return nil, err } return stdout.Bytes(), nil } -// 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 { +// Pipe reads the content of the blob and pipes stdout to the supplied io.Writer. +func (b *Blob) Pipe(ctx context.Context, stdout io.Writer) error { return pipe(ctx, b.parent.repo.path, []string{"show", b.id.String()}, nil, stdout) } diff --git a/blob_test.go b/blob_test.go index 71753843..743c3ed0 100644 --- a/blob_test.go +++ b/blob_test.go @@ -48,9 +48,9 @@ This demo also includes an image with changes on a branch for examination of ima assert.Equal(t, expOutput, string(p)) }) - t.Run("get data with pipeline", func(t *testing.T) { + t.Run("get data with pipe", func(t *testing.T) { stdout := new(bytes.Buffer) - err := blob.Pipeline(ctx, stdout) + err := blob.Pipe(ctx, stdout) assert.Nil(t, err) assert.Equal(t, expOutput, stdout.String()) }) diff --git a/command.go b/command.go index 549fbeb9..5d68d596 100644 --- a/command.go +++ b/command.go @@ -7,6 +7,7 @@ package git import ( "bytes" "context" + "errors" "io" "os" "strconv" @@ -26,22 +27,20 @@ type CommandOptions struct { // applied when the context does not already have a deadline. const DefaultTimeout = time.Minute -// 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. +// cmd builds a *run.Command for git with the given arguments, environment +// variables and working directory. DefaultTimeout will be applied if the context +// does not already have a deadline. 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. if _, ok := ctx.Deadline(); !ok { var timeoutCancel context.CancelFunc ctx, timeoutCancel = context.WithTimeout(ctx, DefaultTimeout) cancel = timeoutCancel } - // run.Cmd joins all parts into a single string and then shell-parses it. - // We must quote each argument so that special characters (spaces, quotes, - // angle brackets, etc.) are preserved correctly. + // run.Cmd joins all parts into a single string and then shell-parses it. We must + // quote each argument so that special characters (spaces, quotes, angle + // brackets, etc.) are preserved correctly. parts := make([]string, 0, 1+len(args)) parts = append(parts, "git") for _, arg := range args { @@ -59,9 +58,9 @@ func cmd(ctx context.Context, dir string, args []string, envs []string) (*run.Co } // 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. +// bytes. Stderr is included in the error message on failure. DefaultTimeout will +// be applied if the context does not already have a deadline. It returns +// ErrExecTimeout if the execution was timed out. func exec(ctx context.Context, dir string, args []string, envs []string) ([]byte, error) { c, cancel := cmd(ctx, dir, args, envs) defer cancel() @@ -71,7 +70,7 @@ func exec(ctx context.Context, dir string, args []string, envs []string) ([]byte logBuf = new(bytes.Buffer) logBuf.Grow(512) defer func() { - logf(dir, args, logBuf.Bytes()) + log(dir, args, logBuf.Bytes()) }() } @@ -81,8 +80,8 @@ func exec(ctx context.Context, dir string, args []string, envs []string) ([]byte stdout := new(bytes.Buffer) 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. + // Capture (partial) stdout for logging even on error, so failed commands produce + // a useful log entry rather than an empty one. if logOutput != nil { data := stdout.Bytes() limit := len(data) @@ -102,7 +101,7 @@ func exec(ctx context.Context, dir string, args []string, envs []string) ([]byte } // pipe executes a git command in the given directory, streaming stdout to the -// given writer. +// given io.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() @@ -119,7 +118,7 @@ func pipe(ctx context.Context, dir string, args []string, envs []string, stdout } defer func() { - logf(dir, args, buf.Bytes()) + log(dir, args, buf.Bytes()) }() } @@ -138,8 +137,8 @@ func committerEnvs(committer *Signature) []string { } } -// logf logs a git command execution with optional output. -func logf(dir string, args []string, output []byte) { +// log logs a git command execution with its output. +func log(dir string, args []string, output []byte) { cmdStr := "git" if len(args) > 0 { quoted := make([]string, len(args)) @@ -153,9 +152,9 @@ func logf(dir string, args []string, output []byte) { cmdStr = "git " + strings.Join(quoted, " ") } if len(dir) == 0 { - log("%s\n%s", cmdStr, output) + logf("%s\n%s", cmdStr, output) } else { - log("%s: %s\n%s", dir, cmdStr, output) + logf("%s: %s\n%s", dir, cmdStr, output) } } @@ -194,7 +193,7 @@ func mapContextError(err error, ctx context.Context) error { return err } if ctxErr := ctx.Err(); ctxErr != nil { - if ctxErr == context.DeadlineExceeded { + if errors.Is(ctxErr, context.DeadlineExceeded) { return ErrExecTimeout } return ctxErr @@ -205,6 +204,7 @@ func mapContextError(err error, ctx context.Context) error { // isExitStatus reports whether err represents a specific process exit status // code, using the run.ExitCoder interface provided by sourcegraph/run. func isExitStatus(err error, code int) bool { - exitCoder, ok := err.(run.ExitCoder) + var exitCoder run.ExitCoder + ok := errors.As(err, &exitCoder) return ok && exitCoder.ExitCode() == code } diff --git a/commit.go b/commit.go index ca4c4727..134f39fb 100644 --- a/commit.go +++ b/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) + err = blob.Pipe(ctx, stdout) if err != nil { return false, err } diff --git a/git.go b/git.go index 91506f00..e96ab1e9 100644 --- a/git.go +++ b/git.go @@ -29,7 +29,7 @@ func SetPrefix(prefix string) { logPrefix = prefix } -func log(format string, args ...interface{}) { +func logf(format string, args ...interface{}) { if logOutput == nil { return } diff --git a/git_test.go b/git_test.go index 5c0e0df9..c4af3655 100644 --- a/git_test.go +++ b/git_test.go @@ -83,7 +83,7 @@ func Test_log(t *testing.T) { var buf bytes.Buffer SetOutput(&buf) - log(test.format, test.args...) + logf(test.format, test.args...) assert.Equal(t, test.expOutput, buf.String()) }) } diff --git a/repo_commit.go b/repo_commit.go index da4e52dc..45c38fd9 100644 --- a/repo_commit.go +++ b/repo_commit.go @@ -85,7 +85,7 @@ func (r *Repository) CatFileCommit(ctx context.Context, rev string, opts ...CatF cache, ok := r.cachedCommits.Get(rev) if ok { - log("Cached commit hit: %s", rev) + logf("Cached commit hit: %s", rev) return cache.(*Commit), nil } diff --git a/repo_tag.go b/repo_tag.go index d2c1ce96..2637514a 100644 --- a/repo_tag.go +++ b/repo_tag.go @@ -55,7 +55,7 @@ l: func (r *Repository) getTag(ctx context.Context, id *SHA1) (*Tag, error) { t, ok := r.cachedTags.Get(id.String()) if ok { - log("Cached tag hit: %s", id) + logf("Cached tag hit: %s", id) return t.(*Tag), nil } diff --git a/repo_tree.go b/repo_tree.go index 715a5e00..ea8f82ca 100644 --- a/repo_tree.go +++ b/repo_tree.go @@ -117,7 +117,7 @@ func (r *Repository) LsTree(ctx context.Context, treeID string, opts ...LsTreeOp cache, ok := r.cachedTrees.Get(treeID) if ok { - log("Cached tree hit: %s", treeID) + logf("Cached tree hit: %s", treeID) return cache.(*Tree), nil } From 1f9c908442a72377b22dcb978944a42504f1ab8d Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 14 Feb 2026 21:32:10 -0500 Subject: [PATCH 09/14] 111 --- command_test.go | 22 +++++++++++----------- commit_archive.go | 20 ++++++++++++-------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/command_test.go b/command_test.go index a1cd98ca..4c02835d 100644 --- a/command_test.go +++ b/command_test.go @@ -26,8 +26,8 @@ func TestExec_ContextTimeout(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - // Use cmd directly with a blocking stdin so the command starts - // successfully and blocks reading until the context deadline fires. + // Use cmd directly with a blocking stdin so the command starts successfully and + // blocks reading until the context deadline fires. c, timeoutCancel := cmd(ctx, "", []string{"hash-object", "--stdin"}, nil) defer timeoutCancel() @@ -37,10 +37,10 @@ func TestExec_ContextTimeout(t *testing.T) { }) } -// 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 the stdin copy goroutine can exit cleanly, -// allowing cmd.Wait() to return. +// blockingReader is an io.Reader that blocks until its cancel channel is closed, +// simulating a stdin that never provides data. When canceled it returns io.EOF +// so that the stdin copy goroutine can exit cleanly, allowing cmd.Wait() to +// return. type blockingReader struct { cancel <-chan struct{} } @@ -53,8 +53,8 @@ func (r blockingReader) Read(p []byte) (int, error) { 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 - // running when cancellation arrives. Close done to unblock the reader. + // Cancel in the background after a short delay so the command is already running + // when cancellation arrives. Close done to unblock the reader. done := make(chan struct{}) go func() { time.Sleep(50 * time.Millisecond) @@ -73,9 +73,9 @@ func TestCmd_ContextCancellation(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. + // A plain context.Background() has no deadline. The command should still succeed + // because DefaultTimeout is applied automatically and "git version" completes + // well within that. ctx := context.Background() stdout, err := exec(ctx, "", []string{"version"}, nil) assert.NoError(t, err) diff --git a/commit_archive.go b/commit_archive.go index a0833fc1..5daba32b 100644 --- a/commit_archive.go +++ b/commit_archive.go @@ -22,13 +22,17 @@ const ( // 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 := exec(ctx, c.repo.path, []string{ - "archive", - "--prefix=" + prefix, - "--format=" + string(format), - "-o", dst, - "--end-of-options", - c.ID.String(), - }, nil) + _, err := exec(ctx, + c.repo.path, + []string{ + "archive", + "--prefix=" + prefix, + "--format=" + string(format), + "-o", dst, + "--end-of-options", + c.ID.String(), + }, + nil, + ) return err } From 49f2962ed119aec19d931a7f2f09708176883d7b Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sun, 15 Feb 2026 11:54:41 -0500 Subject: [PATCH 10/14] 11 --- blob.go | 2 +- command.go | 4 ++-- repo.go | 41 ++++++++++++----------------------------- repo_blame.go | 4 +--- repo_commit.go | 28 +++++++--------------------- repo_diff.go | 39 +++++++++++---------------------------- repo_grep.go | 3 +-- repo_pull.go | 1 - repo_reference.go | 15 +++++---------- repo_remote.go | 35 ++++++++++++----------------------- repo_tag.go | 7 ++----- repo_tree.go | 3 +-- server.go | 8 +++----- 13 files changed, 58 insertions(+), 132 deletions(-) diff --git a/blob.go b/blob.go index 4cb884c5..843e1dbd 100644 --- a/blob.go +++ b/blob.go @@ -33,5 +33,5 @@ func (b *Blob) Bytes(ctx context.Context) ([]byte, error) { // Pipe reads the content of the blob and pipes stdout to the supplied io.Writer. func (b *Blob) Pipe(ctx context.Context, stdout io.Writer) error { - return pipe(ctx, b.parent.repo.path, []string{"show", b.id.String()}, nil, stdout) + return pipe(ctx, b.parent.repo.path, []string{"show", "--end-of-options", b.id.String()}, nil, stdout) } diff --git a/command.go b/command.go index 5d68d596..d7548370 100644 --- a/command.go +++ b/command.go @@ -17,9 +17,9 @@ import ( "github.com/sourcegraph/run" ) -// CommandOptions contains options for running a command. +// CommandOptions contains additional options for running a Git command. type CommandOptions struct { - Args []string + // The additional environment variables to be passed to the underlying Git. Envs []string } diff --git a/repo.go b/repo.go index 82bd00c7..7acfeb5a 100644 --- a/repo.go +++ b/repo.go @@ -75,7 +75,6 @@ func Init(ctx context.Context, path string, opts ...InitOptions) error { } args := []string{"init"} - args = append(args, opt.Args...) if opt.Bare { args = append(args, "--bare") } @@ -133,7 +132,6 @@ func Clone(ctx context.Context, url, dst string, opts ...CloneOptions) error { } args := []string{"clone"} - args = append(args, opt.Args...) if opt.Mirror { args = append(args, "--mirror") } @@ -173,10 +171,10 @@ func (r *Repository) Fetch(ctx context.Context, opts ...FetchOptions) error { } args := []string{"fetch"} - args = append(args, opt.Args...) if opt.Prune { args = append(args, "--prune") } + args = append(args, "--end-of-options") _, err := exec(ctx, r.path, args, opt.Envs) return err @@ -206,13 +204,13 @@ func (r *Repository) Pull(ctx context.Context, opts ...PullOptions) error { } args := []string{"pull"} - args = append(args, opt.Args...) if opt.Rebase { args = append(args, "--rebase") } if opt.All { args = append(args, "--all") } + args = append(args, "--end-of-options") if !opt.All && opt.Remote != "" { args = append(args, opt.Remote) if opt.Branch != "" { @@ -239,9 +237,7 @@ func (r *Repository) Push(ctx context.Context, remote, branch string, opts ...Pu opt = opts[0] } - args := []string{"push"} - args = append(args, opt.Args...) - args = append(args, "--end-of-options", remote, branch) + args := []string{"push", "--end-of-options", remote, branch} _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -264,13 +260,10 @@ func (r *Repository) Checkout(ctx context.Context, branch string, opts ...Checko } args := []string{"checkout"} - args = append(args, opt.Args...) - if opt.BaseBranch != "" { - args = append(args, "-b") - } - args = append(args, branch) if opt.BaseBranch != "" { - args = append(args, opt.BaseBranch) + args = append(args, "-b", branch, "--end-of-options", opt.BaseBranch) + } else { + args = append(args, "--end-of-options", branch) } _, err := exec(ctx, r.path, args, opt.Envs) @@ -298,7 +291,6 @@ func (r *Repository) Reset(ctx context.Context, rev string, opts ...ResetOptions if opt.Hard { args = append(args, "--hard") } - args = append(args, opt.Args...) args = append(args, "--end-of-options", rev) _, err := exec(ctx, r.path, args, opt.Envs) @@ -322,9 +314,7 @@ func (r *Repository) Move(ctx context.Context, src, dst string, opts ...MoveOpti opt = opts[0] } - args := []string{"mv"} - args = append(args, opt.Args...) - args = append(args, "--end-of-options", src, dst) + args := []string{"mv", "--end-of-options", src, dst} _, err := exec(ctx, r.path, args, opt.Envs) return err } @@ -349,7 +339,6 @@ func (r *Repository) Add(ctx context.Context, opts ...AddOptions) error { } args := []string{"add"} - args = append(args, opt.Args...) if opt.All { args = append(args, "--all") } @@ -389,7 +378,7 @@ func (r *Repository) Commit(ctx context.Context, committer *Signature, message s args := []string{"commit"} args = append(args, fmt.Sprintf("--author=%s <%s>", opt.Author.Name, opt.Author.Email)) args = append(args, "-m", message) - args = append(args, opt.Args...) + args = append(args, "--end-of-options") _, err := exec(ctx, r.path, args, envs) // No stderr but exit status 1 means nothing to commit. @@ -444,9 +433,7 @@ func (r *Repository) ShowNameStatus(ctx context.Context, rev string, opts ...Sho done <- struct{}{} }() - args := []string{"show", "--name-status", "--pretty=format:''"} - args = append(args, opt.Args...) - args = append(args, "--end-of-options", rev) + args := []string{"show", "--name-status", "--pretty=format:''", "--end-of-options", rev} err := pipe(ctx, r.path, args, opt.Envs, w) _ = w.Close() // Close writer to exit parsing goroutine @@ -474,9 +461,7 @@ func (r *Repository) RevParse(ctx context.Context, rev string, opts ...RevParseO opt = opts[0] } - args := []string{"rev-parse"} - args = append(args, opt.Args...) - args = append(args, rev) + args := []string{"rev-parse", rev} commitID, err := exec(ctx, r.path, args, opt.Envs) if err != nil { @@ -515,8 +500,7 @@ func (r *Repository) CountObjects(ctx context.Context, opts ...CountObjectsOptio opt = opts[0] } - args := []string{"count-objects", "-v"} - args = append(args, opt.Args...) + args := []string{"count-objects", "-v", "--end-of-options"} stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { @@ -569,8 +553,7 @@ func (r *Repository) Fsck(ctx context.Context, opts ...FsckOptions) error { opt = opts[0] } - args := []string{"fsck"} - args = append(args, opt.Args...) + args := []string{"fsck", "--end-of-options"} _, err := exec(ctx, r.path, args, opt.Envs) return err } diff --git a/repo_blame.go b/repo_blame.go index 269279f3..5f899d4b 100644 --- a/repo_blame.go +++ b/repo_blame.go @@ -24,9 +24,7 @@ func (r *Repository) Blame(ctx context.Context, rev, file string, opts ...BlameO opt = opts[0] } - args := []string{"blame"} - args = append(args, opt.Args...) - args = append(args, "-l", "-s", rev, "--", file) + args := []string{"blame", "-l", "-s", rev, "--", file} stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { diff --git a/repo_commit.go b/repo_commit.go index 45c38fd9..aae97cbc 100644 --- a/repo_commit.go +++ b/repo_commit.go @@ -94,9 +94,7 @@ func (r *Repository) CatFileCommit(ctx context.Context, rev string, opts ...CatF return nil, err } - args := []string{"cat-file"} - args = append(args, opt.Args...) - args = append(args, "commit", commitID) + args := []string{"cat-file", "commit", commitID} stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { @@ -129,9 +127,7 @@ func (r *Repository) CatFileType(ctx context.Context, rev string, opts ...CatFil opt = opts[0] } - args := []string{"cat-file"} - args = append(args, opt.Args...) - args = append(args, "-t", rev) + args := []string{"cat-file", "-t", rev} typ, err := exec(ctx, r.path, args, opt.Envs) if err != nil { @@ -193,9 +189,7 @@ func (r *Repository) Log(ctx context.Context, rev string, opts ...LogOptions) ([ opt = opts[0] } - args := []string{"log"} - args = append(args, opt.Args...) - args = append(args, "--pretty="+LogFormatHashOnly) + args := []string{"log", "--pretty=" + LogFormatHashOnly} if opt.MaxCount > 0 { args = append(args, "--max-count="+strconv.Itoa(opt.MaxCount)) } @@ -358,9 +352,7 @@ func (r *Repository) DiffNameOnly(ctx context.Context, base, head string, opts . opt = opts[0] } - args := []string{"diff"} - args = append(args, opt.Args...) - args = append(args, "--name-only", "--end-of-options") + args := []string{"diff", "--name-only", "--end-of-options"} if opt.NeedsMergeBase { args = append(args, base+"..."+head) } else { @@ -410,9 +402,7 @@ func (r *Repository) RevListCount(ctx context.Context, refspecs []string, opts . return 0, errors.New("must have at least one refspec") } - args := []string{"rev-list"} - args = append(args, opt.Args...) - args = append(args, "--count", "--end-of-options") + args := []string{"rev-list", "--count", "--end-of-options"} args = append(args, refspecs...) args = append(args, "--") if opt.Path != "" { @@ -449,9 +439,7 @@ func (r *Repository) RevList(ctx context.Context, refspecs []string, opts ...Rev return nil, errors.New("must have at least one refspec") } - args := []string{"rev-list"} - args = append(args, opt.Args...) - args = append(args, "--end-of-options") + args := []string{"rev-list", "--end-of-options"} args = append(args, refspecs...) args = append(args, "--") if opt.Path != "" { @@ -481,9 +469,7 @@ func (r *Repository) LatestCommitTime(ctx context.Context, opts ...LatestCommitT opt = opts[0] } - args := []string{"for-each-ref"} - args = append(args, opt.Args...) - args = append(args, "--count=1", "--sort=-committerdate", "--format=%(committerdate:iso8601)") + args := []string{"for-each-ref", "--count=1", "--sort=-committerdate", "--format=%(committerdate:iso8601)", "--end-of-options"} if opt.Branch != "" { args = append(args, RefsHeads+opt.Branch) } diff --git a/repo_diff.go b/repo_diff.go index 368a861d..388391aa 100644 --- a/repo_diff.go +++ b/repo_diff.go @@ -17,7 +17,7 @@ type DiffOptions struct { // The commit ID to used for computing diff between a range of commits (base, // revision]. When not set, only computes diff for a single commit at revision. Base string - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -37,22 +37,16 @@ func (r *Repository) Diff(ctx context.Context, rev string, maxFiles, maxFileLine if opt.Base == "" { // First commit of repository if commit.ParentsCount() == 0 { - args = []string{"show"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "--end-of-options", rev) + args = []string{"show", "--full-index", "--end-of-options", rev} } else { c, err := commit.Parent(ctx, 0) if err != nil { return nil, err } - args = []string{"diff"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "-M", c.ID.String(), "--end-of-options", rev) + args = []string{"diff", "--full-index", "-M", c.ID.String(), "--end-of-options", rev} } } else { - args = []string{"diff"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "-M", opt.Base, "--end-of-options", rev) + args = []string{"diff", "--full-index", "-M", opt.Base, "--end-of-options", rev} } stdout, w := io.Pipe() @@ -81,7 +75,7 @@ const ( // // Docs: https://git-scm.com/docs/git-format-patch type RawDiffOptions struct { - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -102,31 +96,23 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo switch diffType { case RawDiffNormal: if commit.ParentsCount() == 0 { - args = []string{"show"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "--end-of-options", rev) + args = []string{"show", "--full-index", "--end-of-options", rev} } else { c, err := commit.Parent(ctx, 0) if err != nil { return err } - args = []string{"diff"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "-M", c.ID.String(), "--end-of-options", rev) + args = []string{"diff", "--full-index", "-M", c.ID.String(), "--end-of-options", rev} } case RawDiffPatch: if commit.ParentsCount() == 0 { - args = []string{"format-patch"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "--no-signoff", "--no-signature", "--stdout", "--root", "--end-of-options", rev) + args = []string{"format-patch", "--full-index", "--no-signoff", "--no-signature", "--stdout", "--root", "--end-of-options", rev} } else { c, err := commit.Parent(ctx, 0) if err != nil { return err } - args = []string{"format-patch"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "--no-signoff", "--no-signature", "--stdout", "--end-of-options", rev+"..."+c.ID.String()) + args = []string{"format-patch", "--full-index", "--no-signoff", "--no-signature", "--stdout", "--end-of-options", rev + "..." + c.ID.String()} } default: return fmt.Errorf("invalid diffType: %s", diffType) @@ -140,7 +126,7 @@ func (r *Repository) RawDiff(ctx context.Context, rev string, diffType RawDiffFo // DiffBinaryOptions contains optional arguments for producing binary patch. type DiffBinaryOptions struct { - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -152,9 +138,6 @@ func (r *Repository) DiffBinary(ctx context.Context, base, head string, opts ... opt = opts[0] } - args := []string{"diff"} - args = append(args, opt.Args...) - args = append(args, "--full-index", "--binary", "--end-of-options", base, head) - + args := []string{"diff", "--full-index", "--binary", "--end-of-options", base, head} return exec(ctx, r.path, args, opt.Envs) } diff --git a/repo_grep.go b/repo_grep.go index e40a1e4b..7537685a 100644 --- a/repo_grep.go +++ b/repo_grep.go @@ -25,7 +25,7 @@ type GrepOptions struct { WordRegexp bool // Whether use extended regular expressions. ExtendedRegexp bool - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -79,7 +79,6 @@ func (r *Repository) Grep(ctx context.Context, pattern string, opts ...GrepOptio } args := []string{"grep"} - args = append(args, opt.Args...) // Display full-name, line number and column number args = append(args, "--full-name", "--line-number", "--column") if opt.IgnoreCase { diff --git a/repo_pull.go b/repo_pull.go index 33d2be76..5b067030 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -26,7 +26,6 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M } args := []string{"merge-base"} - args = append(args, opt.Args...) args = append(args, "--end-of-options", base, head) stdout, err := exec(ctx, r.path, args, opt.Envs) diff --git a/repo_reference.go b/repo_reference.go index 67e97b03..590d01bd 100644 --- a/repo_reference.go +++ b/repo_reference.go @@ -37,7 +37,7 @@ type Reference struct { // // Docs: https://git-scm.com/docs/git-show-ref#Documentation/git-show-ref.txt---verify type ShowRefVerifyOptions struct { - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -51,9 +51,7 @@ func (r *Repository) ShowRefVerify(ctx context.Context, ref string, opts ...Show opt = opts[0] } - args := []string{"show-ref", "--verify"} - args = append(args, opt.Args...) - args = append(args, "--end-of-options", ref) + args := []string{"show-ref", "--verify", "--end-of-options", ref} stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { @@ -103,7 +101,7 @@ type SymbolicRefOptions struct { // The name of the reference, e.g. "refs/heads/master". When set, it will be // used to update the symbolic ref. Ref string - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -117,7 +115,6 @@ func (r *Repository) SymbolicRef(ctx context.Context, opts ...SymbolicRefOptions } args := []string{"symbolic-ref"} - args = append(args, opt.Args...) if opt.Name == "" { opt.Name = "HEAD" } @@ -143,7 +140,7 @@ type ShowRefOptions struct { Tags bool // The list of patterns to filter results. Patterns []string - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -155,7 +152,6 @@ func (r *Repository) ShowRef(ctx context.Context, opts ...ShowRefOptions) ([]*Re } args := []string{"show-ref"} - args = append(args, opt.Args...) if opt.Heads { args = append(args, "--heads") } @@ -207,7 +203,7 @@ func (r *Repository) Branches(ctx context.Context) ([]string, error) { type DeleteBranchOptions struct { // Indicates whether to force delete the branch. Force bool - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -219,7 +215,6 @@ func (r *Repository) DeleteBranch(ctx context.Context, name string, opts ...Dele } args := []string{"branch"} - args = append(args, opt.Args...) if opt.Force { args = append(args, "-D") } else { diff --git a/repo_remote.go b/repo_remote.go index 50b401e1..bfeac2dc 100644 --- a/repo_remote.go +++ b/repo_remote.go @@ -23,7 +23,7 @@ type LsRemoteOptions struct { Refs bool // The list of patterns to filter results. Patterns []string - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -35,7 +35,6 @@ func LsRemote(ctx context.Context, url string, opts ...LsRemoteOptions) ([]*Refe } args := []string{"ls-remote", "--quiet"} - args = append(args, opt.Args...) if opt.Heads { args = append(args, "--heads") } @@ -89,7 +88,7 @@ type RemoteAddOptions struct { Fetch bool // Indicates whether to add remote as mirror with --mirror=fetch. MirrorFetch bool - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -101,7 +100,6 @@ func (r *Repository) RemoteAdd(ctx context.Context, name, url string, opts ...Re } args := []string{"remote", "add"} - args = append(args, opt.Args...) if opt.Fetch { args = append(args, "-f") } @@ -119,7 +117,7 @@ func (r *Repository) RemoteAdd(ctx context.Context, name, url string, opts ...Re // // Docs: https://git-scm.com/docs/git-remote#Documentation/git-remote.txt-emremoveem type RemoteRemoveOptions struct { - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -130,9 +128,7 @@ func (r *Repository) RemoteRemove(ctx context.Context, name string, opts ...Remo opt = opts[0] } - args := []string{"remote", "remove"} - args = append(args, opt.Args...) - args = append(args, "--end-of-options", name) + args := []string{"remote", "remove", "--end-of-options", name} _, err := exec(ctx, r.path, args, opt.Envs) if err != nil { @@ -149,7 +145,7 @@ func (r *Repository) RemoteRemove(ctx context.Context, name string, opts ...Remo // / // Docs: https://git-scm.com/docs/git-remote#_commands type RemotesOptions struct { - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -160,8 +156,7 @@ func (r *Repository) Remotes(ctx context.Context, opts ...RemotesOptions) ([]str opt = opts[0] } - args := []string{"remote"} - args = append(args, opt.Args...) + args := []string{"remote", "--end-of-options"} stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { @@ -181,7 +176,7 @@ type RemoteGetURLOptions struct { // Indicates whether to get all URLs, including lists that are not part of main // URLs. This option is independent of the Push option. All bool - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -193,7 +188,6 @@ func (r *Repository) RemoteGetURL(ctx context.Context, name string, opts ...Remo } args := []string{"remote", "get-url"} - args = append(args, opt.Args...) if opt.Push { args = append(args, "--push") } @@ -218,7 +212,7 @@ type RemoteSetURLOptions struct { Push bool // The regex to match existing URLs to replace (instead of first). Regex string - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -231,7 +225,6 @@ func (r *Repository) RemoteSetURL(ctx context.Context, name, newurl string, opts } args := []string{"remote", "set-url"} - args = append(args, opt.Args...) if opt.Push { args = append(args, "--push") } @@ -259,7 +252,7 @@ func (r *Repository) RemoteSetURL(ctx context.Context, name, newurl string, opts type RemoteSetURLAddOptions struct { // Indicates whether to get push URLs instead of fetch URLs. Push bool - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -271,9 +264,7 @@ func (r *Repository) RemoteSetURLAdd(ctx context.Context, name, newurl string, o opt = opts[0] } - args := []string{"remote", "set-url"} - args = append(args, opt.Args...) - args = append(args, "--add") + args := []string{"remote", "set-url", "--add"} if opt.Push { args = append(args, "--push") } @@ -293,7 +284,7 @@ func (r *Repository) RemoteSetURLAdd(ctx context.Context, name, newurl string, o type RemoteSetURLDeleteOptions struct { // Indicates whether to get push URLs instead of fetch URLs. Push bool - // The additional options to be passed to the underlying git. + // The additional options to be passed to the underlying Git. CommandOptions } @@ -305,9 +296,7 @@ func (r *Repository) RemoteSetURLDelete(ctx context.Context, name, regex string, opt = opts[0] } - args := []string{"remote", "set-url"} - args = append(args, opt.Args...) - args = append(args, "--delete") + args := []string{"remote", "set-url", "--delete"} if opt.Push { args = append(args, "--push") } diff --git a/repo_tag.go b/repo_tag.go index 2637514a..6571db5c 100644 --- a/repo_tag.go +++ b/repo_tag.go @@ -156,12 +156,12 @@ func (r *Repository) Tags(ctx context.Context, opts ...TagsOptions) ([]string, e } args := []string{"tag", "--list"} - args = append(args, opt.Args...) if opt.SortKey != "" { args = append(args, "--sort="+opt.SortKey) } else { args = append(args, "--sort=-creatordate") } + args = append(args, "--end-of-options") if opt.Pattern != "" { args = append(args, opt.Pattern) } @@ -199,7 +199,6 @@ func (r *Repository) CreateTag(ctx context.Context, name, rev string, opts ...Cr } args := []string{"tag"} - args = append(args, opt.Args...) var envs []string if opt.Annotated { @@ -234,9 +233,7 @@ func (r *Repository) DeleteTag(ctx context.Context, name string, opts ...DeleteT opt = opts[0] } - args := []string{"tag", "--delete"} - args = append(args, opt.Args...) - args = append(args, "--end-of-options", name) + args := []string{"tag", "--delete", "--end-of-options", name} _, err := exec(ctx, r.path, args, opt.Envs) return err diff --git a/repo_tree.go b/repo_tree.go index ea8f82ca..2c77a6e8 100644 --- a/repo_tree.go +++ b/repo_tree.go @@ -132,11 +132,10 @@ func (r *Repository) LsTree(ctx context.Context, treeID string, opts ...LsTreeOp } args := []string{"ls-tree"} - args = append(args, opt.Args...) if opt.Verbatim { args = append(args, "-z") } - args = append(args, treeID) + args = append(args, "--end-of-options", treeID) stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { diff --git a/server.go b/server.go index 9bb8f142..901f966d 100755 --- a/server.go +++ b/server.go @@ -29,10 +29,10 @@ func UpdateServerInfo(ctx context.Context, path string, opts ...UpdateServerInfo } args := []string{"update-server-info"} - args = append(args, opt.Args...) if opt.Force { args = append(args, "--force") } + args = append(args, "--end-of-options") _, err := exec(ctx, path, args, opt.Envs) return err } @@ -58,14 +58,13 @@ func ReceivePack(ctx context.Context, path string, opts ...ReceivePackOptions) ( } args := []string{"receive-pack"} - args = append(args, opt.Args...) if opt.Quiet { args = append(args, "--quiet") } if opt.HTTPBackendInfoRefs { args = append(args, "--http-backend-info-refs") } - args = append(args, ".") + args = append(args, "--end-of-options", ".") return exec(ctx, path, args, opt.Envs) } @@ -97,7 +96,6 @@ func UploadPack(ctx context.Context, path string, opts ...UploadPackOptions) ([] } args := []string{"upload-pack"} - args = append(args, opt.Args...) if opt.StatelessRPC { args = append(args, "--stateless-rpc") } @@ -110,6 +108,6 @@ func UploadPack(ctx context.Context, path string, opts ...UploadPackOptions) ([] if opt.HTTPBackendInfoRefs { args = append(args, "--http-backend-info-refs") } - args = append(args, ".") + args = append(args, "--end-of-options", ".") return exec(ctx, path, args, opt.Envs) } From 193aecab96a94ed8879518291288e92078619f29 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sun, 15 Feb 2026 12:06:19 -0500 Subject: [PATCH 11/14] 111 --- command.go | 1 - repo_blame.go | 1 - repo_commit.go | 2 -- repo_grep.go | 1 - repo_pull.go | 6 +++--- repo_pull_test.go | 24 +++++++++++------------- tree_entry_test.go | 34 ++++++++++++++++++++++++++++++++++ 7 files changed, 48 insertions(+), 21 deletions(-) diff --git a/command.go b/command.go index d7548370..88a5538b 100644 --- a/command.go +++ b/command.go @@ -19,7 +19,6 @@ import ( // CommandOptions contains additional options for running a Git command. type CommandOptions struct { - // The additional environment variables to be passed to the underlying Git. Envs []string } diff --git a/repo_blame.go b/repo_blame.go index 5f899d4b..b10a0d44 100644 --- a/repo_blame.go +++ b/repo_blame.go @@ -25,7 +25,6 @@ func (r *Repository) Blame(ctx context.Context, rev, file string, opts ...BlameO } args := []string{"blame", "-l", "-s", rev, "--", file} - stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err diff --git a/repo_commit.go b/repo_commit.go index aae97cbc..4ffced0f 100644 --- a/repo_commit.go +++ b/repo_commit.go @@ -95,7 +95,6 @@ func (r *Repository) CatFileCommit(ctx context.Context, rev string, opts ...CatF } args := []string{"cat-file", "commit", commitID} - stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err @@ -128,7 +127,6 @@ func (r *Repository) CatFileType(ctx context.Context, rev string, opts ...CatFil } args := []string{"cat-file", "-t", rev} - typ, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return "", err diff --git a/repo_grep.go b/repo_grep.go index 7537685a..3da197c5 100644 --- a/repo_grep.go +++ b/repo_grep.go @@ -79,7 +79,6 @@ func (r *Repository) Grep(ctx context.Context, pattern string, opts ...GrepOptio } args := []string{"grep"} - // Display full-name, line number and column number args = append(args, "--full-name", "--line-number", "--column") if opt.IgnoreCase { args = append(args, "--ignore-case") diff --git a/repo_pull.go b/repo_pull.go index 5b067030..def9350c 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -6,6 +6,7 @@ package git import ( "context" + "fmt" "strings" ) @@ -25,11 +26,10 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M opt = opts[0] } - args := []string{"merge-base"} - args = append(args, "--end-of-options", base, head) - + args := []string{"merge-base", "--end-of-options", base, head} stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { + fmt.Printf("%+v\n", err) if isExitStatus(err, 1) { return "", ErrNoMergeBase } diff --git a/repo_pull_test.go b/repo_pull_test.go index 27a6a637..f4d70226 100644 --- a/repo_pull_test.go +++ b/repo_pull_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestRepository_MergeBase(t *testing.T) { @@ -25,28 +26,25 @@ func TestRepository_MergeBase(t *testing.T) { tests := []struct { base string head string - opt MergeBaseOptions - expMergeBase string + opt MergeBaseOptions + wantMergeBase string }{ { - base: "4e59b72440188e7c2578299fc28ea425fbe9aece", - head: "0eedd79eba4394bbef888c804e899731644367fe", - expMergeBase: "4e59b72440188e7c2578299fc28ea425fbe9aece", + base: "4e59b72440188e7c2578299fc28ea425fbe9aece", + head: "0eedd79eba4394bbef888c804e899731644367fe", + wantMergeBase: "4e59b72440188e7c2578299fc28ea425fbe9aece", }, { - base: "master", - head: "release-1.0", - expMergeBase: "0eedd79eba4394bbef888c804e899731644367fe", + base: "master", + head: "release-1.0", + wantMergeBase: "0eedd79eba4394bbef888c804e899731644367fe", }, } for _, test := range tests { t.Run("", func(t *testing.T) { mb, err := testrepo.MergeBase(ctx, test.base, test.head, test.opt) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, test.expMergeBase, mb) + require.NoError(t, err) + assert.Equal(t, test.wantMergeBase, mb) }) } } diff --git a/tree_entry_test.go b/tree_entry_test.go index 0d305b21..3f1ff9fd 100644 --- a/tree_entry_test.go +++ b/tree_entry_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTreeEntry(t *testing.T) { @@ -31,6 +32,39 @@ func TestTreeEntry(t *testing.T) { assert.Equal(t, "go.mod", e.Name()) } +func TestTreeEntry_Size(t *testing.T) { + ctx := context.Background() + tree, err := testrepo.LsTree(ctx, "0eedd79eba4394bbef888c804e899731644367fe") + require.NoError(t, err) + + es, err := tree.Entries(ctx) + require.NoError(t, err) + + t.Run("blob", func(t *testing.T) { + var entry *TreeEntry + for _, e := range es { + if e.Name() == "README.txt" { + entry = e + break + } + } + require.NotNil(t, entry, "entry README.txt not found") + assert.Equal(t, int64(795), entry.Size(ctx)) + }) + + t.Run("tree returns zero", func(t *testing.T) { + var entry *TreeEntry + for _, e := range es { + if e.IsTree() { + entry = e + break + } + } + require.NotNil(t, entry, "tree entry not found") + assert.Equal(t, int64(0), entry.Size(ctx)) + }) +} + func TestEntries_Sort(t *testing.T) { ctx := context.Background() tree, err := testrepo.LsTree(ctx, "0eedd79eba4394bbef888c804e899731644367fe") From daa6557f9d4770177a2473bc9ec35061946a793c Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sun, 15 Feb 2026 12:10:18 -0500 Subject: [PATCH 12/14] 111 --- repo_pull.go | 2 -- repo_reference.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/repo_pull.go b/repo_pull.go index def9350c..b83d7083 100644 --- a/repo_pull.go +++ b/repo_pull.go @@ -6,7 +6,6 @@ package git import ( "context" - "fmt" "strings" ) @@ -29,7 +28,6 @@ func (r *Repository) MergeBase(ctx context.Context, base, head string, opts ...M args := []string{"merge-base", "--end-of-options", base, head} stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { - fmt.Printf("%+v\n", err) if isExitStatus(err, 1) { return "", ErrNoMergeBase } diff --git a/repo_reference.go b/repo_reference.go index 590d01bd..a3ad2777 100644 --- a/repo_reference.go +++ b/repo_reference.go @@ -52,7 +52,6 @@ func (r *Repository) ShowRefVerify(ctx context.Context, ref string, opts ...Show } args := []string{"show-ref", "--verify", "--end-of-options", ref} - stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "not a valid ref") { @@ -221,7 +220,6 @@ func (r *Repository) DeleteBranch(ctx context.Context, name string, opts ...Dele args = append(args, "-d") } args = append(args, "--end-of-options", name) - _, err := exec(ctx, r.path, args, opt.Envs) return err } From 1fc401dff2804550ffd55806655f7aeddac095b4 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sun, 15 Feb 2026 14:00:22 -0500 Subject: [PATCH 13/14] 123 --- repo_pull_test.go | 4 ++-- repo_remote.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/repo_pull_test.go b/repo_pull_test.go index f4d70226..737c6c8e 100644 --- a/repo_pull_test.go +++ b/repo_pull_test.go @@ -24,8 +24,8 @@ func TestRepository_MergeBase(t *testing.T) { }) tests := []struct { - base string - head string + base string + head string opt MergeBaseOptions wantMergeBase string }{ diff --git a/repo_remote.go b/repo_remote.go index bfeac2dc..ca3ba319 100644 --- a/repo_remote.go +++ b/repo_remote.go @@ -27,7 +27,7 @@ type LsRemoteOptions struct { CommandOptions } -// LsRemote returns a list references in the remote repository. +// LsRemote returns a list of references in the remote repository. func LsRemote(ctx context.Context, url string, opts ...LsRemoteOptions) ([]*Reference, error) { var opt LsRemoteOptions if len(opts) > 0 { From 9511a0857505e59acd0854dce7b97232340c6af2 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sun, 15 Feb 2026 14:03:40 -0500 Subject: [PATCH 14/14] 111 --- repo_remote.go | 2 -- repo_tag.go | 3 --- 2 files changed, 5 deletions(-) diff --git a/repo_remote.go b/repo_remote.go index ca3ba319..e5438d09 100644 --- a/repo_remote.go +++ b/repo_remote.go @@ -129,7 +129,6 @@ func (r *Repository) RemoteRemove(ctx context.Context, name string, opts ...Remo } args := []string{"remote", "remove", "--end-of-options", name} - _, err := exec(ctx, r.path, args, opt.Envs) if err != nil { if strings.Contains(err.Error(), "error: No such remote") || @@ -157,7 +156,6 @@ func (r *Repository) Remotes(ctx context.Context, opts ...RemotesOptions) ([]str } args := []string{"remote", "--end-of-options"} - stdout, err := exec(ctx, r.path, args, opt.Envs) if err != nil { return nil, err diff --git a/repo_tag.go b/repo_tag.go index 6571db5c..9a57fb3e 100644 --- a/repo_tag.go +++ b/repo_tag.go @@ -199,7 +199,6 @@ func (r *Repository) CreateTag(ctx context.Context, name, rev string, opts ...Cr } args := []string{"tag"} - var envs []string if opt.Annotated { args = append(args, "-a", name) @@ -212,7 +211,6 @@ func (r *Repository) CreateTag(ctx context.Context, name, rev string, opts ...Cr args = append(args, "--end-of-options", name) } args = append(args, rev) - envs = append(envs, opt.Envs...) _, err := exec(ctx, r.path, args, envs) return err @@ -234,7 +232,6 @@ func (r *Repository) DeleteTag(ctx context.Context, name string, opts ...DeleteT } args := []string{"tag", "--delete", "--end-of-options", name} - _, err := exec(ctx, r.path, args, opt.Envs) return err }