diff --git a/src/cmd/internal/moddeps/moddeps_test.go b/src/cmd/internal/moddeps/moddeps_test.go index 9ea21873c5ed9..cba401c896aad 100644 --- a/src/cmd/internal/moddeps/moddeps_test.go +++ b/src/cmd/internal/moddeps/moddeps_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "internal/testenv" + "io" "io/fs" "io/ioutil" "os" @@ -21,93 +22,34 @@ import ( "golang.org/x/mod/module" ) -type gorootModule struct { - Path string - Dir string - hasVendor bool -} - -// findGorootModules returns the list of modules found in the GOROOT source tree. -func findGorootModules(t *testing.T) []gorootModule { - t.Helper() - goBin := testenv.GoToolPath(t) - - goroot.once.Do(func() { - goroot.err = filepath.WalkDir(runtime.GOROOT(), func(path string, info fs.DirEntry, err error) error { - if err != nil { - return err - } - if info.IsDir() && (info.Name() == "vendor" || info.Name() == "testdata") { - return filepath.SkipDir - } - if path == filepath.Join(runtime.GOROOT(), "pkg") { - // GOROOT/pkg contains generated artifacts, not source code. - // - // In https://golang.org/issue/37929 it was observed to somehow contain - // a module cache, so it is important to skip. (That helps with the - // running time of this test anyway.) - return filepath.SkipDir - } - if info.IsDir() || info.Name() != "go.mod" { - return nil - } - dir := filepath.Dir(path) - - // Use 'go list' to describe the module contained in this directory (but - // not its dependencies). - cmd := exec.Command(goBin, "list", "-json", "-m") - cmd.Env = append(os.Environ(), "GO111MODULE=on") - cmd.Dir = dir - cmd.Stderr = new(strings.Builder) - out, err := cmd.Output() - if err != nil { - return fmt.Errorf("'go list -json -m' in %s: %w\n%s", dir, err, cmd.Stderr) - } - - var m gorootModule - if err := json.Unmarshal(out, &m); err != nil { - return fmt.Errorf("decoding 'go list -json -m' in %s: %w", dir, err) - } - if m.Path == "" || m.Dir == "" { - return fmt.Errorf("'go list -json -m' in %s failed to populate Path and/or Dir", dir) - } - if _, err := os.Stat(filepath.Join(dir, "vendor")); err == nil { - m.hasVendor = true - } - goroot.modules = append(goroot.modules, m) - return nil - }) - }) - - if goroot.err != nil { - t.Fatal(goroot.err) - } - return goroot.modules -} - -// goroot caches the list of modules found in the GOROOT source tree. -var goroot struct { - once sync.Once - modules []gorootModule - err error -} - -// TestAllDependenciesVendored ensures that all packages imported within GOROOT -// are vendored in the corresponding GOROOT module. +// TestAllDependencies ensures dependencies of all +// modules in GOROOT are in a consistent state. // -// This property allows offline development within the Go project, and ensures -// that all dependency changes are presented in the usual code review process. +// In short mode, it does a limited quick check and stops there. +// In long mode, it also makes a copy of the entire GOROOT tree +// and requires network access to perform more thorough checks. +// Keep this distinction in mind when adding new checks. // -// This test does NOT ensure that the vendored contents match the unmodified -// contents of the corresponding dependency versions. Such as test would require -// network access, and would currently either need to copy the entire GOROOT module -// or explicitly invoke version control to check for changes. -// (See golang.org/issue/36852 and golang.org/issue/27348.) -func TestAllDependenciesVendored(t *testing.T) { +// See issues 36852, 41409, and 43687. +// (Also see golang.org/issue/27348.) +func TestAllDependencies(t *testing.T) { goBin := testenv.GoToolPath(t) + // Ensure that all packages imported within GOROOT + // are vendored in the corresponding GOROOT module. + // + // This property allows offline development within the Go project, and ensures + // that all dependency changes are presented in the usual code review process. + // + // As a quick first-order check, avoid network access and the need to copy the + // entire GOROOT tree or explicitly invoke version control to check for changes. + // Just check that packages are vendored. (In non-short mode, we go on to also + // copy the GOROOT tree and perform more rigorous consistency checks. Jump below + // for more details.) for _, m := range findGorootModules(t) { - t.Run(m.Path, func(t *testing.T) { + // This short test does NOT ensure that the vendored contents match + // the unmodified contents of the corresponding dependency versions. + t.Run(m.Path+"(quick)", func(t *testing.T) { if m.hasVendor { // Load all of the packages in the module to ensure that their // dependencies are vendored. If any imported package is missing, @@ -140,6 +82,226 @@ func TestAllDependenciesVendored(t *testing.T) { } }) } + + // We now get to the slow, but more thorough part of the test. + // Only run it in long test mode. + if testing.Short() { + return + } + + // Ensure that all modules within GOROOT are tidy, vendored, and bundled. + // Ensure that the vendored contents match the unmodified contents of the + // corresponding dependency versions. + // + // The non-short section of this test requires network access and the diff + // command. + // + // It makes a temporary copy of the entire GOROOT tree (where it can safely + // perform operations that may mutate the tree), executes the same module + // maintenance commands that we expect Go developers to run, and then + // diffs the potentially modified module copy with the real one in GOROOT. + // (We could try to rely on Git to do things differently, but that's not the + // path we've chosen at this time. This allows the test to run when the tree + // is not checked into Git.) + + testenv.MustHaveExternalNetwork(t) + if haveDiff := func() bool { + diff, err := exec.Command("diff", "--recursive", "--unified", ".", ".").CombinedOutput() + if err != nil || len(diff) != 0 { + return false + } + diff, err = exec.Command("diff", "--recursive", "--unified", ".", "..").CombinedOutput() + if err == nil || len(diff) == 0 { + return false + } + return true + }(); !haveDiff { + // For now, the diff command is a mandatory dependency of this test. + // This test will primarily run on longtest builders, since few people + // would test the cmd/internal/moddeps package directly, and all.bash + // runs tests in short mode. It's fine to skip if diff is unavailable. + t.Skip("skipping because a diff command with support for --recursive and --unified flags is unavailable") + } + + // Build the bundle binary at the golang.org/x/tools + // module version specified in GOROOT/src/cmd/go.mod. + bundleDir := t.TempDir() + r := runner{Dir: filepath.Join(runtime.GOROOT(), "src/cmd")} + r.run(t, goBin, "build", "-mod=readonly", "-o", bundleDir, "golang.org/x/tools/cmd/bundle") + + var gorootCopyDir string + for _, m := range findGorootModules(t) { + // Create a test-wide GOROOT copy. It can be created once + // and reused between subtests whenever they don't fail. + // + // This is a relatively expensive operation, but it's a pre-requisite to + // be able to safely run commands like "go mod tidy", "go mod vendor", and + // "go generate" on the GOROOT tree content. Those commands may modify the + // tree, and we don't want to happen to the real tree as part of executing + // a test. + if gorootCopyDir == "" { + gorootCopyDir = makeGOROOTCopy(t) + } + + t.Run(m.Path+"(thorough)", func(t *testing.T) { + defer func() { + if t.Failed() { + // The test failed, which means it's possible the GOROOT copy + // may have been modified. No choice but to reset it for next + // module test case. (This is slow, but it happens only during + // test failures.) + gorootCopyDir = "" + } + }() + + rel, err := filepath.Rel(runtime.GOROOT(), m.Dir) + if err != nil { + t.Fatalf("filepath.Rel(%q, %q): %v", runtime.GOROOT(), m.Dir, err) + } + r := runner{ + Dir: filepath.Join(gorootCopyDir, rel), + Env: append(os.Environ(), + // Set GOROOT. + "GOROOT="+gorootCopyDir, + // Explicitly clear PWD and GOROOT_FINAL so that GOROOT=gorootCopyDir is definitely used. + "PWD=", + "GOROOT_FINAL=", + // Add GOROOTcopy/bin and bundleDir to front of PATH. + "PATH="+filepath.Join(gorootCopyDir, "bin")+string(filepath.ListSeparator)+ + bundleDir+string(filepath.ListSeparator)+os.Getenv("PATH"), + ), + } + goBinCopy := filepath.Join(gorootCopyDir, "bin", "go") + r.run(t, goBinCopy, "mod", "tidy") // See issue 43687. + r.run(t, goBinCopy, "mod", "verify") // Verify should be a no-op, but test it just in case. + r.run(t, goBinCopy, "mod", "vendor") // See issue 36852. + pkgs := packagePattern(m.Path) + r.run(t, goBinCopy, "generate", `-run=^//go:generate bundle `, pkgs) // See issue 41409. + advice := "$ cd " + m.Dir + "\n" + + "$ go mod tidy # to remove extraneous dependencies\n" + + "$ go mod vendor # to vendor dependecies\n" + + "$ go generate -run=bundle " + pkgs + " # to regenerate bundled packages\n" + if m.Path == "std" { + r.run(t, goBinCopy, "generate", "syscall", "internal/syscall/...") // See issue 43440. + advice += "$ go generate syscall internal/syscall/... # to regenerate syscall packages\n" + } + // TODO(golang.org/issue/43440): Check anything else influenced by dependency versions. + + diff, err := exec.Command("diff", "--recursive", "--unified", r.Dir, m.Dir).CombinedOutput() + if err != nil || len(diff) != 0 { + t.Errorf(`Module %s in %s is not tidy (-want +got): + +%s +To fix it, run: + +%s +(If module %[1]s is definitely tidy, this could mean +there's a problem in the go or bundle command.)`, m.Path, m.Dir, diff, advice) + } + }) + } +} + +// packagePattern returns a package pattern that matches all packages +// in the module modulePath, and ideally as few others as possible. +func packagePattern(modulePath string) string { + if modulePath == "std" { + return "std" + } + return modulePath + "/..." +} + +// makeGOROOTCopy makes a temporary copy of the current GOROOT tree. +// The goal is to allow the calling test t to safely mutate a GOROOT +// copy without also modifying the original GOROOT. +// +// It copies the entire tree as is, with the exception of the GOROOT/.git +// directory, which is skipped, and the GOROOT/{bin,pkg} directories, +// which are symlinked. This is done for speed, since a GOROOT tree is +// functional without being in a Git repository, and bin and pkg are +// deemed safe to share for the purpose of the TestAllDependencies test. +func makeGOROOTCopy(t *testing.T) string { + t.Helper() + gorootCopyDir := t.TempDir() + err := filepath.Walk(runtime.GOROOT(), func(src string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if src == filepath.Join(runtime.GOROOT(), ".git") { + return filepath.SkipDir + } + + rel, err := filepath.Rel(runtime.GOROOT(), src) + if err != nil { + return fmt.Errorf("filepath.Rel(%q, %q): %v", runtime.GOROOT(), src, err) + } + dst := filepath.Join(gorootCopyDir, rel) + + switch src { + case filepath.Join(runtime.GOROOT(), "bin"), + filepath.Join(runtime.GOROOT(), "pkg"): + // If the OS supports symlinks, use them instead + // of copying the bin and pkg directories. + if err := os.Symlink(src, dst); err == nil { + return filepath.SkipDir + } + } + + perm := info.Mode() & os.ModePerm + if info.Mode()&os.ModeSymlink != 0 { + info, err = os.Stat(src) + if err != nil { + return err + } + perm = info.Mode() & os.ModePerm + } + + // If it's a directory, make a corresponding directory. + if info.IsDir() { + return os.MkdirAll(dst, perm|0200) + } + + // Copy the file bytes. + // We can't create a symlink because the file may get modified; + // we need to ensure that only the temporary copy is affected. + s, err := os.Open(src) + if err != nil { + return err + } + defer s.Close() + d, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm) + if err != nil { + return err + } + _, err = io.Copy(d, s) + if err != nil { + d.Close() + return err + } + return d.Close() + }) + if err != nil { + t.Fatal(err) + } + return gorootCopyDir +} + +type runner struct { + Dir string + Env []string +} + +// run runs the command and requires that it succeeds. +func (r runner) run(t *testing.T, args ...string) { + t.Helper() + cmd := exec.Command(args[0], args[1:]...) + cmd.Dir = r.Dir + cmd.Env = r.Env + out, err := cmd.CombinedOutput() + if err != nil { + t.Logf("> %s\n", strings.Join(args, " ")) + t.Fatalf("command failed: %s\n%s", err, out) + } } // TestDependencyVersionsConsistent verifies that each module in GOROOT that @@ -159,8 +321,7 @@ func TestDependencyVersionsConsistent(t *testing.T) { seen := map[string]map[requirement][]gorootModule{} // module path → requirement → set of modules with that requirement for _, m := range findGorootModules(t) { if !m.hasVendor { - // TestAllDependenciesVendored will ensure that the module has no - // dependencies. + // TestAllDependencies will ensure that the module has no dependencies. continue } @@ -233,3 +394,74 @@ func TestDependencyVersionsConsistent(t *testing.T) { } } } + +type gorootModule struct { + Path string + Dir string + hasVendor bool +} + +// findGorootModules returns the list of modules found in the GOROOT source tree. +func findGorootModules(t *testing.T) []gorootModule { + t.Helper() + goBin := testenv.GoToolPath(t) + + goroot.once.Do(func() { + goroot.err = filepath.WalkDir(runtime.GOROOT(), func(path string, info fs.DirEntry, err error) error { + if err != nil { + return err + } + if info.IsDir() && (info.Name() == "vendor" || info.Name() == "testdata") { + return filepath.SkipDir + } + if path == filepath.Join(runtime.GOROOT(), "pkg") { + // GOROOT/pkg contains generated artifacts, not source code. + // + // In https://golang.org/issue/37929 it was observed to somehow contain + // a module cache, so it is important to skip. (That helps with the + // running time of this test anyway.) + return filepath.SkipDir + } + if info.IsDir() || info.Name() != "go.mod" { + return nil + } + dir := filepath.Dir(path) + + // Use 'go list' to describe the module contained in this directory (but + // not its dependencies). + cmd := exec.Command(goBin, "list", "-json", "-m") + cmd.Env = append(os.Environ(), "GO111MODULE=on") + cmd.Dir = dir + cmd.Stderr = new(strings.Builder) + out, err := cmd.Output() + if err != nil { + return fmt.Errorf("'go list -json -m' in %s: %w\n%s", dir, err, cmd.Stderr) + } + + var m gorootModule + if err := json.Unmarshal(out, &m); err != nil { + return fmt.Errorf("decoding 'go list -json -m' in %s: %w", dir, err) + } + if m.Path == "" || m.Dir == "" { + return fmt.Errorf("'go list -json -m' in %s failed to populate Path and/or Dir", dir) + } + if _, err := os.Stat(filepath.Join(dir, "vendor")); err == nil { + m.hasVendor = true + } + goroot.modules = append(goroot.modules, m) + return nil + }) + }) + + if goroot.err != nil { + t.Fatal(goroot.err) + } + return goroot.modules +} + +// goroot caches the list of modules found in the GOROOT source tree. +var goroot struct { + once sync.Once + modules []gorootModule + err error +}