diff --git a/checks/requiredmodules/go.mod b/checks/requiredmodules/go.mod new file mode 100644 index 000000000..2e7734205 --- /dev/null +++ b/checks/requiredmodules/go.mod @@ -0,0 +1,3 @@ +module git.novox.be/novox/mesh-catalog/checks/requiredmodules + +go 1.22 diff --git a/checks/requiredmodules/main.go b/checks/requiredmodules/main.go new file mode 100644 index 000000000..9fc63a36c --- /dev/null +++ b/checks/requiredmodules/main.go @@ -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 . [ ...] +// +// 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] [ ...]") + 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") +} diff --git a/checks/requiredmodules/main_test.go b/checks/requiredmodules/main_test.go new file mode 100644 index 000000000..3ea1ebbc1 --- /dev/null +++ b/checks/requiredmodules/main_test.go @@ -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) + } +} diff --git a/checks/requiredmodules/reads-outside b/checks/requiredmodules/reads-outside new file mode 100644 index 000000000..4300f90f9 --- /dev/null +++ b/checks/requiredmodules/reads-outside @@ -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 diff --git a/modules/mesh-build-go/go.mod b/modules/mesh-build-go/go.mod index c0cf6eb1c..13f373b82 100644 --- a/modules/mesh-build-go/go.mod +++ b/modules/mesh-build-go/go.mod @@ -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