diff --git a/git/operations.go b/git/operations.go index a49cb71a5..dd8277ee8 100644 --- a/git/operations.go +++ b/git/operations.go @@ -3,15 +3,14 @@ package git import ( "bufio" "bytes" + "context" "encoding/json" "fmt" "io" - "io/ioutil" "os" "os/exec" "strings" - - "context" + "sync" "github.com/pkg/errors" ) @@ -303,7 +302,7 @@ func changed(ctx context.Context, workingDir, ref string, subPaths []string) ([] } // traceGitCommand returns a log line that can be useful when debugging and developing git activity -func traceGitCommand(args []string, config gitCmdConfig, stdout string, stderr string) string { +func traceGitCommand(args []string, config gitCmdConfig, stdOutAndStdErr string) string { for _, exemptedCommand := range exemptedTraceCommands { if exemptedCommand == args[0] { return "" @@ -318,19 +317,34 @@ func traceGitCommand(args []string, config gitCmdConfig, stdout string, stderr s } command := `git ` + strings.Join(args, " ") - out := prepare(stdout) - err := prepare(stderr) + out := prepare(stdOutAndStdErr) return fmt.Sprintf( - "TRACE: command=%q out=%q err=%q dir=%q env=%q", + "TRACE: command=%q out=%q dir=%q env=%q", command, out, - err, config.dir, strings.Join(config.env, ","), ) } +type threadSafeBuffer struct { + bytes.Buffer + sync.Mutex +} + +func (b *threadSafeBuffer) Write(p []byte) (n int, err error) { + b.Lock() + defer b.Unlock() + return b.Buffer.Write(p) +} + +func (b *threadSafeBuffer) ReadFrom(r io.Reader) (n int64, err error) { + b.Lock() + defer b.Unlock() + return b.Buffer.ReadFrom(r) +} + // execGitCmd runs a `git` command with the supplied arguments. func execGitCmd(ctx context.Context, args []string, config gitCmdConfig) error { c := exec.CommandContext(ctx, "git", args...) @@ -339,30 +353,26 @@ func execGitCmd(ctx context.Context, args []string, config gitCmdConfig) error { c.Dir = config.dir } c.Env = append(env(), config.env...) - c.Stdout = ioutil.Discard + stdOutAndStdErr := &threadSafeBuffer{} + c.Stdout = stdOutAndStdErr + c.Stderr = stdOutAndStdErr if config.out != nil { - c.Stdout = config.out - } - errOut := &bytes.Buffer{} - c.Stderr = errOut - - traceStdout := &bytes.Buffer{} - traceStderr := &bytes.Buffer{} - if trace { - c.Stdout = io.MultiWriter(c.Stdout, traceStdout) - c.Stderr = io.MultiWriter(c.Stderr, traceStderr) + c.Stdout = io.MultiWriter(c.Stdout, config.out) } err := c.Run() if err != nil { - msg := findErrorMessage(errOut) - if msg != "" { - err = errors.New(msg) + if len(stdOutAndStdErr.Bytes()) > 0 { + err = errors.New(stdOutAndStdErr.String()) + msg := findErrorMessage(stdOutAndStdErr) + if msg != "" { + err = fmt.Errorf("%s, full output:\n %s", msg, err.Error()) + } } } if trace { - if traceCommand := traceGitCommand(args, config, traceStdout.String(), traceStderr.String()); traceCommand != "" { + if traceCommand := traceGitCommand(args, config, stdOutAndStdErr.String()); traceCommand != "" { println(traceCommand) } } diff --git a/git/operations_test.go b/git/operations_test.go index 74fcedae7..113af66f4 100644 --- a/git/operations_test.go +++ b/git/operations_test.go @@ -316,7 +316,7 @@ func TestTraceGitCommand(t *testing.T) { dir: "/tmp/flux-working628880789", }, }, - expected: `TRACE: command="git clone --branch master /tmp/flux-gitclone239583443 /tmp/flux-working628880789" out="" err="" dir="/tmp/flux-working628880789" env=""`, + expected: `TRACE: command="git clone --branch master /tmp/flux-gitclone239583443 /tmp/flux-working628880789" out="" dir="/tmp/flux-working628880789" env=""`, }, { name: "git rev-list", @@ -333,7 +333,7 @@ func TestTraceGitCommand(t *testing.T) { dir: "/tmp/flux-gitclone239583443", }, }, - expected: `TRACE: command="git rev-list --max-count 1 flux-sync --" out="b9d6a543acf8085ff6bed23fac17f8dc71bfcb66" err="" dir="/tmp/flux-gitclone239583443" env=""`, + expected: `TRACE: command="git rev-list --max-count 1 flux-sync --" out="b9d6a543acf8085ff6bed23fac17f8dc71bfcb66" dir="/tmp/flux-gitclone239583443" env=""`, }, { name: "git config email", @@ -347,7 +347,7 @@ func TestTraceGitCommand(t *testing.T) { dir: "/tmp/flux-working056923691", }, }, - expected: `TRACE: command="git config user.email support@weave.works" out="" err="" dir="/tmp/flux-working056923691" env=""`, + expected: `TRACE: command="git config user.email support@weave.works" out="" dir="/tmp/flux-working056923691" env=""`, }, { name: "git notes", @@ -363,7 +363,7 @@ func TestTraceGitCommand(t *testing.T) { }, out: "refs/notes/flux", }, - expected: `TRACE: command="git notes --ref flux get-ref" out="refs/notes/flux" err="" dir="/tmp/flux-working647148942" env=""`, + expected: `TRACE: command="git notes --ref flux get-ref" out="refs/notes/flux" dir="/tmp/flux-working647148942" env=""`, }, } for _, example := range examples { @@ -371,7 +371,6 @@ func TestTraceGitCommand(t *testing.T) { example.input.args, example.input.config, example.input.out, - example.input.err, ) assert.Equal(t, example.expected, actual) }