diff --git a/dashboard/builder/bench.go b/dashboard/builder/bench.go index cb2b0278..78e14f3c 100644 --- a/dashboard/builder/bench.go +++ b/dashboard/builder/bench.go @@ -87,23 +87,24 @@ func (b *Builder) buildBenchmark(workpath string, update bool) (benchBin, log st "GOPATH=" + gopath}, b.envv()...) // First, download without installing. - cmd := []string{gobin, "get", "-d"} + args := []string{"get", "-d"} if update { - cmd = append(cmd, "-u") + args = append(args, "-u") } - cmd = append(cmd, *benchPath) + args = append(args, *benchPath) var buildlog bytes.Buffer - ok, err := runOutput(*buildTimeout, env, &buildlog, workpath, cmd...) - if !ok || err != nil { + runOpts := []runOpt{runTimeout(*buildTimeout), runEnv(env), allOutput(&buildlog), runDir(workpath)} + err = run(exec.Command(gobin, args...), runOpts...) + if err != nil { fmt.Fprintf(&buildlog, "go get -d %s failed: %s", *benchPath, err) return "", buildlog.String(), err } // Then, build into workpath. benchBin = filepath.Join(workpath, "benchbin") + exeExt - cmd = []string{gobin, "build", "-o", benchBin, *benchPath} + args = []string{"build", "-o", benchBin, *benchPath} buildlog.Reset() - ok, err = runOutput(*buildTimeout, env, &buildlog, workpath, cmd...) - if !ok || err != nil { + err = run(exec.Command(gobin, args...), runOpts...) + if err != nil { fmt.Fprintf(&buildlog, "go build %s failed: %s", *benchPath, err) return "", buildlog.String(), err } @@ -172,23 +173,23 @@ func (b *Builder) executeBenchmark(workpath, hash, benchBin, bench string, procs "GOGCTRACE=1", // before Go1.2 fmt.Sprintf("GOMAXPROCS=%v", procs)}, b.envv()...) - cmd := []string{benchBin, + args := []string{ "-bench", bench, "-benchmem", strconv.Itoa(*benchMem), "-benchtime", benchTime.String(), "-benchnum", strconv.Itoa(*benchNum), "-tmpdir", workpath} if affinity != 0 { - cmd = append(cmd, "-affinity", strconv.Itoa(affinity)) + args = append(args, "-affinity", strconv.Itoa(affinity)) } benchlog := new(bytes.Buffer) - ok, err := runOutput(*buildTimeout, env, benchlog, workpath, cmd...) + err := run(exec.Command(benchBin, args...), runEnv(env), allOutput(benchlog), runDir(workpath)) if strip := benchlog.Len() - 512<<10; strip > 0 { // Leave the last 512K, that part contains metrics. benchlog = bytes.NewBuffer(benchlog.Bytes()[strip:]) } artifacts = []PerfArtifact{{Type: "log", Body: benchlog.String()}} - if !ok || err != nil { + if err != nil { if err != nil { log.Printf("Failed to execute benchmark '%v': %v", bench, err) ok = false diff --git a/dashboard/builder/env.go b/dashboard/builder/env.go index cb38741c..554d25f5 100644 --- a/dashboard/builder/env.go +++ b/dashboard/builder/env.go @@ -9,6 +9,7 @@ import ( "fmt" "io/ioutil" "os" + "os/exec" "path/filepath" "regexp" "runtime" @@ -151,16 +152,10 @@ func (env *gccgoEnv) setup(repo *Repo, workpath, hash string, envv []string) (st } // get the modified files for this commit. - statusCmd := []string{ - "hg", - "status", - "--no-status", - "--change", - hash, - } var buf bytes.Buffer - if _, err := runOutput(*cmdTimeout, envv, &buf, repo.Path, statusCmd...); err != nil { + if err := run(exec.Command("hg", "status", "--no-status", "--change", hash), + allOutput(&buf), runDir(repo.Path), runEnv(envv)); err != nil { return "", fmt.Errorf("Failed to find the modified files for %s: %s", hash, err) } modifiedFiles := strings.Split(buf.String(), "\n") @@ -204,7 +199,7 @@ func (env *gccgoEnv) setup(repo *Repo, workpath, hash string, envv []string) (st } buf.Reset() - if _, err := runOutput(*cmdTimeout, envv, &buf, gccpath, logCmd...); err != nil { + if err := run(exec.Command(gccpath, logCmd...), runEnv(envv), allOutput(&buf), runDir(gccpath)); err != nil { return "", fmt.Errorf("%s: %s", errMsg, err) } gccRev := buf.String() @@ -214,13 +209,7 @@ func (env *gccgoEnv) setup(repo *Repo, workpath, hash string, envv []string) (st // checkout gccRev // TODO(cmang): Fix this to work in parallel mode. - checkoutCmd := []string{ - "git", - "reset", - "--hard", - strings.TrimSpace(gccRev), - } - if _, err := runOutput(*cmdTimeout, envv, ioutil.Discard, gccpath, checkoutCmd...); err != nil { + if err := run(exec.Command("git", "reset", "--hard", strings.TrimSpace(gccRev)), runEnv(envv), runDir(gccpath)); err != nil { return "", fmt.Errorf("Failed to checkout commit at revision %s: %s", gccRev, err) } @@ -231,18 +220,16 @@ func (env *gccgoEnv) setup(repo *Repo, workpath, hash string, envv []string) (st } // configure GCC with substituted gofrontend and libgo - gccConfigCmd := []string{ - filepath.Join(gccpath, "configure"), + if err := run(exec.Command(filepath.Join(gccpath, "configure"), "--enable-languages=c,c++,go", "--disable-bootstrap", "--disable-multilib", - } - if _, err := runOutput(*cmdTimeout, envv, ioutil.Discard, gccobjdir, gccConfigCmd...); err != nil { - return "", fmt.Errorf("Failed to configure GCC: %s", err) + ), runEnv(envv), runDir(gccobjdir)); err != nil { + return "", fmt.Errorf("Failed to configure GCC: %v", err) } // build gcc - if _, err := runOutput(*buildTimeout, envv, ioutil.Discard, gccobjdir, "make"); err != nil { + if err := run(exec.Command("make"), runTimeout(*buildTimeout), runEnv(envv), runDir(gccobjdir)); err != nil { return "", fmt.Errorf("Failed to build GCC: %s", err) } diff --git a/dashboard/builder/exec.go b/dashboard/builder/exec.go index 590eb9b7..c40301f7 100644 --- a/dashboard/builder/exec.go +++ b/dashboard/builder/exec.go @@ -5,70 +5,62 @@ package main import ( - "bytes" "fmt" "io" "log" - "os" "os/exec" "time" ) -// run is a simple wrapper for exec.Run/Close -func run(d time.Duration, envv []string, dir string, argv ...string) error { - if *verbose { - log.Println("run", argv) +// run runs a command with optional arguments. +func run(cmd *exec.Cmd, opts ...runOpt) error { + a := runArgs{cmd, *cmdTimeout} + for _, opt := range opts { + opt.modArgs(&a) + } + if *verbose { + log.Printf("running %v", a.cmd.Args) } - cmd := exec.Command(argv[0], argv[1:]...) - cmd.Dir = dir - cmd.Env = envv - cmd.Stderr = os.Stderr if err := cmd.Start(); err != nil { return err } - err := timeout(d, cmd.Wait) + err := timeout(a.timeout, cmd.Wait) if _, ok := err.(timeoutError); ok { cmd.Process.Kill() } return err } -// runLog runs a process and returns the combined stdout/stderr. It returns -// process combined stdout and stderr output, exit status and error. The -// error returned is nil, if process is started successfully, even if exit -// status is not successful. -func runLog(timeout time.Duration, envv []string, dir string, argv ...string) (string, bool, error) { - var b bytes.Buffer - ok, err := runOutput(timeout, envv, &b, dir, argv...) - return b.String(), ok, err +// Zero or more runOpts can be passed to run to modify the command +// before it's run. +type runOpt interface { + modArgs(*runArgs) } -// runOutput runs a process and directs any output to the supplied writer. -// It returns exit status and error. The error returned is nil, if process -// is started successfully, even if exit status is not successful. -func runOutput(d time.Duration, envv []string, out io.Writer, dir string, argv ...string) (bool, error) { - if *verbose { - log.Println("runOutput", argv) +// allOutput sends both stdout and stderr to w. +func allOutput(w io.Writer) optFunc { + return func(a *runArgs) { + a.cmd.Stdout = w + a.cmd.Stderr = w } +} - cmd := exec.Command(argv[0], argv[1:]...) - cmd.Dir = dir - cmd.Env = envv - cmd.Stdout = out - cmd.Stderr = out - - startErr := cmd.Start() - if startErr != nil { - return false, startErr +func runTimeout(timeout time.Duration) optFunc { + return func(a *runArgs) { + a.timeout = timeout } +} - if err := timeout(d, cmd.Wait); err != nil { - if _, ok := err.(timeoutError); ok { - cmd.Process.Kill() - } - return false, err +func runDir(dir string) optFunc { + return func(a *runArgs) { + a.cmd.Dir = dir + } +} + +func runEnv(env []string) optFunc { + return func(a *runArgs) { + a.cmd.Env = env } - return true, nil } // timeout runs f and returns its error value, or if the function does not @@ -93,3 +85,14 @@ type timeoutError time.Duration func (e timeoutError) Error() string { return fmt.Sprintf("timed out after %v", time.Duration(e)) } + +// optFunc implements runOpt with a function, like http.HandlerFunc. +type optFunc func(*runArgs) + +func (f optFunc) modArgs(a *runArgs) { f(a) } + +// internal detail to exec.go: +type runArgs struct { + cmd *exec.Cmd + timeout time.Duration +} diff --git a/dashboard/builder/main.go b/dashboard/builder/main.go index 0cbcca79..b9498b5b 100644 --- a/dashboard/builder/main.go +++ b/dashboard/builder/main.go @@ -12,6 +12,7 @@ import ( "io/ioutil" "log" "os" + "os/exec" "path/filepath" "regexp" "runtime" @@ -409,15 +410,14 @@ func (b *Builder) buildRepoOnHash(workpath, hash, cmd string) (buildLog string, } // build - var buildlog bytes.Buffer + var buildbuf bytes.Buffer logfile := filepath.Join(workpath, "build.log") f, err := os.Create(logfile) if err != nil { - buildLog = err.Error() - return + return err.Error(), 0, err } defer f.Close() - w := io.MultiWriter(f, &buildlog) + w := io.MultiWriter(f, &buildbuf) // go's build command is a script relative to the srcDir, whereas // gccgo's build command is usually "make check-go" in the srcDir. @@ -427,23 +427,28 @@ func (b *Builder) buildRepoOnHash(workpath, hash, cmd string) (buildLog string, } } - // make sure commands with extra arguments are handled properly - splitCmd := strings.Split(cmd, " ") + // naive splitting of command from its arguments: + args := strings.Split(cmd, " ") + c := exec.Command(args[0], args[1:]...) + c.Dir = srcDir + c.Env = b.envv() + if *verbose { + c.Stdout = io.MultiWriter(os.Stdout, w) + c.Stderr = io.MultiWriter(os.Stderr, w) + } else { + c.Stdout = w + c.Stderr = w + } + startTime := time.Now() - ok, err := runOutput(*buildTimeout, b.envv(), w, srcDir, splitCmd...) - runTime = time.Now().Sub(startTime) - if !ok && err == nil { - err = fmt.Errorf("build failed") + err = run(c, runTimeout(*buildTimeout)) + runTime = time.Since(startTime) + if err != nil { + fmt.Fprintf(w, "Build complete, duration %v. Result: error: %v\n", runTime, err) + } else { + fmt.Fprintf(w, "Build complete, duration %v. Result: success\n", runTime) } - errf := func() string { - if err != nil { - return fmt.Sprintf("error: %v", err) - } - return "success" - } - fmt.Fprintf(w, "Build complete, duration %v. Result: %v\n", runTime, errf()) - buildLog = buildlog.String() - return + return buildbuf.String(), runTime, err } // failBuild checks for a new commit for this builder @@ -516,13 +521,12 @@ func (b *Builder) buildSubrepo(goRoot, goPath, pkg, hash string) (string, error) } // fetch package and dependencies - log, ok, err := runLog(*cmdTimeout, env, goPath, goTool, "get", "-d", pkg+"/...") - if err == nil && !ok { - err = fmt.Errorf("go exited with status 1") - } + var outbuf bytes.Buffer + err := run(exec.Command(goTool, "get", "-d", pkg+"/..."), runEnv(env), allOutput(&outbuf), runDir(goPath)) if err != nil { - return log, err + return outbuf.String(), err } + outbuf.Reset() // hg update to the specified hash pkgmaster, err := vcs.RepoRootForImportPath(pkg, *verbose) @@ -538,11 +542,9 @@ func (b *Builder) buildSubrepo(goRoot, goPath, pkg, hash string) (string, error) } // test the package - log, ok, err = runLog(*buildTimeout, env, goPath, goTool, "test", "-short", pkg+"/...") - if err == nil && !ok { - err = fmt.Errorf("go exited with status 1") - } - return log, err + err = run(exec.Command(goTool, "test", "-short", pkg+"/..."), + runTimeout(*buildTimeout), runEnv(env), allOutput(&outbuf), runDir(goPath)) + return outbuf.String(), err } // repoForTool returns the correct RepoRoot for the buildTool, or an error if diff --git a/dashboard/builder/vcs.go b/dashboard/builder/vcs.go index 49859af3..78e8008e 100644 --- a/dashboard/builder/vcs.go +++ b/dashboard/builder/vcs.go @@ -9,6 +9,7 @@ import ( "encoding/xml" "fmt" "os" + "os/exec" "path/filepath" "strings" "sync" @@ -73,9 +74,10 @@ func (r *Repo) Export(path, rev string) error { return err } - args := []string{r.Master.VCS.Cmd, "archive", "-t", "files", "-r", rev, path} - if err := run(*cmdTimeout, nil, downloadPath, args...); err != nil { - return fmt.Errorf("executing %s: %v", strings.Join(args, " "), err) + cmd := exec.Command(r.Master.VCS.Cmd, "archive", "-t", "files", "-r", rev, path) + cmd.Dir = downloadPath + if err := run(cmd); err != nil { + return fmt.Errorf("executing %v: %v", cmd.Args, err) } return nil }