mesh-build-go requires the checker client it imports; a check refuses any module importing a mesh module its go.mod does not require (issue 1921, issue 1966)
mesh/check failed (4): unit: git.novox.be/novox/m…; and 3 more | 62fe1cc36d30 on f2b332de73d6 | judged by the candidate's mesh-build-go at e8757a23634d
mesh/delivery-group group feat/registry-milestone rejected: composed together: the checker's verdict at the members' heads (8a80e8e86858): unit: git.novox.be/n…
mesh/delivery closed: a newer head of the same pull request
mesh/check failed (4): unit: git.novox.be/novox/m…; and 3 more | 62fe1cc36d30 on f2b332de73d6 | judged by the candidate's mesh-build-go at e8757a23634d
mesh/delivery-group group feat/registry-milestone rejected: composed together: the checker's verdict at the members' heads (8a80e8e86858): unit: git.novox.be/n…
mesh/delivery closed: a newer head of the same pull request
Quick fix: add the one require to mesh-build-go's go.mod (a minute). Thorough fix, chosen because the kind was met twice (a local go.work resolving what go.mod never names) and hides itself from every local build: checks/requiredmodules reads every go.mod and Go source (tests and tagged files included), finds the owning module by the longest module line among the repositories beside, and fails in go test and the repository check before any push. The scan over the seven group heads found this one instance. Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i
This commit is contained in:
@@ -0,0 +1,3 @@
|
||||
module git.novox.be/novox/mesh-catalog/checks/requiredmodules
|
||||
|
||||
go 1.22
|
||||
@@ -0,0 +1,204 @@
|
||||
// Command requiredmodules refuses a Go module that imports a package of a mesh module its go.mod does not require
|
||||
// (issue 1921, issue 1966; the second time a local workspace hid it: the candidate's build resolves only what go.mod says).
|
||||
//
|
||||
// A developer's go.work resolves every sibling module, so a source can import a package whose module the go.mod never
|
||||
// names and still build; the build seat builds against the go.mod alone and then fails with "finding module for
|
||||
// package". This check reads every go.mod under the roots it reports and every Go source beside it (test files and files
|
||||
// behind a build tag included, testdata, vendor and node_modules left out) and, for each import under
|
||||
// git.novox.be/novox/, finds the module that owns the package (the longest `module` line, among the modules of every
|
||||
// root it is given, that is a prefix of the import) and demands that module to be the go.mod's own or one it requires.
|
||||
// Where no known module owns the import, any own or required module that is a prefix of it satisfies it.
|
||||
//
|
||||
// go run . <root> [<root> ...]
|
||||
//
|
||||
// The first root is reported; the others (the repositories cloned beside, MESH_CHECK_BESIDE) only say which module owns
|
||||
// which path. With -all every root is reported. Exits 1 on a finding.
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
const prefix = "git.novox.be/novox/"
|
||||
|
||||
// Mod is a go.mod: where it is, its module path and the modules it requires.
|
||||
type Mod struct {
|
||||
Dir string
|
||||
Path string
|
||||
Requires map[string]bool
|
||||
}
|
||||
|
||||
// Modules reads every go.mod under root.
|
||||
func Modules(root string) ([]Mod, error) {
|
||||
var out []Mod
|
||||
err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if d.IsDir() {
|
||||
switch d.Name() {
|
||||
case "testdata", "vendor", "node_modules", ".git":
|
||||
return fs.SkipDir
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if d.Name() != "go.mod" {
|
||||
return nil
|
||||
}
|
||||
raw, err := os.ReadFile(p)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
m := Mod{Dir: filepath.Dir(p), Requires: map[string]bool{}}
|
||||
in := false
|
||||
for _, line := range strings.Split(string(raw), "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
if i := strings.Index(line, "//"); i >= 0 {
|
||||
line = strings.TrimSpace(line[:i])
|
||||
}
|
||||
f := strings.Fields(line)
|
||||
switch {
|
||||
case len(f) == 2 && f[0] == "module":
|
||||
m.Path = strings.Trim(f[1], `"`)
|
||||
case len(f) >= 1 && f[0] == "require" && len(f) >= 2 && f[1] == "(":
|
||||
in = true
|
||||
case line == ")":
|
||||
in = false
|
||||
case len(f) >= 3 && f[0] == "require":
|
||||
m.Requires[f[1]] = true
|
||||
case in && len(f) >= 2:
|
||||
m.Requires[f[0]] = true
|
||||
}
|
||||
}
|
||||
if m.Path == "" {
|
||||
return fmt.Errorf("%s names no module", p)
|
||||
}
|
||||
out = append(out, m)
|
||||
return nil
|
||||
})
|
||||
return out, err
|
||||
}
|
||||
|
||||
func within(path, module string) bool { return path == module || strings.HasPrefix(path, module+"/") }
|
||||
|
||||
// Findings reports, for every go.mod under the report roots, each imported package whose module the go.mod neither is nor
|
||||
// requires; known are the other roots that say which module owns a path.
|
||||
func Findings(report, known []string) ([]string, error) {
|
||||
var owners []string
|
||||
var reported []Mod
|
||||
for _, r := range report {
|
||||
ms, err := Modules(r)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
reported = append(reported, ms...)
|
||||
}
|
||||
for _, r := range append(append([]string{}, report...), known...) {
|
||||
ms, err := Modules(r)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, m := range ms {
|
||||
owners = append(owners, m.Path)
|
||||
}
|
||||
}
|
||||
var out []string
|
||||
for _, m := range reported {
|
||||
seen := map[string]bool{}
|
||||
err := filepath.WalkDir(m.Dir, func(p string, d fs.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if d.IsDir() {
|
||||
switch d.Name() {
|
||||
case "testdata", "vendor", "node_modules", ".git":
|
||||
return fs.SkipDir
|
||||
}
|
||||
if p != m.Dir {
|
||||
if _, err := os.Stat(filepath.Join(p, "go.mod")); err == nil {
|
||||
return fs.SkipDir // a nested module is judged by its own go.mod
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if !strings.HasSuffix(p, ".go") {
|
||||
return nil
|
||||
}
|
||||
f, err := parser.ParseFile(token.NewFileSet(), p, nil, parser.ImportsOnly)
|
||||
if err != nil {
|
||||
return fmt.Errorf("%s: %w", p, err)
|
||||
}
|
||||
for _, im := range f.Imports {
|
||||
path, _ := strconv.Unquote(im.Path.Value)
|
||||
if !strings.HasPrefix(path, prefix) || seen[path] {
|
||||
continue
|
||||
}
|
||||
owner := ""
|
||||
for _, o := range owners {
|
||||
if within(path, o) && len(o) > len(owner) {
|
||||
owner = o
|
||||
}
|
||||
}
|
||||
ok := false
|
||||
if owner != "" {
|
||||
ok = owner == m.Path || m.Requires[owner]
|
||||
} else {
|
||||
ok = within(path, m.Path)
|
||||
for r := range m.Requires {
|
||||
ok = ok || within(path, r)
|
||||
}
|
||||
}
|
||||
if !ok {
|
||||
seen[path] = true
|
||||
if owner == "" {
|
||||
owner = "(its module)"
|
||||
}
|
||||
rel, _ := filepath.Rel(m.Dir, p)
|
||||
out = append(out, fmt.Sprintf("%s: %s imports %s (module %s), which its go.mod does not require", filepath.Join(m.Dir, "go.mod"), rel, path, owner))
|
||||
}
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
sort.Strings(out)
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func main() {
|
||||
args := os.Args[1:]
|
||||
all := len(args) > 0 && args[0] == "-all"
|
||||
if all {
|
||||
args = args[1:]
|
||||
}
|
||||
if len(args) < 1 {
|
||||
fmt.Fprintln(os.Stderr, "usage: requiredmodules [-all] <root> [<root> ...]")
|
||||
os.Exit(2)
|
||||
}
|
||||
report, known := args[:1], args[1:]
|
||||
if all {
|
||||
report, known = args, nil
|
||||
}
|
||||
found, err := Findings(report, known)
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, err)
|
||||
os.Exit(2)
|
||||
}
|
||||
for _, f := range found {
|
||||
fmt.Println(f)
|
||||
}
|
||||
if len(found) > 0 {
|
||||
os.Exit(1)
|
||||
}
|
||||
fmt.Println("every module requires the mesh modules it imports")
|
||||
}
|
||||
@@ -0,0 +1,78 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func tree(t *testing.T, files map[string]string) string {
|
||||
t.Helper()
|
||||
root := t.TempDir()
|
||||
for p, c := range files {
|
||||
full := filepath.Join(root, p)
|
||||
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(full, []byte(c), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
return root
|
||||
}
|
||||
|
||||
const aMod = "module git.novox.be/novox/r/a\n\ngo 1.22\n"
|
||||
const bMod = "module git.novox.be/novox/r/b/sub\n\ngo 1.22\n"
|
||||
|
||||
// A module that imports a package of a mesh module its go.mod does not require is refused, in a test file and behind a
|
||||
// build tag too; the module's own packages, a required module's and a nested module's (only when required) are not.
|
||||
func TestAModuleImportsOnlyWhatItRequires(t *testing.T) {
|
||||
src := func(imp string) string { return "package x\n\nimport _ \"" + imp + "\"\n" }
|
||||
for _, row := range []struct {
|
||||
name string
|
||||
files map[string]string
|
||||
want string
|
||||
}{
|
||||
{"its own package", map[string]string{"a/go.mod": aMod, "a/x.go": src("git.novox.be/novox/r/a/other")}, ""},
|
||||
{"a required module", map[string]string{"a/go.mod": aMod + "\nrequire git.novox.be/novox/r/b/sub v0.0.0\n", "a/x.go": src("git.novox.be/novox/r/b/sub/pkg"), "b/go.mod": bMod}, ""},
|
||||
{"a required module in a block", map[string]string{"a/go.mod": aMod + "\nrequire (\n\tgit.novox.be/novox/r/b/sub v0.0.0 // indirect\n)\n", "a/x.go": src("git.novox.be/novox/r/b/sub/pkg"), "b/go.mod": bMod}, ""},
|
||||
{"a module not required", map[string]string{"a/go.mod": aMod, "a/x.go": src("git.novox.be/novox/r/b/sub/pkg"), "b/go.mod": bMod}, "x.go imports git.novox.be/novox/r/b/sub/pkg"},
|
||||
{"a test file", map[string]string{"a/go.mod": aMod, "a/x_test.go": src("git.novox.be/novox/r/b/sub/pkg"), "b/go.mod": bMod}, "x_test.go imports"},
|
||||
{"an integration file", map[string]string{"a/go.mod": aMod, "a/x.go": "//go:build integration\n\n" + src("git.novox.be/novox/r/b/sub/pkg"), "b/go.mod": bMod}, "x.go imports"},
|
||||
{"a module the longest owner is not", map[string]string{"a/go.mod": aMod + "\nrequire git.novox.be/novox/r/b v0.0.0\n", "a/x.go": src("git.novox.be/novox/r/b/sub/pkg"), "b/go.mod": "module git.novox.be/novox/r/b\n", "b/sub/go.mod": bMod}, "does not require"},
|
||||
{"a testdata fixture", map[string]string{"a/go.mod": aMod, "a/testdata/x.go": src("git.novox.be/novox/r/b/sub/pkg"), "a/x.go": "package x\n"}, ""},
|
||||
{"an outside module", map[string]string{"a/go.mod": aMod, "a/x.go": src("github.com/x/y")}, ""},
|
||||
} {
|
||||
t.Run(row.name, func(t *testing.T) {
|
||||
got, err := Findings([]string{tree(t, row.files)}, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if all := strings.Join(got, "\n"); (row.want == "") != (all == "") || !strings.Contains(all, row.want) {
|
||||
t.Fatalf("findings = %q, want one containing %q", all, row.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The script's run of this check over the real tree (novox/hq ADR 0529 clause 2): the repositories cloned beside it, when
|
||||
// the check says where, tell which module owns a path.
|
||||
func TestEveryCatalogueModuleRequiresWhatItImports(t *testing.T) {
|
||||
var known []string
|
||||
if beside := os.Getenv("MESH_CHECK_BESIDE"); beside != "" {
|
||||
es, _ := os.ReadDir(beside)
|
||||
for _, e := range es {
|
||||
if e.IsDir() && e.Name() != "mesh-catalog" {
|
||||
known = append(known, filepath.Join(beside, e.Name()))
|
||||
}
|
||||
}
|
||||
}
|
||||
got, err := Findings([]string{filepath.Join("..", "..")}, known)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, f := range got {
|
||||
t.Error(f)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,6 @@
|
||||
# What these tests read outside their package, as paths from the repository's root (a glob, or a directory read whole;
|
||||
# ? before a path whose absence is read). mesh-build-go hashes them into the package's digest, so its verdict is kept
|
||||
# until one of them changes (issue 1936). An empty list, or a line matching nothing, keeps nothing.
|
||||
# every go.mod and Go source of the modules and of the checks, whose imports and requires are read.
|
||||
modules
|
||||
checks
|
||||
@@ -7,6 +7,7 @@ require (
|
||||
git.novox.be/novox/mesh-contracts/health-and-repair/go/clients/meshconditionsclient v0.0.0-00010101000000-000000000000
|
||||
git.novox.be/novox/mesh-delivery/contracts/go/clients/containerimageregistryclient v0.0.0-00010101000000-000000000000
|
||||
git.novox.be/novox/mesh-delivery/contracts/go/clients/meshcatalogclient v0.0.0-00010101000000-000000000000
|
||||
git.novox.be/novox/mesh-delivery/contracts/go/clients/meshdeliverycheckerclient v0.0.0-00010101000000-000000000000
|
||||
git.novox.be/novox/mesh-delivery/contracts/go/seats/containerimageregistry v0.0.0-00010101000000-000000000000
|
||||
git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog v0.0.0-00010101000000-000000000000
|
||||
git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker v0.0.0-00010101000000-000000000000
|
||||
|
||||
Reference in New Issue
Block a user