Stop SimpleExec swallowing errors. Fixed #1183.

ExitError conditions now return a plain error with message like:

    Error running git [config --system filter.lfs.clean git-lfs clean -- %f]: 'error: could not lock config file /etc/gitconfig: Permission denied' 'exit status 255'

Also:

* Made Set/Unset Config commands return errors when they occur.
* Made install attribute set fail properly with an error message e.g. if permission-denied.  Previously this failed silently and reported success.

***NOTE*** this commit contains Go 1.6-specific APIs.  The following commit will remove those, and that following commit can be reverted when Go >= 1.6 is required.

git/git.go

@@ -304,55 +304,55 @@ func (c *gitConfig) FindLocal(val string) string {
 }
 
 // SetGlobal sets the git config value for the key in the global config
-func (c *gitConfig) SetGlobal(key, val string) {
-	subprocess.SimpleExec("git", "config", "--global", key, val)
+func (c *gitConfig) SetGlobal(key, val string) (string, error) {
+	return subprocess.SimpleExec("git", "config", "--global", key, val)
 }
 
 // SetSystem sets the git config value for the key in the system config
-func (c *gitConfig) SetSystem(key, val string) {
-	subprocess.SimpleExec("git", "config", "--system", key, val)
+func (c *gitConfig) SetSystem(key, val string) (string, error) {
+	return subprocess.SimpleExec("git", "config", "--system", key, val)
 }
 
 // UnsetGlobal removes the git config value for the key from the global config
-func (c *gitConfig) UnsetGlobal(key string) {
-	subprocess.SimpleExec("git", "config", "--global", "--unset", key)
+func (c *gitConfig) UnsetGlobal(key string) (string, error) {
+	return subprocess.SimpleExec("git", "config", "--global", "--unset", key)
 }
 
 // UnsetSystem removes the git config value for the key from the system config
-func (c *gitConfig) UnsetSystem(key string) {
-	subprocess.SimpleExec("git", "config", "--system", "--unset", key)
+func (c *gitConfig) UnsetSystem(key string) (string, error) {
+	return subprocess.SimpleExec("git", "config", "--system", "--unset", key)
 }
 
 // UnsetGlobalSection removes the entire named section from the global config
-func (c *gitConfig) UnsetGlobalSection(key string) {
-	subprocess.SimpleExec("git", "config", "--global", "--remove-section", key)
+func (c *gitConfig) UnsetGlobalSection(key string) (string, error) {
+	return subprocess.SimpleExec("git", "config", "--global", "--remove-section", key)
 }
 
 // UnsetGlobalSection removes the entire named section from the system config
-func (c *gitConfig) UnsetSystemSection(key string) {
-	subprocess.SimpleExec("git", "config", "--system", "--remove-section", key)
+func (c *gitConfig) UnsetSystemSection(key string) (string, error) {
+	return subprocess.SimpleExec("git", "config", "--system", "--remove-section", key)
 }
 
 // SetLocal sets the git config value for the key in the specified config file
-func (c *gitConfig) SetLocal(file, key, val string) {
+func (c *gitConfig) SetLocal(file, key, val string) (string, error) {
 	args := make([]string, 1, 5)
 	args[0] = "config"
 	if len(file) > 0 {
 		args = append(args, "--file", file)
 	}
 	args = append(args, key, val)
-	subprocess.SimpleExec("git", args...)
+	return subprocess.SimpleExec("git", args...)
 }
 
 // UnsetLocalKey removes the git config value for the key from the specified config file
-func (c *gitConfig) UnsetLocalKey(file, key string) {
+func (c *gitConfig) UnsetLocalKey(file, key string) (string, error) {
 	args := make([]string, 1, 5)
 	args[0] = "config"
 	if len(file) > 0 {
 		args = append(args, "--file", file)
 	}
 	args = append(args, "--unset", key)
-	subprocess.SimpleExec("git", args...)
+	return subprocess.SimpleExec("git", args...)
 }
 
 // List lists all of the git config values

lfs/attribute.go

@@ -74,18 +74,21 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error {
 	}
 
 	if opt.Force || shouldReset(currentValue) {
+		var err error
 		if opt.Local {
+			// ignore error for unset, git returns non-zero if missing
 			git.Config.UnsetLocalKey("", key)
-			git.Config.SetLocal("", key, value)
+			_, err = git.Config.SetLocal("", key, value)
 		} else if opt.System {
+			// ignore error for unset, git returns non-zero if missing
 			git.Config.UnsetSystem(key)
-			git.Config.SetSystem(key, value)
+			_, err = git.Config.SetSystem(key, value)
 		} else {
+			// ignore error for unset, git returns non-zero if missing
 			git.Config.UnsetGlobal(key)
-			git.Config.SetGlobal(key, value)
+			_, err = git.Config.SetGlobal(key, value)
 		}
-
-		return nil
+		return err
 	} else if currentValue != value {
 		return fmt.Errorf("The %s attribute should be \"%s\" but is \"%s\"",
 			key, value, currentValue)

subprocess/subprocess.go

@@ -17,14 +17,20 @@ func SimpleExec(name string, args ...string) (string, error) {
 	cmd := ExecCommand(name, args...)
 
 	output, err := cmd.Output()
-	if _, ok := err.(*exec.ExitError); ok {
-		return "", nil
-	}
-	if err != nil {
-		return fmt.Sprintf("Error running %s %s", name, args), err
+	if exitError, ok := err.(*exec.ExitError); ok {
+		errorOutput := strings.TrimSpace(string(exitError.Stderr))
+		if errorOutput == "" {
+			// some commands might write nothing to stderr but something to stdout in error-conditions, in which case, we'll use that
+			// in the error string
+			errorOutput = strings.TrimSpace(string(output))
+		}
+		formattedErr := fmt.Errorf("Error running %s %s: '%s' '%s'", name, args, errorOutput, strings.TrimSpace(exitError.Error()))
+
+		// return "" as output in error case, for callers that don't care about errors but rely on "" returned, in-case stdout != ""
+		return "", formattedErr
 	}
 
-	return strings.Trim(string(output), " \n"), nil
+	return strings.Trim(string(output), " \n"), err
 }
 
 // An env for an exec.Command without GIT_TRACE