cmd/go: improvements

Print build errors to stderr during 'go run'.
Stream test output during 'go test' (no args).  Fixes issue 2731.
Add go test -i to install test dependencies.  Fixes issue 2685.
Fix data race in exitStatus.  Fixes issue 2709.
Fix tool paths.  Fixes issue 2817.

R=golang-dev, bradfitz, n13m3y3r, r
CC=golang-dev
https://golang.org/cl/5591045

src/cmd/go/build.go

@@ -184,6 +184,7 @@ type builder struct {
 	gcflags     []string             // additional flags for Go compiler
 	actionCache map[cacheKey]*action // a cache of already-constructed actions
 	mkdirCache  map[string]bool      // a cache of created directories
+	print       func(args ...interface{}) (int, error)
 
 	output    sync.Mutex
 	scriptDir string // current directory in printed script
@@ -240,6 +241,7 @@ var (
 
 func (b *builder) init() {
 	var err error
+	b.print = fmt.Print
 	b.actionCache = make(map[cacheKey]*action)
 	b.mkdirCache = make(map[string]bool)
 	b.goarch = buildContext.GOARCH
@@ -454,7 +456,7 @@ func (b *builder) do(root *action) {
 
 		if err != nil {
 			if err == errPrintedOutput {
-				exitStatus = 2
+				setExitStatus(2)
 			} else {
 				errorf("%s", err)
 			}
@@ -742,7 +744,7 @@ func (b *builder) copyFile(dst, src string, perm os.FileMode) error {
 	os.Remove(dst)
 	df, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
 	if err != nil {
-		if runtime.GOOS != "windows" {
+		if !toolIsWindows {
 			return err
 		}
 		// Windows does not allow to replace binary file
@@ -799,7 +801,7 @@ func (b *builder) fmtcmd(dir string, format string, args ...interface{}) string
 func (b *builder) showcmd(dir string, format string, args ...interface{}) {
 	b.output.Lock()
 	defer b.output.Unlock()
-	fmt.Println(b.fmtcmd(dir, format, args...))
+	b.print(b.fmtcmd(dir, format, args...) + "\n")
 }
 
 // showOutput prints "# desc" followed by the given output.
@@ -836,7 +838,7 @@ func (b *builder) showOutput(dir, desc, out string) {
 
 	b.output.Lock()
 	defer b.output.Unlock()
-	fmt.Print(prefix, suffix)
+	b.print(prefix, suffix)
 }
 
 // relPaths returns a copy of paths with absolute paths
@@ -987,8 +989,7 @@ func (goToolchain) gc(b *builder, p *Package, obj string, importArgs []string, g
 		gcargs = append(gcargs, "-+")
 	}
 
-	binary := filepath.Join(goroot, "bin/go-tool/", b.arch+"g")
-	args := stringList(binary, "-o", ofile, b.gcflags, gcargs, importArgs)
+	args := stringList(tool(b.arch+"g"), "-o", ofile, b.gcflags, gcargs, importArgs)
 	for _, f := range gofiles {
 		args = append(args, mkAbs(p.Dir, f))
 	}
@@ -997,8 +998,7 @@ func (goToolchain) gc(b *builder, p *Package, obj string, importArgs []string, g
 
 func (goToolchain) asm(b *builder, p *Package, obj, ofile, sfile string) error {
 	sfile = mkAbs(p.Dir, sfile)
-	binary := filepath.Join(goroot, "bin/go-tool/", b.arch+"a")
-	return b.run(p.Dir, p.ImportPath, binary, "-I", obj, "-o", ofile, "-DGOOS_"+b.goos, "-DGOARCH_"+b.goarch, sfile)
+	return b.run(p.Dir, p.ImportPath, tool(b.arch+"a"), "-I", obj, "-o", ofile, "-DGOOS_"+b.goos, "-DGOARCH_"+b.goarch, sfile)
 }
 
 func (goToolchain) pkgpath(basedir string, p *Package) string {
@@ -1010,20 +1010,18 @@ func (goToolchain) pack(b *builder, p *Package, objDir, afile string, ofiles []s
 	for _, f := range ofiles {
 		absOfiles = append(absOfiles, mkAbs(objDir, f))
 	}
-	return b.run(p.Dir, p.ImportPath, filepath.Join(goroot, "bin/go-tool/pack"), "grc", mkAbs(objDir, afile), absOfiles)
+	return b.run(p.Dir, p.ImportPath, tool("pack"), "grc", mkAbs(objDir, afile), absOfiles)
 }
 
 func (goToolchain) ld(b *builder, p *Package, out string, allactions []*action, mainpkg string, ofiles []string) error {
 	importArgs := b.includeArgs("-L", allactions)
-	binary := filepath.Join(goroot, "bin/go-tool/", b.arch+"l")
-	return b.run(p.Dir, p.ImportPath, binary, "-o", out, importArgs, mainpkg)
+	return b.run(p.Dir, p.ImportPath, tool(b.arch+"l"), "-o", out, importArgs, mainpkg)
 }
 
 func (goToolchain) cc(b *builder, p *Package, objdir, ofile, cfile string) error {
 	inc := filepath.Join(goroot, "pkg", fmt.Sprintf("%s_%s", b.goos, b.goarch))
 	cfile = mkAbs(p.Dir, cfile)
-	binary := filepath.Join(goroot, "bin/go-tool/", b.arch+"c")
-	return b.run(p.Dir, p.ImportPath, binary, "-FVw",
+	return b.run(p.Dir, p.ImportPath, tool(b.arch+"c"), "-FVw",
 		"-I", objdir, "-I", inc, "-o", ofile,
 		"-DGOOS_"+b.goos, "-DGOARCH_"+b.goarch, cfile)
 }
@@ -1136,7 +1134,7 @@ func (b *builder) gccCmd(objdir string) []string {
 var cgoRe = regexp.MustCompile(`[/\\:]`)
 
 func (b *builder) cgo(p *Package, cgoExe, obj string, gccfiles []string) (outGo, outObj []string, err error) {
-	if b.goos != runtime.GOOS {
+	if b.goos != toolGOOS {
 		return nil, nil, errors.New("cannot use cgo when compiling for a different operating system")
 	}
 

src/cmd/go/fix.go

@@ -25,6 +25,6 @@ func runFix(cmd *Command, args []string) {
 		// Use pkg.gofiles instead of pkg.Dir so that
 		// the command only applies to this package,
 		// not to packages in subdirectories.
-		run(stringList("gofix", relPaths(pkg.gofiles)))
+		run(stringList(tool("fix"), relPaths(pkg.gofiles)))
 	}
 }

src/cmd/go/get.go

@@ -132,7 +132,7 @@ func download(arg string, stk *importStack) {
 	}
 
 	if *getFix {
-		run(stringList("gofix", relPaths(p.gofiles)))
+		run(stringList(tool("fix"), relPaths(p.gofiles)))
 
 		// The imports might have changed, so reload again.
 		p = reloadPackage(arg, stk)

src/cmd/go/main.go

@@ -17,6 +17,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"sync"
 	"text/template"
 	"unicode"
 	"unicode/utf8"
@@ -88,6 +89,15 @@ var commands = []*Command{
 }
 
 var exitStatus = 0
+var exitMu sync.Mutex
+
+func setExitStatus(n int) {
+	exitMu.Lock()
+	if exitStatus < n {
+		exitStatus = n
+	}
+	exitMu.Unlock()
+}
 
 func main() {
 	flag.Usage = usage
@@ -268,7 +278,7 @@ func fatalf(format string, args ...interface{}) {
 
 func errorf(format string, args ...interface{}) {
 	log.Printf(format, args...)
-	exitStatus = 1
+	setExitStatus(1)
 }
 
 var logf = log.Printf

src/cmd/go/pkg.go

@@ -8,7 +8,6 @@ import (
 	"go/build"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sort"
 	"strings"
 	"time"
@@ -276,13 +275,13 @@ func scanPackage(ctxt *build.Context, t *build.Tree, arg, importPath, dir string
 
 	if info.Package == "main" {
 		_, elem := filepath.Split(importPath)
-		if t.Goroot && isGoTool[p.ImportPath] {
-			p.target = filepath.Join(t.Path, "bin/go-tool", elem)
-		} else {
-			if ctxt.GOOS != runtime.GOOS || ctxt.GOARCH != runtime.GOARCH {
+		if ctxt.GOOS != toolGOOS || ctxt.GOARCH != toolGOARCH {
 			// Install cross-compiled binaries to subdirectories of bin.
 			elem = ctxt.GOOS + "_" + ctxt.GOARCH + "/" + elem
 		}
+		if t.Goroot && isGoTool[p.ImportPath] {
+			p.target = filepath.Join(t.Path, "bin/go-tool", elem)
+		} else {
 			p.target = filepath.Join(t.BinDir(), elem)
 		}
 		if ctxt.GOOS == "windows" {

src/cmd/go/run.go

@@ -4,7 +4,11 @@
 
 package main
 
-import "strings"
+import (
+	"fmt"
+	"os"
+	"strings"
+)
 
 var cmdRun = &Command{
 	UsageLine: "run [-a] [-n] [-x] gofiles... [arguments...]",
@@ -28,9 +32,14 @@ func init() {
 	cmdRun.Flag.BoolVar(&buildX, "x", false, "")
 }
 
+func printStderr(args ...interface{}) (int, error) {
+	return fmt.Fprint(os.Stderr, args...)
+}
+
 func runRun(cmd *Command, args []string) {
 	var b builder
 	b.init()
+	b.print = printStderr
 	i := 0
 	for i < len(args) && strings.HasSuffix(args[i], ".go") {
 		i++

src/cmd/go/test.go

@@ -15,6 +15,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"sort"
 	"strings"
 	"text/template"
 	"time"
@@ -81,6 +82,7 @@ The flags handled by 'go test' are:
 
 	-i
 	    Install packages that are dependencies of the test.
+	    Do not run the test.
 
 	-p n
 	    Compile and test up to n packages in parallel.
@@ -191,24 +193,21 @@ See the documentation of the testing package for more information.
 
 var (
 	testC            bool     // -c flag
+	testI            bool     // -i flag
 	testP            int      // -p flag
 	testX            bool     // -x flag
 	testV            bool     // -v flag
 	testFiles        []string // -file flag(s)  TODO: not respected
 	testArgs         []string
-	testShowPass bool // whether to display passing output
 	testBench        bool
+	testStreamOutput bool // show output as it is generated
+	testShowPass     bool // show passing output
 )
 
 func runTest(cmd *Command, args []string) {
 	var pkgArgs []string
 	pkgArgs, testArgs = testFlags(args)
 
-	// show test PASS output when no packages
-	// are listed (implicitly current directory: "go test")
-	// or when the -v flag has been given.
-	testShowPass = len(pkgArgs) == 0 || testV
-
 	pkgs := packagesForBuild(pkgArgs)
 	if len(pkgs) == 0 {
 		fatalf("no packages to test")
@@ -218,6 +217,21 @@ func runTest(cmd *Command, args []string) {
 		fatalf("cannot use -c flag with multiple packages")
 	}
 
+	// show passing test output (after buffering) with -v flag.
+	// must buffer because tests are running in parallel, and
+	// otherwise the output will get mixed.
+	testShowPass = testV
+
+	// stream test output (no buffering) when no package has
+	// been given on the command line (implicit current directory)
+	// or when benchmarking.
+	// Also stream if we're showing output anyway with a
+	// single package under test.  In that case, streaming the
+	// output produces the same result as not streaming,
+	// just more immediately.
+	testStreamOutput = len(pkgArgs) == 0 || testBench ||
+		(len(pkgs) <= 1 && testShowPass)
+
 	buildX = testX
 	if testP > 0 {
 		buildP = testP
@@ -226,6 +240,38 @@ func runTest(cmd *Command, args []string) {
 	var b builder
 	b.init()
 
+	if testI {
+		buildV = testV
+
+		deps := map[string]bool{
+			// Dependencies for testmain.
+			"testing": true,
+			"regexp":  true,
+		}
+		for _, p := range pkgs {
+			// Dependencies for each test.
+			for _, path := range p.info.Imports {
+				deps[path] = true
+			}
+			for _, path := range p.info.TestImports {
+				deps[path] = true
+			}
+		}
+
+		all := []string{}
+		for path := range deps {
+			all = append(all, path)
+		}
+		sort.Strings(all)
+
+		a := &action{}
+		for _, p := range packagesForBuild(all) {
+			a.deps = append(a.deps, b.action(modeInstall, modeInstall, p))
+		}
+		b.do(a)
+		return
+	}
+
 	var builds, runs, prints []*action
 
 	// Prepare build + run + print actions for all packages being tested.
@@ -284,7 +330,7 @@ func runTest(cmd *Command, args []string) {
 		}
 	}
 	if warned {
-		fmt.Fprintf(os.Stderr, "installing these packages with 'go install' will speed future tests.\n\n")
+		fmt.Fprintf(os.Stderr, "installing these packages with 'go test -i' will speed future tests.\n\n")
 	}
 
 	b.do(root)
@@ -473,15 +519,20 @@ func (b *builder) runTest(a *action) error {
 		// We were unable to build the binary.
 		a.failed = false
 		fmt.Fprintf(a.testOutput, "FAIL\t%s [build failed]\n", a.p.ImportPath)
-		exitStatus = 1
+		setExitStatus(1)
 		return nil
 	}
 
 	cmd := exec.Command(args[0], args[1:]...)
 	cmd.Dir = a.p.Dir
 	var buf bytes.Buffer
+	if testStreamOutput {
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
+	} else {
 		cmd.Stdout = &buf
 		cmd.Stderr = &buf
+	}
 
 	t0 := time.Now()
 	err := cmd.Start()
@@ -511,21 +562,21 @@ func (b *builder) runTest(a *action) error {
 	t1 := time.Now()
 	t := fmt.Sprintf("%.3fs", t1.Sub(t0).Seconds())
 	if err == nil {
-		fmt.Fprintf(a.testOutput, "ok  \t%s\t%s\n", a.p.ImportPath, t)
 		if testShowPass {
 			a.testOutput.Write(out)
 		}
+		fmt.Fprintf(a.testOutput, "ok  \t%s\t%s\n", a.p.ImportPath, t)
 		return nil
 	}
 
-	fmt.Fprintf(a.testOutput, "FAIL\t%s\t%s\n", a.p.ImportPath, t)
-	exitStatus = 1
+	setExitStatus(1)
 	if len(out) > 0 {
 		a.testOutput.Write(out)
 		// assume printing the test binary's exit status is superfluous
 	} else {
 		fmt.Fprintf(a.testOutput, "%s\n", err)
 	}
+	fmt.Fprintf(a.testOutput, "FAIL\t%s\t%s\n", a.p.ImportPath, t)
 
 	return nil
 }

src/cmd/go/testflag.go

@@ -40,7 +40,7 @@ var usageMessage = `Usage of go test:
 // usage prints a usage message and exits.
 func testUsage() {
 	fmt.Fprint(os.Stderr, usageMessage)
-	exitStatus = 2
+	setExitStatus(2)
 	exit()
 }
 
@@ -58,6 +58,7 @@ var testFlagDefn = []*testFlagSpec{
 	// local.
 	{name: "c", isBool: true},
 	{name: "file", multiOK: true},
+	{name: "i", isBool: true},
 	{name: "p"},
 	{name: "x", isBool: true},
 
@@ -119,6 +120,8 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 		switch f.name {
 		case "c":
 			setBoolFlag(&testC, value)
+		case "i":
+			setBoolFlag(&testI, value)
 		case "p":
 			setIntFlag(&testP, value)
 		case "x":

src/cmd/go/tool.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"sort"
 	"strings"
 )
@@ -27,37 +28,43 @@ For more about each tool command, see 'go tool command -h'.
 }
 
 var (
-	toolGoos       = build.DefaultContext.GOOS
-	toolIsWindows  = toolGoos == "windows"
-	toolBinToolDir = filepath.Join(build.Path[0].Path, "bin", "go-tool")
+	toolGOOS      = runtime.GOOS
+	toolGOARCH    = runtime.GOARCH
+	toolIsWindows = toolGOOS == "windows"
+	toolDir       = filepath.Join(build.Path[0].Path, "bin", "go-tool")
 )
 
 const toolWindowsExtension = ".exe"
 
+func tool(name string) string {
+	p := filepath.Join(toolDir, name)
+	if toolIsWindows {
+		p += toolWindowsExtension
+	}
+	return p
+}
+
 func runTool(cmd *Command, args []string) {
 	if len(args) == 0 {
 		listTools()
 		return
 	}
-	tool := args[0]
+	toolName := args[0]
 	// The tool name must be lower-case letters and numbers.
-	for _, c := range tool {
+	for _, c := range toolName {
 		switch {
 		case 'a' <= c && c <= 'z', '0' <= c && c <= '9':
 		default:
 			fmt.Fprintf(os.Stderr, "go tool: bad tool name %q\n", tool)
-			exitStatus = 2
+			setExitStatus(2)
 			return
 		}
 	}
-	toolPath := toolBinToolDir + "/" + tool
-	if toolIsWindows {
-		toolPath += toolWindowsExtension
-	}
+	toolPath := tool(toolName)
 	// Give a nice message if there is no tool with that name.
 	if _, err := os.Stat(toolPath); err != nil {
 		fmt.Fprintf(os.Stderr, "go tool: no such tool %q\n", tool)
-		exitStatus = 3
+		setExitStatus(3)
 		return
 	}
 	toolCmd := &exec.Cmd{
@@ -69,23 +76,24 @@ func runTool(cmd *Command, args []string) {
 	err := toolCmd.Run()
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "go tool %s failed: %s\n", tool, err)
-		exitStatus = 1
+		setExitStatus(1)
 		return
 	}
 }
 
 // listTools prints a list of the available tools in the go-tools directory.
 func listTools() {
-	toolDir, err := os.Open(toolBinToolDir)
+	f, err := os.Open(toolDir)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "go tool: no tool directory: %s\n", err)
-		exitStatus = 2
+		setExitStatus(2)
 		return
 	}
-	names, err := toolDir.Readdirnames(-1)
+	defer f.Close()
+	names, err := f.Readdirnames(-1)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "go tool: can't read directory: %s\n", err)
-		exitStatus = 2
+		setExitStatus(2)
 		return
 	}
 	sort.Strings(names)

src/cmd/go/vet.go

@@ -25,6 +25,6 @@ func runVet(cmd *Command, args []string) {
 		// Use pkg.gofiles instead of pkg.Dir so that
 		// the command only applies to this package,
 		// not to packages in subdirectories.
-		run("govet", relPaths(pkg.gofiles))
+		run(tool("vet"), relPaths(pkg.gofiles))
 	}
 }