From 86463c157b063de47c8faef51a2f7c6de5fe4e7c Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 9 May 2019 22:57:45 -0400 Subject: [PATCH] cmd/vet/all: delete The work of running full vet on std and cmd during local development has moved to go test, which of course runs during all.bash. For errors in other GOOS/GOARCH combinations, the misc-compile builders (running buildall.bash) also now run go vet std cmd. The vetall builder need not do anything anymore. Make it a no-op until it can be retired, and remove cmd/vet/all and its special case in the go command. Fixes #31916. Change-Id: I8f30d184c382ea7c2c8f520e5618f680db633968 Reviewed-on: https://go-review.googlesource.com/c/go/+/176440 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/dist/test.go | 16 - src/cmd/go/internal/modload/init.go | 2 +- src/cmd/vet/all/main.go | 423 --------------------------- src/cmd/vet/all/whitelist/all.txt | 20 -- src/cmd/vet/all/whitelist/readme.txt | 4 - 5 files changed, 1 insertion(+), 464 deletions(-) delete mode 100644 src/cmd/vet/all/main.go delete mode 100644 src/cmd/vet/all/whitelist/all.txt delete mode 100644 src/cmd/vet/all/whitelist/readme.txt diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index ba8ba4e89e..8f9aabdbbd 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -383,22 +383,6 @@ func (t *tester) registerRaceBenchTest(pkg string) { var stdOutErrAreTerminals func() bool func (t *tester) registerTests() { - if strings.HasSuffix(os.Getenv("GO_BUILDER_NAME"), "-vetall") { - // Run vet over std and cmd and call it quits. - for k := range cgoEnabled { - osarch := k - t.tests = append(t.tests, distTest{ - name: "vet/" + osarch, - heading: "cmd/vet/all", - fn: func(dt *distTest) error { - t.addCmd(dt, "src/cmd/vet/all", "go", "run", "main.go", "-p="+osarch) - return nil - }, - }) - } - return - } - // Fast path to avoid the ~1 second of `go list std cmd` when // the caller lists specific tests to run. (as the continuous // build coordinator does). diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index b51e411421..3f3e8f8526 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -357,7 +357,7 @@ func InitMod() { func modFileToBuildList() { Target = modFile.Module.Mod targetPrefix = Target.Path - if rel := search.InDir(cwd, cfg.GOROOTsrc); rel != "" && rel != filepath.FromSlash("cmd/vet/all") { + if rel := search.InDir(cwd, cfg.GOROOTsrc); rel != "" { targetInGorootSrc = true if Target.Path == "std" { targetPrefix = "" diff --git a/src/cmd/vet/all/main.go b/src/cmd/vet/all/main.go deleted file mode 100644 index b917cb86e7..0000000000 --- a/src/cmd/vet/all/main.go +++ /dev/null @@ -1,423 +0,0 @@ -// Copyright 2016 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// +build ignore - -// The vet/all command runs go vet on the standard library and commands. -// It compares the output against a set of whitelists -// maintained in the whitelist directory. -// -// This program attempts to build packages from golang.org/x/tools, -// which must be in your GOPATH. -package main - -import ( - "bufio" - "bytes" - "flag" - "fmt" - "go/build" - "go/types" - "internal/testenv" - "io" - "io/ioutil" - "log" - "os" - "os/exec" - "path/filepath" - "runtime" - "strings" - "sync/atomic" -) - -var ( - flagPlatforms = flag.String("p", "", "platform(s) to use e.g. linux/amd64,darwin/386") - flagAll = flag.Bool("all", false, "run all platforms") - flagNoLines = flag.Bool("n", false, "don't print line numbers") -) - -var cmdGoPath string -var failed uint32 // updated atomically - -func main() { - log.SetPrefix("vet/all: ") - log.SetFlags(log.Lshortfile) - - var err error - cmdGoPath, err = testenv.GoTool() - if err != nil { - log.Print("could not find cmd/go; skipping") - // We're on a platform that can't run cmd/go. - // We want this script to be able to run as part of all.bash, - // so return cleanly rather than with exit code 1. - return - } - - flag.Parse() - switch { - case *flagAll && *flagPlatforms != "": - log.Print("-all and -p flags are incompatible") - flag.Usage() - os.Exit(2) - case *flagPlatforms != "": - vetPlatforms(parseFlagPlatforms()) - case *flagAll: - vetPlatforms(allPlatforms()) - default: - hostPlatform.vet() - } - if atomic.LoadUint32(&failed) != 0 { - os.Exit(1) - } -} - -var hostPlatform = platform{os: build.Default.GOOS, arch: build.Default.GOARCH} - -func allPlatforms() []platform { - var pp []platform - cmd := exec.Command(cmdGoPath, "tool", "dist", "list") - cmd.Stderr = new(strings.Builder) - out, err := cmd.Output() - if err != nil { - log.Fatalf("%s: %v\n%s", strings.Join(cmd.Args, " "), err, cmd.Stderr) - } - lines := bytes.Split(out, []byte{'\n'}) - for _, line := range lines { - if len(line) == 0 { - continue - } - pp = append(pp, parsePlatform(string(line))) - } - return pp -} - -func parseFlagPlatforms() []platform { - var pp []platform - components := strings.Split(*flagPlatforms, ",") - for _, c := range components { - pp = append(pp, parsePlatform(c)) - } - return pp -} - -func parsePlatform(s string) platform { - vv := strings.Split(s, "/") - if len(vv) != 2 { - log.Fatalf("could not parse platform %s, must be of form goos/goarch", s) - } - return platform{os: vv[0], arch: vv[1]} -} - -type whitelist map[string]int - -// load adds entries from the whitelist file, if present, for os/arch to w. -func (w whitelist) load(goos string, goarch string) { - sz := types.SizesFor("gc", goarch) - if sz == nil { - log.Fatalf("unknown type sizes for arch %q", goarch) - } - archbits := 8 * sz.Sizeof(types.Typ[types.UnsafePointer]) - - // Look up whether goarch has a shared arch suffix, - // such as mips64x for mips64 and mips64le. - archsuff := goarch - if x, ok := archAsmX[goarch]; ok { - archsuff = x - } - - // Load whitelists. - filenames := []string{ - "all.txt", - goos + ".txt", - goarch + ".txt", - goos + "_" + goarch + ".txt", - fmt.Sprintf("%dbit.txt", archbits), - } - if goarch != archsuff { - filenames = append(filenames, - archsuff+".txt", - goos+"_"+archsuff+".txt", - ) - } - - // We allow error message templates using GOOS and GOARCH. - if goos == "android" { - goos = "linux" // so many special cases :( - } - - // Read whitelists and do template substitution. - replace := strings.NewReplacer("GOOS", goos, "GOARCH", goarch, "ARCHSUFF", archsuff) - - for _, filename := range filenames { - path := filepath.Join("whitelist", filename) - f, err := os.Open(path) - if err != nil { - // Allow not-exist errors; not all combinations have whitelists. - if os.IsNotExist(err) { - continue - } - log.Fatal(err) - } - scan := bufio.NewScanner(f) - for scan.Scan() { - line := scan.Text() - if len(line) == 0 || strings.HasPrefix(line, "//") { - continue - } - w[replace.Replace(line)]++ - } - if err := scan.Err(); err != nil { - log.Fatal(err) - } - } -} - -type platform struct { - os string - arch string -} - -func (p platform) String() string { - return p.os + "/" + p.arch -} - -// ignorePathPrefixes are file path prefixes that should be ignored wholesale. -var ignorePathPrefixes = [...]string{ - // These testdata dirs have lots of intentionally broken/bad code for tests. - "cmd/go/testdata/", - "cmd/vet/testdata/", - "go/printer/testdata/", -} - -func vetPlatforms(pp []platform) { - for _, p := range pp { - p.vet() - } -} - -func (p platform) vet() { - if p.os == "linux" && (p.arch == "riscv64" || p.arch == "sparc64") { - // TODO(tklauser): enable as soon as these ports have fully landed - fmt.Printf("skipping %s/%s\n", p.os, p.arch) - return - } - - if p.os == "windows" && p.arch == "arm" { - // TODO(jordanrh1): enable as soon as the windows/arm port has fully landed - fmt.Println("skipping windows/arm") - return - } - - var buf bytes.Buffer - fmt.Fprintf(&buf, "go run main.go -p %s\n", p) - - // Load whitelist(s). - w := make(whitelist) - w.load(p.os, p.arch) - - var vetCmd []string - - if os.Getenv("GO_BUILDER_NAME") == "" { - vetCmd = []string{cmdGoPath, "vet"} - } else { - // Build the go/packages-based vet command from the x/tools - // repo. It is considerably faster than "go vet", which rebuilds - // the standard library. - tmpdir, err := ioutil.TempDir("", "cmd-vet-all") - if err != nil { - log.Fatal(err) - } - defer os.RemoveAll(tmpdir) - - vetTool := filepath.Join(tmpdir, "vet") - vetCmd = []string{ - vetTool, - // "-nilness=0", // expensive, uses SSA - } - - cmd := exec.Command(cmdGoPath, "build", "-o", vetTool, "golang.org/x/tools/go/analysis/cmd/vet") - cmd.Env = append(os.Environ(), - // Setting GO111MODULE to on is redundant in master - // (Go 1.13), but not if we backport this to Go 1.11/1.12 - // release branches (for our own builder usage) or if - // master ends up reverting its GO111MODULE default. If - // that happens, we want to force it on here anyway, as - // we're now depending on it. - "GO111MODULE=on", - ) - // Use the module that cmd/vet/all is a part of: - cmd.Dir = filepath.Join(runtime.GOROOT(), "src", "cmd", "vet", "all") - - // golang.org/x/tools does not have a vendor directory, so don't try to use - // one in module mode. - for i, v := range cmd.Env { - if strings.HasPrefix(v, "GOFLAGS=") { - var goflags []string - for _, f := range strings.Fields(strings.TrimPrefix(v, "GOFLAGS=")) { - if f != "-mod=vendor" && f != "--mod=vendor" { - goflags = append(goflags, f) - } - } - cmd.Env[i] = strings.Join(goflags, " ") - } - } - - cmd.Stderr = os.Stderr - cmd.Stdout = os.Stderr - if err := cmd.Run(); err != nil { - log.Fatalf("%s: %v", strings.Join(cmd.Args, " "), err) - } - } - - // TODO: The unsafeptr checks are disabled for now, - // because there are so many false positives, - // and no clear way to improve vet to eliminate large chunks of them. - // And having them in the whitelists will just cause annoyance - // and churn when working on the runtime. - cmd := exec.Command(vetCmd[0], - append(vetCmd[1:], - "-unsafeptr=0", - "std", - "cmd/...", - "cmd/compile/internal/gc/testdata", - )...) - cmd.Dir = filepath.Join(runtime.GOROOT(), "src") - cmd.Env = append(os.Environ(), "GOOS="+p.os, "GOARCH="+p.arch, "CGO_ENABLED=0") - stderr, err := cmd.StderrPipe() - if err != nil { - log.Fatal(err) - } - if err := cmd.Start(); err != nil { - log.Fatal(err) - } - - // Process vet output. - scan := bufio.NewScanner(stderr) - var parseFailed bool -NextLine: - for scan.Scan() { - line := scan.Text() - if strings.HasPrefix(line, "vet: ") { - // Typecheck failure: Malformed syntax or multiple packages or the like. - // This will yield nicer error messages elsewhere, so ignore them here. - - // This includes warnings from asmdecl of the form: - // "vet: foo.s:16: [amd64] cannot check cross-package assembly function" - continue - } - - if strings.HasPrefix(line, "panic: ") { - // Panic in vet. Don't filter anything, we want the complete output. - parseFailed = true - fmt.Fprintf(os.Stderr, "panic in vet (to reproduce: go run main.go -p %s):\n", p) - fmt.Fprintln(os.Stderr, line) - io.Copy(os.Stderr, stderr) - break - } - if strings.HasPrefix(line, "# ") { - // 'go vet' prefixes the output of each vet invocation by a comment: - // # [package] - continue - } - - // Parse line. - // Assume the part before the first ": " - // is the "file:line:col: " information. - // TODO(adonovan): parse vet -json output. - var file, lineno, msg string - if i := strings.Index(line, ": "); i >= 0 { - msg = line[i+len(": "):] - - words := strings.Split(line[:i], ":") - switch len(words) { - case 3: - _ = words[2] // ignore column - fallthrough - case 2: - lineno = words[1] - fallthrough - case 1: - file = words[0] - - // Make the file name relative to GOROOT/src. - if rel, err := filepath.Rel(cmd.Dir, file); err == nil { - file = rel - } - default: - // error: too many columns - } - } - if file == "" { - if !parseFailed { - parseFailed = true - fmt.Fprintf(os.Stderr, "failed to parse %s output:\n# %s\n", p, strings.Join(cmd.Args, " ")) - } - fmt.Fprintln(os.Stderr, line) - continue - } - - msg = strings.TrimSpace(msg) - - for _, ignore := range ignorePathPrefixes { - if strings.HasPrefix(file, filepath.FromSlash(ignore)) { - continue NextLine - } - } - - key := file + ": " + msg - if w[key] == 0 { - // Vet error with no match in the whitelist. Print it. - if *flagNoLines { - fmt.Fprintf(&buf, "%s: %s\n", file, msg) - } else { - fmt.Fprintf(&buf, "%s:%s: %s\n", file, lineno, msg) - } - atomic.StoreUint32(&failed, 1) - continue - } - w[key]-- - } - if parseFailed { - atomic.StoreUint32(&failed, 1) - return - } - if scan.Err() != nil { - log.Fatalf("failed to scan vet output: %v", scan.Err()) - } - err = cmd.Wait() - // We expect vet to fail. - // Make sure it has failed appropriately, though (for example, not a PathError). - if _, ok := err.(*exec.ExitError); !ok { - log.Fatalf("unexpected go vet execution failure: %v", err) - } - printedHeader := false - if len(w) > 0 { - for k, v := range w { - if v != 0 { - if !printedHeader { - fmt.Fprintln(&buf, "unmatched whitelist entries:") - printedHeader = true - } - for i := 0; i < v; i++ { - fmt.Fprintln(&buf, k) - } - atomic.StoreUint32(&failed, 1) - } - } - } - - os.Stdout.Write(buf.Bytes()) -} - -// archAsmX maps architectures to the suffix usually used for their assembly files, -// if different than the arch name itself. -var archAsmX = map[string]string{ - "android": "linux", - "mips64": "mips64x", - "mips64le": "mips64x", - "mips": "mipsx", - "mipsle": "mipsx", - "ppc64": "ppc64x", - "ppc64le": "ppc64x", -} diff --git a/src/cmd/vet/all/whitelist/all.txt b/src/cmd/vet/all/whitelist/all.txt deleted file mode 100644 index 6148679571..0000000000 --- a/src/cmd/vet/all/whitelist/all.txt +++ /dev/null @@ -1,20 +0,0 @@ -// Non-platform-specific vet whitelist. See readme.txt for details. - -// Compiler tests that make sure even vet-failing code adheres to the spec. -cmd/compile/internal/gc/testdata/arithConst_test.go: a (64 bits) too small for shift of 4294967296 -cmd/compile/internal/gc/testdata/arithConst_test.go: a (64 bits) too small for shift of 4294967296 -cmd/compile/internal/gc/testdata/arithConst_test.go: a (32 bits) too small for shift of 4294967295 -cmd/compile/internal/gc/testdata/arithConst_test.go: a (32 bits) too small for shift of 4294967295 -cmd/compile/internal/gc/testdata/arithConst_test.go: a (16 bits) too small for shift of 65535 -cmd/compile/internal/gc/testdata/arithConst_test.go: a (16 bits) too small for shift of 65535 -cmd/compile/internal/gc/testdata/arithConst_test.go: a (8 bits) too small for shift of 255 -cmd/compile/internal/gc/testdata/arithConst_test.go: a (8 bits) too small for shift of 255 -cmd/compile/internal/gc/testdata/arith_test.go: x (64 bits) too small for shift of 100 -cmd/compile/internal/gc/testdata/arith_test.go: int32(x) (32 bits) too small for shift of 4294967295 -cmd/compile/internal/gc/testdata/arith_test.go: int16(x) (16 bits) too small for shift of 65535 -cmd/compile/internal/gc/testdata/arith_test.go: int8(x) (8 bits) too small for shift of 255 -cmd/compile/internal/gc/testdata/arith_test.go: w (32 bits) too small for shift of 32 -cmd/compile/internal/gc/testdata/break_test.go: unreachable code -cmd/compile/internal/gc/testdata/break_test.go: unreachable code -cmd/compile/internal/gc/testdata/namedReturn_test.go: self-assignment of t to t -cmd/compile/internal/gc/testdata/short_test.go: unreachable code diff --git a/src/cmd/vet/all/whitelist/readme.txt b/src/cmd/vet/all/whitelist/readme.txt deleted file mode 100644 index 4f83757dbc..0000000000 --- a/src/cmd/vet/all/whitelist/readme.txt +++ /dev/null @@ -1,4 +0,0 @@ -This directory contains whitelists for vet complaints about the standard library and commands. -They are line-based and unordered, although counts of duplicated lines matter. -Each line matches vet's output, except that line numbers are removed to avoid churn. -There are also os-, arch-, and bitwidth-specific whitelists. -- 2.48.1