]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: do not build test packages unnecessarily during go vet
authorRuss Cox <rsc@golang.org>
Fri, 10 May 2019 01:56:53 +0000 (21:56 -0400)
committerRuss Cox <rsc@golang.org>
Wed, 15 May 2019 17:02:43 +0000 (17:02 +0000)
Vet needs export data for the imports of the package it is analyzing.
Vet does not need export data for the package itself, since vet will
do its own type checking. Assuming that vet is just as good as the compiler
at detecting invalid programs, don't run the compiler unnecessarily.

This especially matters for tests without external test files or for
which the external test files do not import the test-augmented original
package. In that case, the test-augmented original package need not
be compiled at all.

Cuts time for 'go clean -cache && go vet -x cmd/compile/internal/ssa'
from 7.6r 24.3u 2.8s to 3.5r 8.5u 1.9s, by not running the compiler
on the augmented test package.

There is still more to be done here - if we do need to build a
test-augmented package, we rerun cgo unnecessarily.
But this is a big help.

Cuts time for 'go vet std cmd' by about 30%.

For #31916.

Change-Id: If6136b4d384f1da77aed90b43f1a6b95f09b5d86
Reviewed-on: https://go-review.googlesource.com/c/go/+/176438
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
src/cmd/go/go_test.go
src/cmd/go/internal/work/action.go
src/cmd/go/internal/work/exec.go

