mesh-build-go: lint only the packages a unit level runs, in one vet pass with every tag unless a tag leaves a file out, so a small change's lint costs a few packages (issue 1921, try speed S3)
This commit is contained in:
@@ -60,17 +60,23 @@ func lintable(dir string, each func(path string) error) error {
|
||||
})
|
||||
}
|
||||
|
||||
// unformatted are the Go files of a directory gofmt would change, relative to the tree, and the build tags they name.
|
||||
func unformatted(tree, d string) (files, tags []string, err error) {
|
||||
// unformatted are the Go files of a directory gofmt would change, relative to the tree, the build tags the directory's
|
||||
// files name, and whether a file is left out by one of them (a negated tag, //go:build !integration). Only files whose
|
||||
// directory (relative to the go.mod directory, "." for its own) is in only are checked for gofmt; nil checks every file.
|
||||
func unformatted(tree, d string, only map[string]bool) (files, tags []string, negated bool, err error) {
|
||||
seen := map[string]bool{}
|
||||
err = lintable(filepath.Join(tree, d), func(path string) error {
|
||||
base := filepath.Join(tree, d)
|
||||
err = lintable(base, func(path string) error {
|
||||
src, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if out, ferr := format.Source(src); ferr != nil || !bytes.Equal(out, src) {
|
||||
rel, _ := filepath.Rel(tree, path)
|
||||
files = append(files, filepath.ToSlash(rel))
|
||||
pkgDir, _ := filepath.Rel(base, filepath.Dir(path))
|
||||
if only == nil || only[filepath.ToSlash(pkgDir)] {
|
||||
if out, ferr := format.Source(src); ferr != nil || !bytes.Equal(out, src) {
|
||||
rel, _ := filepath.Rel(tree, path)
|
||||
files = append(files, filepath.ToSlash(rel))
|
||||
}
|
||||
}
|
||||
scan := bufio.NewScanner(bytes.NewReader(src))
|
||||
for scan.Scan() {
|
||||
@@ -79,8 +85,15 @@ func unformatted(tree, d string) (files, tags []string, err error) {
|
||||
break
|
||||
}
|
||||
if expr, ok := strings.CutPrefix(line, "//go:build "); ok {
|
||||
for _, tag := range buildTag.FindAllString(expr, -1) {
|
||||
if !notATag[tag] && !strings.HasPrefix(tag, "go1") && !seen[tag] {
|
||||
for _, at := range buildTag.FindAllStringIndex(expr, -1) {
|
||||
tag := expr[at[0]:at[1]]
|
||||
if notATag[tag] || strings.HasPrefix(tag, "go1") {
|
||||
continue
|
||||
}
|
||||
if before := strings.TrimRight(expr[:at[0]], " ("); strings.HasSuffix(before, "!") {
|
||||
negated = true
|
||||
}
|
||||
if !seen[tag] {
|
||||
seen[tag] = true
|
||||
tags = append(tags, tag)
|
||||
}
|
||||
@@ -91,13 +104,43 @@ func unformatted(tree, d string) (files, tags []string, err error) {
|
||||
})
|
||||
sort.Strings(files)
|
||||
sort.Strings(tags)
|
||||
return files, tags, err
|
||||
return files, tags, negated, err
|
||||
}
|
||||
|
||||
// lint is gofmt and go vet of one go.mod directory d of the tree, run with run in the directory's go environment; a
|
||||
// failure of either is a Failure of the level naming what failed.
|
||||
func (m *Module) lint(ctx context.Context, run forgekey.Runner, tree, d string, env []string) ([]Failure, error) {
|
||||
files, tags, err := unformatted(tree, d)
|
||||
// lintScope is what of a go.mod directory d the lint covers, from the patterns the level runs there: every package
|
||||
// (./..., and the directories nil) when a pattern is relative or none is given, else the import paths given and the
|
||||
// directories of their files, relative to d.
|
||||
func lintScope(tree, d string, patterns []string) (targets []string, dirs map[string]bool) {
|
||||
if len(patterns) == 0 {
|
||||
return []string{"./..."}, nil
|
||||
}
|
||||
raw, _ := os.ReadFile(filepath.Join(tree, d, "go.mod"))
|
||||
mp := modulePath(string(raw))
|
||||
dirs = map[string]bool{}
|
||||
for _, p := range patterns {
|
||||
if strings.HasPrefix(p, "./") || p == "." {
|
||||
return []string{"./..."}, nil
|
||||
}
|
||||
rel, ok := strings.CutPrefix(p, mp)
|
||||
if mp == "" || !ok || (rel != "" && !strings.HasPrefix(rel, "/")) {
|
||||
return []string{"./..."}, nil // a package of no module here: the whole directory, as before
|
||||
}
|
||||
if rel = strings.TrimPrefix(rel, "/"); rel == "" {
|
||||
rel = "."
|
||||
}
|
||||
dirs[rel] = true
|
||||
targets = append(targets, p)
|
||||
}
|
||||
return targets, dirs
|
||||
}
|
||||
|
||||
// lint is gofmt and go vet of one go.mod directory d of the tree, run with run in the directory's go environment, over
|
||||
// the packages the level runs there (patterns, as packagesByDir gives them; issue 1921, try speed S3); a failure of either
|
||||
// is a Failure of the level naming what failed. go vet runs once with every tag the directory names, which vets the
|
||||
// untagged files too; only a directory where a tag leaves a file out vets also without tags, both passes at once.
|
||||
func (m *Module) lint(ctx context.Context, run forgekey.Runner, tree, d string, env, patterns []string) ([]Failure, error) {
|
||||
targets, only := lintScope(tree, d, patterns)
|
||||
files, tags, negated, err := unformatted(tree, d, only)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("reading the Go files of %s: %w", d, err)
|
||||
}
|
||||
@@ -106,32 +149,47 @@ func (m *Module) lint(ctx context.Context, run forgekey.Runner, tree, d string,
|
||||
failed = append(failed, Failure{Package: d, Test: "gofmt",
|
||||
Output: "not as gofmt writes them (run gofmt -w): " + strings.Join(files, ", ")})
|
||||
}
|
||||
sets := [][]string{nil}
|
||||
if len(tags) > 0 {
|
||||
sets = append(sets, tags)
|
||||
sets := [][]string{tags}
|
||||
if len(tags) > 0 && negated {
|
||||
sets = [][]string{nil, tags}
|
||||
}
|
||||
for _, set := range sets {
|
||||
type vetRun struct {
|
||||
name string
|
||||
r forgekey.Ran
|
||||
}
|
||||
runs := make([]vetRun, len(sets))
|
||||
var wg sync.WaitGroup
|
||||
for i, set := range sets {
|
||||
args := []string{"-C", filepath.Join(tree, d), "vet"}
|
||||
name := "go vet"
|
||||
if len(set) > 0 {
|
||||
args = append(args, "-tags", strings.Join(set, ","))
|
||||
name += " -tags " + strings.Join(set, ",")
|
||||
}
|
||||
args = append(args, "./...")
|
||||
r := run(ctx, env, "go", args...)
|
||||
if r.Err != "" {
|
||||
return nil, fmt.Errorf("%s in %s could not be run: %s", name, d, firstLine(r.Err+"\n"+r.Stderr))
|
||||
args = append(args, targets...)
|
||||
runs[i].name = name
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
runs[i].r = run(ctx, env, "go", args...)
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
for _, v := range runs {
|
||||
if v.r.Err != "" {
|
||||
return nil, fmt.Errorf("%s in %s could not be run: %s", v.name, d, firstLine(v.r.Err+"\n"+v.r.Stderr))
|
||||
}
|
||||
if r.Status != 0 {
|
||||
failed = append(failed, Failure{Package: d, Test: name, Output: strings.TrimSpace(r.Stderr + "\n" + r.Stdout)})
|
||||
if v.r.Status != 0 {
|
||||
failed = append(failed, Failure{Package: d, Test: v.name, Output: strings.TrimSpace(v.r.Stderr + "\n" + v.r.Stdout)})
|
||||
}
|
||||
}
|
||||
return failed, nil
|
||||
}
|
||||
|
||||
// lintAll lints each directory, up to m.parallel() at once like the level's tests; the failures join in directory order
|
||||
// and the error is the first directory's, so the answer does not depend on which ended first (issue 1930).
|
||||
func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string, dirs []string, env func(d string) []string) ([]Failure, error) {
|
||||
// and the error is the first directory's, so the answer does not depend on which ended first (issue 1930). byDir names
|
||||
// the packages the level runs in each directory (packagesByDir); nil lints each directory whole.
|
||||
func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string, dirs []string, env func(d string) []string, byDir map[string][]string) ([]Failure, error) {
|
||||
type answer struct {
|
||||
failed []Failure
|
||||
err error
|
||||
@@ -154,7 +212,7 @@ func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string,
|
||||
crashMu.Unlock()
|
||||
}
|
||||
}()
|
||||
answers[i].failed, answers[i].err = m.lint(ctx, run, tree, d, env(d))
|
||||
answers[i].failed, answers[i].err = m.lint(ctx, run, tree, d, env(d), byDir[d])
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
@@ -28,8 +29,9 @@ func writeTree(t *testing.T, files map[string]string) string {
|
||||
}
|
||||
|
||||
// 1887/D7-go, ADR 0529 §1: an unformatted file fails the unit level naming the file (testdata and vendor are left
|
||||
// alone), and go vet runs with no tag and with every tag the files name together, integration and isolation included, so
|
||||
// a vet error behind isolation fails the level naming the run.
|
||||
// alone), and go vet runs with every tag the files name together, integration and isolation included, so a vet error
|
||||
// behind isolation fails the level naming the run. One pass with every tag vets the untagged files too (issue 1921, try
|
||||
// speed S3), as no file here is left out by a tag.
|
||||
func TestTheUnitLevelRunsGofmtAndVetWithEveryTag(t *testing.T) {
|
||||
tree := writeTree(t, map[string]string{
|
||||
"go.mod": "module x\n\ngo 1.26\n",
|
||||
@@ -51,11 +53,11 @@ func TestTheUnitLevelRunsGofmtAndVetWithEveryTag(t *testing.T) {
|
||||
}
|
||||
return forgekey.Ran{}
|
||||
}
|
||||
failed, err := (&Module{}).lint(context.Background(), run, tree, ".", nil)
|
||||
failed, err := (&Module{}).lint(context.Background(), run, tree, ".", nil, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Join(vetted, " | ") != "vet ./... | vet -tags integration,isolation ./..." {
|
||||
if strings.Join(vetted, " | ") != "vet -tags integration,isolation ./..." {
|
||||
t.Errorf("vet ran %v", vetted)
|
||||
}
|
||||
if len(failed) != 2 || failed[0].Test != "gofmt" || !strings.Contains(failed[0].Output, "b.go") ||
|
||||
@@ -67,7 +69,7 @@ func TestTheUnitLevelRunsGofmtAndVetWithEveryTag(t *testing.T) {
|
||||
}
|
||||
// A formatted directory that vets passes.
|
||||
clean := writeTree(t, map[string]string{"go.mod": "module y\n", "a.go": "package y\n"})
|
||||
if failed, err := (&Module{}).lint(context.Background(), func(context.Context, []string, string, ...string) forgekey.Ran { return forgekey.Ran{} }, clean, ".", nil); err != nil || len(failed) != 0 {
|
||||
if failed, err := (&Module{}).lint(context.Background(), func(context.Context, []string, string, ...string) forgekey.Ran { return forgekey.Ran{} }, clean, ".", nil, nil); err != nil || len(failed) != 0 {
|
||||
t.Errorf("a clean directory: %+v, %v", failed, err)
|
||||
}
|
||||
}
|
||||
@@ -121,7 +123,7 @@ func TestTheLintOfManyDirectoriesRunsAtOnceUpToTheBound(t *testing.T) {
|
||||
return forgekey.Ran{}
|
||||
}
|
||||
m := &Module{Parallel: 3}
|
||||
failed, err := m.lintAll(context.Background(), run, tree, dirs, func(string) []string { return nil })
|
||||
failed, err := m.lintAll(context.Background(), run, tree, dirs, func(string) []string { return nil }, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -132,3 +134,68 @@ func TestTheLintOfManyDirectoriesRunsAtOnceUpToTheBound(t *testing.T) {
|
||||
t.Errorf("failed = %+v, want b's then e's", failed)
|
||||
}
|
||||
}
|
||||
|
||||
// 1921/S3: the lint of a level that runs some packages vets those packages alone, in one pass with every tag their
|
||||
// directory names, and reports a gofmt fault only in their files: a fault in a package the level keeps was reported
|
||||
// when that package changed.
|
||||
func TestTheLintOfSomePackagesVetsThemInOnePassAndFormatsOnlyTheirFiles(t *testing.T) {
|
||||
tree := writeTree(t, map[string]string{
|
||||
"go.mod": "module x\n\ngo 1.26\n",
|
||||
"a/a.go": "package a\n\nfunc A() {}\n",
|
||||
"a/a_integration_test.go": "//go:build integration\n\npackage a\n",
|
||||
"b/b.go": "package b\nfunc B() {}\n",
|
||||
"a/deeper/d.go": "package deeper\nfunc D() {}\n",
|
||||
})
|
||||
var vetted []string
|
||||
run := func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran {
|
||||
vetted = append(vetted, strings.Join(args[2:], " "))
|
||||
return forgekey.Ran{}
|
||||
}
|
||||
failed, err := (&Module{}).lint(context.Background(), run, tree, ".", nil, []string{"x/a"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Join(vetted, " | ") != "vet -tags integration x/a" {
|
||||
t.Errorf("vet ran %v, want one pass over x/a with its tag", vetted)
|
||||
}
|
||||
if len(failed) != 0 {
|
||||
t.Errorf("the faults of packages the level keeps were reported: %+v", failed)
|
||||
}
|
||||
vetted = nil
|
||||
failed, err = (&Module{}).lint(context.Background(), run, tree, ".", nil, []string{"x/b"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(failed) != 1 || failed[0].Test != "gofmt" || !strings.Contains(failed[0].Output, "b/b.go") || strings.Contains(failed[0].Output, "d.go") {
|
||||
t.Errorf("failed = %+v, want gofmt naming b/b.go alone", failed)
|
||||
}
|
||||
if strings.Join(vetted, " | ") != "vet -tags integration x/b" {
|
||||
t.Errorf("vet ran %v", vetted)
|
||||
}
|
||||
}
|
||||
|
||||
// 1921/S3: a file a tag leaves out (//go:build !integration) is vetted by no pass with the tag, so a directory holding
|
||||
// one keeps the pass without tags beside the pass with them.
|
||||
func TestADirectoryWithAFileATagLeavesOutKeepsThePassWithoutTags(t *testing.T) {
|
||||
tree := writeTree(t, map[string]string{
|
||||
"go.mod": "module x\n",
|
||||
"a.go": "package x\n",
|
||||
"fake.go": "//go:build !integration\n\npackage x\n",
|
||||
"real_test.go": "//go:build integration\n\npackage x\n",
|
||||
})
|
||||
var mu sync.Mutex
|
||||
var vetted []string
|
||||
run := func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran {
|
||||
mu.Lock()
|
||||
vetted = append(vetted, strings.Join(args[2:], " "))
|
||||
mu.Unlock()
|
||||
return forgekey.Ran{}
|
||||
}
|
||||
if _, err := (&Module{}).lint(context.Background(), run, tree, ".", nil, nil); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
slices.Sort(vetted)
|
||||
if strings.Join(vetted, " | ") != "vet -tags integration ./... | vet ./..." {
|
||||
t.Errorf("vet ran %v, want the pass without tags and the pass with them", vetted)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -211,7 +211,7 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR
|
||||
linted := m.clock()
|
||||
failed, err := m.lintAll(ctx, run, tree, todo, func(d string) []string {
|
||||
return append(m.goEnvFor(filepath.Join(tree, d)), o.env...)
|
||||
})
|
||||
}, byDir)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user