cmd/go: add Package.StaleReason for debugging with go list

It comes up every few months that we can't understand why the go command is rebuilding some package. Add diagnostics so that the go command can explain itself if asked. For golang#2775, golang#3506, golang#12074. Change-Id: I1c73b492589b49886bf31a8f9d05514adbd6ed70 Reviewed-on: https://go-review.googlesource.com/22432 Reviewed-by: Rob Pike <r@golang.org>
mk0x9 · Apr 27, 2016 · 0b5fbf7 · 0b5fbf7
1 parent 525ae3f
commit 0b5fbf7
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 59 deletions.
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
@@ -568,6 +568,7 @@ syntax of package template.  The default output is equivalent to -f
         Goroot        bool   // is this package in the Go root?
         Standard      bool   // is this package part of the standard Go library?
         Stale         bool   // would 'go install' do anything for this package?
+        StaleReason   string // explanation for Stale==true
         Root          string // Go root or Go path dir containing this package
 
         // Source files

diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go
@@ -465,6 +465,7 @@ func runBuild(cmd *Command, args []string) {
 		p := pkgs[0]
 		p.target = *buildO
 		p.Stale = true // must build - not up to date
+		p.StaleReason = "build -o flag in use"
 		a := b.action(modeInstall, depMode, p)
 		b.do(a)
 		return
@@ -836,6 +837,7 @@ func goFilesPackage(gofiles []string) *Package {
 
 	pkg.Target = pkg.target
 	pkg.Stale = true
+	pkg.StaleReason = "files named on command line"
 
 	computeStale(pkg)
 	return pkg

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
@@ -524,32 +524,43 @@ func (tg *testgoData) wantArchive(path string) {
 	}
 }
 
-// isStale returns whether pkg is stale.
-func (tg *testgoData) isStale(pkg string) bool {
-	tg.run("list", "-f", "{{.Stale}}", pkg)
-	switch v := strings.TrimSpace(tg.getStdout()); v {
-	case "true":
-		return true
-	case "false":
-		return false
-	default:
-		tg.t.Fatalf("unexpected output checking staleness of package %v: %v", pkg, v)
-		panic("unreachable")
+// isStale reports whether pkg is stale, and why
+func (tg *testgoData) isStale(pkg string) (bool, string) {
+	tg.run("list", "-f", "{{.Stale}}:{{.StaleReason}}", pkg)
+	v := strings.TrimSpace(tg.getStdout())
+	f := strings.SplitN(v, ":", 2)
+	if len(f) == 2 {
+		switch f[0] {
+		case "true":
+			return true, f[1]
+		case "false":
+			return false, f[1]
+		}
 	}
+	tg.t.Fatalf("unexpected output checking staleness of package %v: %v", pkg, v)
+	panic("unreachable")
 }
 
 // wantStale fails with msg if pkg is not stale.
-func (tg *testgoData) wantStale(pkg, msg string) {
-	if !tg.isStale(pkg) {
+func (tg *testgoData) wantStale(pkg, reason, msg string) {
+	stale, why := tg.isStale(pkg)
+	if !stale {
 		tg.t.Fatal(msg)
 	}
+	if reason == "" && why != "" || !strings.Contains(why, reason) {
+		tg.t.Errorf("wrong reason for Stale=true: %q, want %q", why, reason)
+	}
 }
 
 // wantNotStale fails with msg if pkg is stale.
-func (tg *testgoData) wantNotStale(pkg, msg string) {
-	if tg.isStale(pkg) {
+func (tg *testgoData) wantNotStale(pkg, reason, msg string) {
+	stale, why := tg.isStale(pkg)
+	if stale {
 		tg.t.Fatal(msg)
 	}
+	if reason == "" && why != "" || !strings.Contains(why, reason) {
+		tg.t.Errorf("wrong reason for Stale=false: %q, want %q", why, reason)
+	}
 }
 
 // cleanup cleans up a test that runs testgo.
@@ -708,7 +719,7 @@ func TestNewReleaseRebuildsStalePackagesInGOPATH(t *testing.T) {
 	tg.tempFile("d1/src/p1/p1.go", `package p1`)
 	tg.setenv("GOPATH", tg.path("d1"))
 	tg.run("install", "-a", "p1")
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale, incorrectly")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale, incorrectly")
 	tg.sleep()
 
 	// Changing mtime and content of runtime/internal/sys/sys.go
@@ -717,28 +728,28 @@ func TestNewReleaseRebuildsStalePackagesInGOPATH(t *testing.T) {
 	sys := runtime.GOROOT() + "/src/runtime/internal/sys/sys.go"
 	restore := addNL(sys)
 	defer restore()
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale, incorrectly, after updating runtime/internal/sys/sys.go")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale, incorrectly, after updating runtime/internal/sys/sys.go")
 	restore()
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale, incorrectly, after restoring runtime/internal/sys/sys.go")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale, incorrectly, after restoring runtime/internal/sys/sys.go")
 
 	// But changing runtime/internal/sys/zversion.go should have an effect:
 	// that's how we tell when we flip from one release to another.
 	zversion := runtime.GOROOT() + "/src/runtime/internal/sys/zversion.go"
 	restore = addNL(zversion)
 	defer restore()
-	tg.wantStale("p1", "./testgo list claims p1 is NOT stale, incorrectly, after changing to new release")
+	tg.wantStale("p1", "build ID mismatch", "./testgo list claims p1 is NOT stale, incorrectly, after changing to new release")
 	restore()
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale, incorrectly, after changing back to old release")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale, incorrectly, after changing back to old release")
 	addNL(zversion)
-	tg.wantStale("p1", "./testgo list claims p1 is NOT stale, incorrectly, after changing again to new release")
+	tg.wantStale("p1", "build ID mismatch", "./testgo list claims p1 is NOT stale, incorrectly, after changing again to new release")
 	tg.run("install", "p1")
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale after building with new release")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale after building with new release")
 
 	// Restore to "old" release.
 	restore()
-	tg.wantStale("p1", "./testgo list claims p1 is NOT stale, incorrectly, after changing to old release after new build")
+	tg.wantStale("p1", "build ID mismatch", "./testgo list claims p1 is NOT stale, incorrectly, after changing to old release after new build")
 	tg.run("install", "p1")
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale after building with old release")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale after building with old release")
 
 	// Everything is out of date. Rebuild to leave things in a better state.
 	tg.run("install", "std")
@@ -821,8 +832,8 @@ func TestGoInstallRebuildsStalePackagesInOtherGOPATH(t *testing.T) {
 	sep := string(filepath.ListSeparator)
 	tg.setenv("GOPATH", tg.path("d1")+sep+tg.path("d2"))
 	tg.run("install", "p1")
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale, incorrectly")
-	tg.wantNotStale("p2", "./testgo list claims p2 is stale, incorrectly")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale, incorrectly")
+	tg.wantNotStale("p2", "", "./testgo list claims p2 is stale, incorrectly")
 	tg.sleep()
 	if f, err := os.OpenFile(tg.path("d2/src/p2/p2.go"), os.O_WRONLY|os.O_APPEND, 0); err != nil {
 		t.Fatal(err)
@@ -831,12 +842,12 @@ func TestGoInstallRebuildsStalePackagesInOtherGOPATH(t *testing.T) {
 	} else {
 		tg.must(f.Close())
 	}
-	tg.wantStale("p2", "./testgo list claims p2 is NOT stale, incorrectly")
-	tg.wantStale("p1", "./testgo list claims p1 is NOT stale, incorrectly")
+	tg.wantStale("p2", "newer source file", "./testgo list claims p2 is NOT stale, incorrectly")
+	tg.wantStale("p1", "stale dependency", "./testgo list claims p1 is NOT stale, incorrectly")
 
 	tg.run("install", "p1")
-	tg.wantNotStale("p2", "./testgo list claims p2 is stale after reinstall, incorrectly")
-	tg.wantNotStale("p1", "./testgo list claims p1 is stale after reinstall, incorrectly")
+	tg.wantNotStale("p2", "", "./testgo list claims p2 is stale after reinstall, incorrectly")
+	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale after reinstall, incorrectly")
 }
 
 func TestGoInstallDetectsRemovedFiles(t *testing.T) {
@@ -850,13 +861,13 @@ func TestGoInstallDetectsRemovedFiles(t *testing.T) {
 		package mypkg`)
 	tg.setenv("GOPATH", tg.path("."))
 	tg.run("install", "mypkg")
-	tg.wantNotStale("mypkg", "./testgo list mypkg claims mypkg is stale, incorrectly")
+	tg.wantNotStale("mypkg", "", "./testgo list mypkg claims mypkg is stale, incorrectly")
 	// z.go was not part of the build; removing it is okay.
 	tg.must(os.Remove(tg.path("src/mypkg/z.go")))
-	tg.wantNotStale("mypkg", "./testgo list mypkg claims mypkg is stale after removing z.go; should not be stale")
+	tg.wantNotStale("mypkg", "", "./testgo list mypkg claims mypkg is stale after removing z.go; should not be stale")
 	// y.go was part of the package; removing it should be detected.
 	tg.must(os.Remove(tg.path("src/mypkg/y.go")))
-	tg.wantStale("mypkg", "./testgo list mypkg claims mypkg is NOT stale after removing y.go; should be stale")
+	tg.wantStale("mypkg", "build ID mismatch", "./testgo list mypkg claims mypkg is NOT stale after removing y.go; should be stale")
 }
 
 func TestWildcardMatchesSyntaxErrorDirs(t *testing.T) {
@@ -919,13 +930,13 @@ func TestGoInstallDetectsRemovedFilesInPackageMain(t *testing.T) {
 		package main`)
 	tg.setenv("GOPATH", tg.path("."))
 	tg.run("install", "mycmd")
-	tg.wantNotStale("mycmd", "./testgo list mypkg claims mycmd is stale, incorrectly")
+	tg.wantNotStale("mycmd", "", "./testgo list mypkg claims mycmd is stale, incorrectly")
 	// z.go was not part of the build; removing it is okay.
 	tg.must(os.Remove(tg.path("src/mycmd/z.go")))
-	tg.wantNotStale("mycmd", "./testgo list mycmd claims mycmd is stale after removing z.go; should not be stale")
+	tg.wantNotStale("mycmd", "", "./testgo list mycmd claims mycmd is stale after removing z.go; should not be stale")
 	// y.go was part of the package; removing it should be detected.
 	tg.must(os.Remove(tg.path("src/mycmd/y.go")))
-	tg.wantStale("mycmd", "./testgo list mycmd claims mycmd is NOT stale after removing y.go; should be stale")
+	tg.wantStale("mycmd", "build ID mismatch", "./testgo list mycmd claims mycmd is NOT stale after removing y.go; should be stale")
 }
 
 func testLocalRun(tg *testgoData, exepath, local, match string) {
@@ -1317,7 +1328,7 @@ func TestPackageMainTestImportsArchiveNotBinary(t *testing.T) {
 	tg.sleep()
 	tg.run("test", "main_test")
 	tg.run("install", "main_test")
-	tg.wantNotStale("main_test", "after go install, main listed as stale")
+	tg.wantNotStale("main_test", "", "after go install, main listed as stale")
 	tg.run("test", "main_test")
 }
 
@@ -1327,9 +1338,9 @@ func TestPackageNotStaleWithTrailingSlash(t *testing.T) {
 	defer tg.cleanup()
 	goroot := runtime.GOROOT()
 	tg.setenv("GOROOT", goroot+"/")
-	tg.wantNotStale("runtime", "with trailing slash in GOROOT, runtime listed as stale")
-	tg.wantNotStale("os", "with trailing slash in GOROOT, os listed as stale")
-	tg.wantNotStale("io", "with trailing slash in GOROOT, io listed as stale")
+	tg.wantNotStale("runtime", "", "with trailing slash in GOROOT, runtime listed as stale")
+	tg.wantNotStale("os", "", "with trailing slash in GOROOT, os listed as stale")
+	tg.wantNotStale("io", "", "with trailing slash in GOROOT, io listed as stale")
 }
 
 // With $GOBIN set, binaries get installed to $GOBIN.

diff --git a/src/cmd/go/list.go b/src/cmd/go/list.go
@@ -41,6 +41,7 @@ syntax of package template.  The default output is equivalent to -f
         Goroot        bool   // is this package in the Go root?
         Standard      bool   // is this package part of the standard Go library?
         Stale         bool   // would 'go install' do anything for this package?
+        StaleReason   string // explanation for Stale==true
         Root          string // Go root or Go path dir containing this package
 
         // Source files

diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go
@@ -39,6 +39,7 @@ type Package struct {
 	Goroot        bool   `json:",omitempty"` // is this package found in the Go root?
 	Standard      bool   `json:",omitempty"` // is this package part of the standard Go library?
 	Stale         bool   `json:",omitempty"` // would 'go install' do anything for this package?
+	StaleReason   string `json:",omitempty"` // why is Stale true?
 	Root          string `json:",omitempty"` // Go root or Go path dir containing this package
 	ConflictDir   string `json:",omitempty"` // Dir is hidden by this other directory
 
@@ -1085,7 +1086,7 @@ func packageList(roots []*Package) []*Package {
 // at the named pkgs (command-line arguments).
 func computeStale(pkgs ...*Package) {
 	for _, p := range packageList(pkgs) {
-		p.Stale = isStale(p)
+		p.Stale, p.StaleReason = isStale(p)
 	}
 }
 
@@ -1356,14 +1357,15 @@ var isGoRelease = strings.HasPrefix(runtime.Version(), "go1")
 // standard library, even in release versions. This makes
 // 'go build -tags netgo' work, among other things.
 
-// isStale reports whether package p needs to be rebuilt.
-func isStale(p *Package) bool {
+// isStale reports whether package p needs to be rebuilt,
+// along with the reason why.
+func isStale(p *Package) (bool, string) {
 	if p.Standard && (p.ImportPath == "unsafe" || buildContext.Compiler == "gccgo") {
 		// fake, builtin package
-		return false
+		return false, "builtin package"
 	}
 	if p.Error != nil {
-		return true
+		return true, "errors loading package"
 	}
 
 	// A package without Go sources means we only found
@@ -1373,23 +1375,26 @@ func isStale(p *Package) bool {
 	// only useful with the specific version of the toolchain that
 	// created them.
 	if len(p.gofiles) == 0 && !p.usesSwig() {
-		return false
+		return false, "no source files"
 	}
 
 	// If the -a flag is given, rebuild everything.
 	if buildA {
-		return true
+		return true, "build -a flag in use"
 	}
 
 	// If there's no install target or it's already marked stale, we have to rebuild.
-	if p.target == "" || p.Stale {
-		return true
+	if p.target == "" {
+		return true, "no install target"
+	}
+	if p.Stale {
+		return true, p.StaleReason
 	}
 
 	// Package is stale if completely unbuilt.
 	fi, err := os.Stat(p.target)
 	if err != nil {
-		return true
+		return true, "cannot stat install target"
 	}
 
 	// Package is stale if the expected build ID differs from the
@@ -1402,13 +1407,13 @@ func isStale(p *Package) bool {
 	// See issue 8290 and issue 10702.
 	targetBuildID, err := readBuildID(p)
 	if err == nil && targetBuildID != p.buildID {
-		return true
+		return true, "build ID mismatch"
 	}
 
 	// Package is stale if a dependency is.
 	for _, p1 := range p.deps {
 		if p1.Stale {
-			return true
+			return true, "stale dependency"
 		}
 	}
 
@@ -1431,7 +1436,7 @@ func isStale(p *Package) bool {
 	// install is to run make.bash, which will remove the old package archives
 	// before rebuilding.)
 	if p.Standard && isGoRelease {
-		return false
+		return false, "standard package in Go release distribution"
 	}
 
 	// Time-based staleness.
@@ -1446,7 +1451,7 @@ func isStale(p *Package) bool {
 	// Package is stale if a dependency is, or if a dependency is newer.
 	for _, p1 := range p.deps {
 		if p1.target != "" && olderThan(p1.target) {
-			return true
+			return true, "newer dependency"
 		}
 	}
 
@@ -1465,10 +1470,10 @@ func isStale(p *Package) bool {
 	// taken care of above (at least when the installed Go is a released version).
 	if p.Root != goroot {
 		if olderThan(buildToolchain.compiler()) {
-			return true
+			return true, "newer compiler"
 		}
 		if p.build.IsCommand() && olderThan(buildToolchain.linker()) {
-			return true
+			return true, "newer linker"
 		}
 	}
 
@@ -1513,11 +1518,11 @@ func isStale(p *Package) bool {
 	srcs := stringList(p.GoFiles, p.CFiles, p.CXXFiles, p.MFiles, p.HFiles, p.FFiles, p.SFiles, p.CgoFiles, p.SysoFiles, p.SwigFiles, p.SwigCXXFiles)
 	for _, src := range srcs {
 		if olderThan(filepath.Join(p.Dir, src)) {
-			return true
+			return true, "newer source file"
 		}
 	}
 
-	return false
+	return false, ""
 }
 
 // computeBuildID computes the build ID for p, leaving it in p.buildID.

diff --git a/src/cmd/go/test.go b/src/cmd/go/test.go
@@ -512,7 +512,8 @@ func runTest(cmd *Command, args []string) {
 				continue
 			}
 			p.Stale = true // rebuild
-			p.fake = true  // do not warn about rebuild
+			p.StaleReason = "rebuild for coverage"
+			p.fake = true // do not warn about rebuild
 			p.coverMode = testCoverMode
 			var coverFiles []string
 			coverFiles = append(coverFiles, p.GoFiles...)
@@ -749,6 +750,7 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
 		ptest.fake = true
 		ptest.forceLibrary = true
 		ptest.Stale = true
+		ptest.StaleReason = "rebuild for test"
 		ptest.build = new(build.Package)
 		*ptest.build = *p.build
 		m := map[string][]token.Position{}
@@ -1027,6 +1029,7 @@ func recompileForTest(pmain, preal, ptest *Package, testDir string) {
 				p.target = ""
 				p.fake = true
 				p.Stale = true
+				p.StaleReason = "depends on package being tested"
 			}
 		}