index 1ec82ad532d863a4095c23967b844040ae923826..49b0e6d255e748c6dc88e2b4d31dcb09b7e4daed 100644 (file)
@@ -5263,8 +5263,14 @@ func TestCacheVet(t *testing.T) {
        if strings.Contains(os.Getenv("GODEBUG"), "gocacheverify") {
                t.Skip("GODEBUG gocacheverify")
        }
-       if cfg.Getenv("GOCACHE") == "off" {
-               tooSlow(t)
+       if testing.Short() {
+               // In short mode, reuse cache.
+               // Test failures may be masked if the cache has just the right entries already
+               // (not a concern during all.bash, which runs in a clean cache).
+               if cfg.Getenv("GOCACHE") == "off" {
+                       tooSlow(t)
+               }
+       } else {
                tg.makeTempdir()
                tg.setenv("GOCACHE", tg.path("cache"))
        }
index 7a74b1bb0d185238235113fb294b8c8da57e222e..f17137e6668d766be26c025a943304c0d50eeba6 100644 (file)
@@ -84,10 +84,11 @@ type Action struct {
        actionID cache.ActionID // cache ID of action input
        buildID  string         // build ID of action output
 
-       VetxOnly bool       // Mode=="vet": only being called to supply info about dependencies
-       needVet  bool       // Mode=="build": need to fill in vet config
-       vetCfg   *vetConfig // vet config
-       output   []byte     // output redirect buffer (nil means use b.Print)
+       VetxOnly  bool       // Mode=="vet": only being called to supply info about dependencies
+       needVet   bool       // Mode=="build": need to fill in vet config
+       needBuild bool       // Mode=="build": need to do actual build (can be false if needVet is true)
+       vetCfg    *vetConfig // vet config
+       output    []byte     // output redirect buffer (nil means use b.Print)
 
        // Execution state.
        pending  int  // number of deps yet to complete
@@ -212,6 +213,8 @@ const (
        ModeBuild BuildMode = iota
        ModeInstall
        ModeBuggyInstall
+
+       ModeVetOnly = 1 << 8
 )
 
 func (b *Builder) Init() {
@@ -354,6 +357,9 @@ func (b *Builder) AutoAction(mode, depMode BuildMode, p *load.Package) *Action {
 // depMode is the action (build or install) to use when building dependencies.
 // To turn package main into an executable, call b.Link instead.
 func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Action {
+       vetOnly := mode&ModeVetOnly != 0
+       mode &^= ModeVetOnly
+
        if mode != ModeBuild && (p.Internal.Local || p.Module != nil) && p.Target == "" {
                // Imported via local path or using modules. No permanent target.
                mode = ModeBuild
@@ -400,6 +406,19 @@ func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Actio
                return a
        })
 
+       // Find the build action; the cache entry may have been replaced
+       // by the install action during (*Builder).installAction.
+       buildAction := a
+       switch buildAction.Mode {
+       case "build", "built-in package":
+               // ok
+       case "build-install":
+               buildAction = a.Deps[0]
+       default:
+               panic("lost build action: " + buildAction.Mode)
+       }
+       buildAction.needBuild = buildAction.needBuild || !vetOnly
+
        // Construct install action.
        if mode == ModeInstall || mode == ModeBuggyInstall {
                a = b.installAction(a, mode)
@@ -421,7 +440,7 @@ func (b *Builder) VetAction(mode, depMode BuildMode, p *load.Package) *Action {
 func (b *Builder) vetAction(mode, depMode BuildMode, p *load.Package) *Action {
        // Construct vet action.
        a := b.cacheAction("vet", p, func() *Action {
-               a1 := b.CompileAction(mode, depMode, p)
+               a1 := b.CompileAction(mode|ModeVetOnly, depMode, p)
 
                // vet expects to be able to import "fmt".
                var stk load.ImportStack
index 6f8dca9b8938cf6e9597d176e0b698b992d46f87..9381bd6f1ebd56468995f0f67e64161a0b2bc67a 100644 (file)
@@ -367,8 +367,8 @@ func (b *Builder) build(a *Action) (err error) {
                return 0
        }
 
-       cached := false
-       need := bit(needBuild, !b.IsCmdList || b.NeedExport) |
+       cachedBuild := false
+       need := bit(needBuild, !b.IsCmdList && a.needBuild || b.NeedExport) |
                bit(needCgoHdr, b.needCgoHdr(a)) |
                bit(needVet, a.needVet) |
                bit(needCompiledGoFiles, b.NeedCompiledGoFiles)
@@ -377,6 +377,11 @@ func (b *Builder) build(a *Action) (err error) {
                if b.useCache(a, p, b.buildActionID(a), p.Target) {
                        // We found the main output in the cache.
                        // If we don't need any other outputs, we can stop.
+                       // Otherwise, we need to write files to a.Objdir (needVet, needCgoHdr).
+                       // Remember that we might have them in cache
+                       // and check again after we create a.Objdir.
+                       cachedBuild = true
+                       a.output = []byte{} // start saving output in case we miss any cache results
                        need &^= needBuild
                        if b.NeedExport {
                                p.Export = a.built
@@ -384,16 +389,11 @@ func (b *Builder) build(a *Action) (err error) {
                        if need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) {
                                need &^= needCompiledGoFiles
                        }
-                       // Otherwise, we need to write files to a.Objdir (needVet, needCgoHdr).
-                       // Remember that we might have them in cache
-                       // and check again after we create a.Objdir.
-                       cached = true
-                       a.output = []byte{} // start saving output in case we miss any cache results
                }
 
                // Source files might be cached, even if the full action is not
                // (e.g., go list -compiled -find).
-               if !cached && need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) {
+               if !cachedBuild && need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) {
                        need &^= needCompiledGoFiles
                }
 
@@ -438,21 +438,20 @@ func (b *Builder) build(a *Action) (err error) {
        }
        objdir := a.Objdir
 
-       if cached {
-               if need&needCgoHdr != 0 && b.loadCachedCgoHdr(a) {
-                       need &^= needCgoHdr
-               }
+       // Load cached cgo header, but only if we're skipping the main build (cachedBuild==true).
+       if cachedBuild && need&needCgoHdr != 0 && b.loadCachedCgoHdr(a) {
+               need &^= needCgoHdr
+       }
 
-               // Load cached vet config, but only if that's all we have left
-               // (need == needVet, not testing just the one bit).
-               // If we are going to do a full build anyway,
-               // we're going to regenerate the files below anyway.
-               if need == needVet && b.loadCachedVet(a) {
-                       need &^= needVet
-               }
-               if need == 0 {
-                       return nil
-               }
+       // Load cached vet config, but only if that's all we have left
+       // (need == needVet, not testing just the one bit).
+       // If we are going to do a full build anyway,
+       // we're going to regenerate the files below anyway.
+       if need == needVet && b.loadCachedVet(a) {
+               need &^= needVet
+       }
+       if need == 0 {
+               return nil
        }
 
        // make target directory