Merge pull request #1180 from tamird/errors-with-comands

gps: include output in error

internal/gps/cmd.go

@@ -0,0 +1,17 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package gps
+
+func (c cmd) Args() []string {
+	return c.Cmd.Args
+}
+
+func (c cmd) SetDir(dir string) {
+	c.Cmd.Dir = dir
+}
+
+func (c cmd) SetEnv(env []string) {
+	c.Cmd.Env = env
+}

internal/gps/cmd_unix.go

@@ -21,41 +21,33 @@ type cmd struct {
 	ctx context.Context
 	// cancel is called when the graceful shutdown timeout expires.
 	cancel context.CancelFunc
-	cmd    *exec.Cmd
+	Cmd    *exec.Cmd
 }
 
 func commandContext(ctx context.Context, name string, arg ...string) cmd {
 	// Grab the caller's context and pass a derived one to CommandContext.
 	c := cmd{ctx: ctx}
 	ctx, cancel := context.WithCancel(ctx)
-	c.cmd = exec.CommandContext(ctx, name, arg...)
+	c.Cmd = exec.CommandContext(ctx, name, arg...)
 	c.cancel = cancel
 	return c
 }
 
-func (c cmd) Args() []string {
-	return c.cmd.Args
-}
-
-func (c cmd) SetDir(dir string) {
-	c.cmd.Dir = dir
-}
-
 // CombinedOutput is like (*os/exec.Cmd).CombinedOutput except that it
 // terminates subprocesses gently (via os.Interrupt), but resorts to Kill if
 // the subprocess fails to exit after 1 minute.
 func (c cmd) CombinedOutput() ([]byte, error) {
 	// Adapted from (*os/exec.Cmd).CombinedOutput
-	if c.cmd.Stdout != nil {
+	if c.Cmd.Stdout != nil {
 		return nil, errors.New("exec: Stdout already set")
 	}
-	if c.cmd.Stderr != nil {
+	if c.Cmd.Stderr != nil {
 		return nil, errors.New("exec: Stderr already set")
 	}
 	var b bytes.Buffer
-	c.cmd.Stdout = &b
-	c.cmd.Stderr = &b
-	if err := c.cmd.Start(); err != nil {
+	c.Cmd.Stdout = &b
+	c.Cmd.Stderr = &b
+	if err := c.Cmd.Start(); err != nil {
 		return nil, err
 	}
 
@@ -71,7 +63,7 @@ func (c cmd) CombinedOutput() ([]byte, error) {
 	go func() {
 		select {
 		case <-c.ctx.Done():
-			if err := c.cmd.Process.Signal(os.Interrupt); err != nil {
+			if err := c.Cmd.Process.Signal(os.Interrupt); err != nil {
 				// If an error comes back from attempting to signal, proceed
 				// immediately to hard kill.
 				c.cancel()
@@ -82,7 +74,7 @@ func (c cmd) CombinedOutput() ([]byte, error) {
 		}
 	}()
 
-	if err := c.cmd.Wait(); err != nil {
+	if err := c.Cmd.Wait(); err != nil {
 		return nil, err
 	}
 	return b.Bytes(), nil

internal/gps/cmd_windows.go

@@ -16,11 +16,3 @@ type cmd struct {
 func commandContext(ctx context.Context, name string, arg ...string) cmd {
 	return cmd{Cmd: exec.CommandContext(ctx, name, arg...)}
 }
-
-func (c cmd) Args() []string {
-	return c.Cmd.Args
-}
-
-func (c cmd) SetDir(dir string) {
-	c.Cmd.Dir = dir
-}

internal/gps/vcs_source.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 
@@ -142,8 +141,8 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin
 	defer fs.RenameWithFallback(bak, idx)
 
 	{
-		cmd := exec.CommandContext(ctx, "git", "read-tree", rev.String())
-		cmd.Dir = r.LocalPath()
+		cmd := commandContext(ctx, "git", "read-tree", rev.String())
+		cmd.SetDir(r.LocalPath())
 		if out, err := cmd.CombinedOutput(); err != nil {
 			return errors.Wrap(err, string(out))
 		}
@@ -161,8 +160,8 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin
 	// down, the sparse checkout controls, as well as restore the original
 	// index and HEAD.
 	{
-		cmd := exec.CommandContext(ctx, "git", "checkout-index", "-a", "--prefix="+to)
-		cmd.Dir = r.LocalPath()
+		cmd := commandContext(ctx, "git", "checkout-index", "-a", "--prefix="+to)
+		cmd.SetDir(r.LocalPath())
 		if out, err := cmd.CombinedOutput(); err != nil {
 			return errors.Wrap(err, string(out))
 		}
@@ -174,12 +173,12 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin
 func (s *gitSource) listVersions(ctx context.Context) (vlist []PairedVersion, err error) {
 	r := s.repo
 
-	cmd := exec.CommandContext(ctx, "git", "ls-remote", r.Remote())
+	cmd := commandContext(ctx, "git", "ls-remote", r.Remote())
 	// Ensure no prompting for PWs
-	cmd.Env = append([]string{"GIT_ASKPASS=", "GIT_TERMINAL_PROMPT=0"}, os.Environ()...)
+	cmd.SetEnv(append([]string{"GIT_ASKPASS=", "GIT_TERMINAL_PROMPT=0"}, os.Environ()...))
 	out, err := cmd.CombinedOutput()
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, string(out))
 	}
 
 	all := bytes.Split(bytes.TrimSpace(out), []byte("\n"))
@@ -396,8 +395,8 @@ func (s *bzrSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
 	}
 
 	// Now, list all the tags
-	tagsCmd := exec.CommandContext(ctx, "bzr", "tags", "--show-ids", "-v")
-	tagsCmd.Dir = r.LocalPath()
+	tagsCmd := commandContext(ctx, "bzr", "tags", "--show-ids", "-v")
+	tagsCmd.SetDir(r.LocalPath())
 	out, err := tagsCmd.CombinedOutput()
 	if err != nil {
 		return nil, errors.Wrap(err, string(out))
@@ -405,8 +404,8 @@ func (s *bzrSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
 
 	all := bytes.Split(bytes.TrimSpace(out), []byte("\n"))
 
-	viCmd := exec.CommandContext(ctx, "bzr", "version-info", "--custom", "--template={revision_id}", "--revision=branch:.")
-	viCmd.Dir = r.LocalPath()
+	viCmd := commandContext(ctx, "bzr", "version-info", "--custom", "--template={revision_id}", "--revision=branch:.")
+	viCmd.SetDir(r.LocalPath())
 	branchrev, err := viCmd.CombinedOutput()
 	if err != nil {
 		return nil, errors.Wrap(err, string(branchrev))
@@ -478,8 +477,8 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
 	}
 
 	// Now, list all the tags
-	tagsCmd := exec.CommandContext(ctx, "hg", "tags", "--debug", "--verbose")
-	tagsCmd.Dir = r.LocalPath()
+	tagsCmd := commandContext(ctx, "hg", "tags", "--debug", "--verbose")
+	tagsCmd.SetDir(r.LocalPath())
 	out, err := tagsCmd.CombinedOutput()
 	if err != nil {
 		return nil, errors.Wrap(err, string(out))
@@ -514,8 +513,8 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
 	// bookmarks next, because the presence of the magic @ bookmark has to
 	// determine how we handle the branches
 	var magicAt bool
-	bookmarksCmd := exec.CommandContext(ctx, "hg", "bookmarks", "--debug")
-	bookmarksCmd.Dir = r.LocalPath()
+	bookmarksCmd := commandContext(ctx, "hg", "bookmarks", "--debug")
+	bookmarksCmd.SetDir(r.LocalPath())
 	out, err = bookmarksCmd.CombinedOutput()
 	if err != nil {
 		// better nothing than partial and misleading
@@ -549,8 +548,8 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
 		}
 	}
 
-	cmd := exec.CommandContext(ctx, "hg", "branches", "-c", "--debug")
-	cmd.Dir = r.LocalPath()
+	cmd := commandContext(ctx, "hg", "branches", "-c", "--debug")
+	cmd.SetDir(r.LocalPath())
 	out, err = cmd.CombinedOutput()
 	if err != nil {
 		// better nothing than partial and misleading