Repositories

Omar Jarjur <ojarjur@google.com>
00c0e8 Submitting review 2c9bff89f0f8
Omar Jarjur committed at 2015-12-17 01:10:06

Improve the error messages returned when a git command fails.

Previously, we were simply cascading the error returned by the instance of exec.Command. However, that winds up just being something of the form "exit status 128", with all of the real error message going to the Stderr field.

As such, this commit changes the behavior to save the data written to stderr, and use it to construct a new error to return.

Modified repository/git.go

@@ -18,6 +18,7 @@ limitations under the License.
package repository
import (
"bytes"
"crypto/sha1"
"encoding/json"
"fmt"
@@ -34,11 +35,24 @@ type GitRepo struct {
}
// Run the given git command and return its stdout, or an error if the command fails.
func (repo *GitRepo) runGitCommand(args ...string) (string, error) {
func (repo *GitRepo) runGitCommandRaw(args ...string) (string, string, error) {
cmd := exec.Command("git", args...)
cmd.Dir = repo.Path
out, err := cmd.Output()
return strings.Trim(string(out), "\n"), err
var stdout bytes.Buffer
var stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err := cmd.Run()
return strings.TrimSpace(stdout.String()), strings.TrimSpace(stderr.String()), err
}
// Run the given git command and return its stdout, or an error if the command fails.
func (repo *GitRepo) runGitCommand(args ...string) (string, error) {
stdout, stderr, err := repo.runGitCommandRaw(args...)
if err != nil {
err = fmt.Errorf(stderr)
}
return stdout, err
}
// Run the given git command using the same stdin, stdout, and stderr as the review tool.
@@ -55,7 +69,7 @@ func (repo *GitRepo) runGitCommandInline(args ...string) error {
// and returns the corresponding GitRepo instance if it is.
func NewGitRepo(path string) (*GitRepo, error) {
repo := &GitRepo{Path: path}
_, err := repo.runGitCommand("rev-parse")
_, _, err := repo.runGitCommandRaw("rev-parse")
if err == nil {
return repo, nil
}
@@ -206,7 +220,7 @@ func (repo *GitRepo) MergeBase(a, b string) (string, error) {
// IsAncestor determines if the first argument points to a commit that is an ancestor of the second.
func (repo *GitRepo) IsAncestor(ancestor, descendant string) (bool, error) {
_, err := repo.runGitCommand("merge-base", "--is-ancestor", ancestor, descendant)
_, _, err := repo.runGitCommandRaw("merge-base", "--is-ancestor", ancestor, descendant)
if err == nil {
return true, nil
}