fix(git): properly log errors from git (#199)

Author: Joshua-Anderson

cmd/git.go

@@ -18,7 +18,10 @@ func (d DeisCmd) GitRemote(appID, remote string, force bool) error {
 	if err != nil {
 		//If git remote doesn't exist, create it without issue
 		if err == git.ErrRemoteNotFound {
-			git.CreateRemote(s.Client.ControllerURL.Host, remote, appID)
+			err := git.CreateRemote(s.Client.ControllerURL.Host, remote, appID)
+			if err != nil {
+				return err
+			}
 			d.Printf(remoteCreationMsg, remote, appID)
 			return nil
 		}
@@ -35,8 +38,14 @@ func (d DeisCmd) GitRemote(appID, remote string, force bool) error {
 
 	if force {
 		d.Printf("Deleting git remote %s.\n", remote)
-		git.DeleteRemote(remote)
-		git.CreateRemote(s.Client.ControllerURL.Host, remote, appID)
+		err := git.DeleteRemote(remote)
+		if err != nil {
+			return err
+		}
+		err = git.CreateRemote(s.Client.ControllerURL.Host, remote, appID)
+		if err != nil {
+			return err
+		}
 		d.Printf(remoteCreationMsg, remote, appID)
 		return nil
 	}

pkg/git/git.go

@@ -3,7 +3,6 @@ package git
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -14,23 +13,25 @@ import (
 // ErrRemoteNotFound is returned when the remote cannot be found in git
 var ErrRemoteNotFound = errors.New("Could not find remote matching app in 'git remote -v'")
 
+func gitError(err *exec.ExitError, cmd []string) error {
+	msg := fmt.Sprintf("Error when running '%s'\n", strings.Join(cmd, " "))
+	out := string(err.Stderr)
+	if out != "" {
+		msg += strings.TrimSpace(out)
+	}
+
+	return errors.New(msg)
+}
+
 // CreateRemote adds a git remote in the current directory.
 func CreateRemote(host, remote, appID string) error {
-	cmd := exec.Command("git", "remote", "add", remote, RemoteURL(host, appID))
-	stderr, err := cmd.StderrPipe()
-
+	cmd := []string{"git", "remote", "add", remote, RemoteURL(host, appID)}
+	_, err := exec.Command(cmd[0], cmd[1:]...).Output()
 	if err != nil {
-		return err
-	}
-
-	if err = cmd.Start(); err != nil {
-		return err
+		return gitError(err.(*exec.ExitError), cmd)
 	}
 
-	output, _ := ioutil.ReadAll(stderr)
-	fmt.Print(string(output))
-
-	return cmd.Wait()
+	return nil
 }
 
 // DeleteAppRemotes removes all git remotes corresponding to an app in the repository.
@@ -52,23 +53,28 @@ func DeleteAppRemotes(host, appID string) error {
 
 // DeleteRemote removes a remote from the repository
 func DeleteRemote(name string) error {
-	_, err := exec.Command("git", "remote", "remove", name).Output()
-	return err
+	cmd := []string{"git", "remote", "remove", name}
+	_, err := exec.Command(cmd[0], cmd[1:]...).Output()
+	if err != nil {
+		return gitError(err.(*exec.ExitError), cmd)
+	}
+
+	return nil
 }
 
 // remoteNamesFromAppID returns the git remote names for an app
 func remoteNamesFromAppID(host, appID string) ([]string, error) {
-	out, err := exec.Command("git", "remote", "-v").Output()
+	cmd := []string{"git", "remote", "-v"}
+	out, err := exec.Command(cmd[0], cmd[1:]...).Output()
 
 	if err != nil {
-		return []string{}, err
+		return []string{}, gitError(err.(*exec.ExitError), cmd)
 	}
 
-	cmd := string(out)
 	remotes := []string{}
 
 lines:
-	for _, line := range strings.Split(cmd, "\n") {
+	for _, line := range strings.Split(string(out), "\n") {
 		if strings.Contains(line, RemoteURL(host, appID)) {
 			name := strings.Split(line, "\t")[0]
 			// git remote -v can show duplicate remotes, so don't add a remote if it already has been added
@@ -103,19 +109,17 @@ func DetectAppName(host string) (string, error) {
 }
 
 func findRemote(host string) (string, error) {
-	out, err := exec.Command("git", "remote", "-v").Output()
-
+	cmd := []string{"git", "remote", "-v"}
+	out, err := exec.Command(cmd[0], cmd[1:]...).Output()
 	if err != nil {
-		return "", err
+		return "", gitError(err.(*exec.ExitError), cmd)
 	}
 
-	cmd := string(out)
-
 	// Strip off any trailing :port number after the host name.
 	host = strings.Split(host, ":")[0]
 	builderHost := getBuilderHostname(host)
 
-	for _, line := range strings.Split(cmd, "\n") {
+	for _, line := range strings.Split(string(out), "\n") {
 		for _, remote := range strings.Split(line, " ") {
 			if strings.Contains(remote, host) || strings.Contains(remote, builderHost) {
 				return strings.Split(remote, "\t")[1], nil
@@ -142,14 +146,15 @@ func getBuilderHostname(host string) string {
 
 // RemoteValue gets the url that a git remote is set to.
 func RemoteValue(name string) (string, error) {
-	out, err := exec.Command("git", "remote", "get-url", name).Output()
+	cmd := []string{"git", "remote", "get-url", name}
+	out, err := exec.Command(cmd[0], cmd[1:]...).Output()
 
 	if err != nil {
 		// get the return code of the program and see if it equals not found
 		if err.(*exec.ExitError).Sys().(syscall.WaitStatus).ExitStatus() == 128 {
 			return "", ErrRemoteNotFound
 		}
-		return "", err
+		return "", gitError(err.(*exec.ExitError), cmd)
 	}
 
 	return strings.Trim(string(out), "\n"), nil