cmd/internal/moddeps: check content of all modules in GOROOT
Expand the scope of the TestAllDependenciesVendored test to check
that all modules in GOROOT are tidy, that packages are vendored,
the vendor content matches the upstream copy exactly, and that
bundled packages are re-generated (using x/tools/cmd/bundle at
the version selected in cmd module; this is deterministic and
guaranteed to be updated over time).
This is done in a conceptually simple way:
1. Make a temporary copy of the entire GOROOT tree (except .git),
one that is safe to modify.
2. Run a list of high-level commands, the same commands we expect
Go developers should be able to run in a normal complete GOROOT
tree to make it clean and tidy.
3. Diff the end result with the original GOROOT tree being tested
to catch any unexpected differences.
The current set of commands that are run require the cmd/go command,
and a functional compiler itself (because re-generating the syscall
package involves a directive like //go:generate go run [...]). As a
result, copying a large majority of the GOROOT tree is a requirement.
Instead of looking for the few files or directories that can we can
get away not copying (e.g., the testdata directories aren't strictly
needed at this time), we opt not to optimize and just do the simple
copy. This is motivated by these reasons:
• We end up having a complete, normal GOROOT tree, one that happens
to be located at another path. There's a very high likelihood that
module management/code generation commands, both the ones we run
today and any additional ones that we might want to add in the
future, will result in correct results even as the Go project
evolves over time.
• Having a completely stand-alone copy of the GOROOT tree without
symlinks minimizes the risk of some of the module management/code
generation commands, either now or in the future, from modifying
the user's original GOROOT tree, something that should not happen
during test execution. Overlays achieved with symlinks work well
when we can guarantee only new files are added, but that isn't
the case here.
• Copying the entire GOROOT (without .git), takes around 5 seconds
on a fairly modern computer with an SSD. The most we can save is
a couple of seconds.
(We make some minor exceptions: the GOROOT/.git directory isn't copied,
and GOROOT/{bin,pkg} are deemed safe to share and thus symlink instead
of copying. If these optimizations cease to be viable to make, we'll
need to remove them.)
Since this functionality is fairly expensive to execute and requires
network access, it runs only when the test is executed without -short
flag. The previous behavior of the TestAllDependenciesVendored test is
kept in -short test mode. all.bash runs package tests with -short flag,
so its behavior is unchanged. The expectation is that the new test will
run on some of the longtest builders to catch problems. Users can invoke
the test manually 'go test cmd/internal/moddeps' (and it's run as part
of 'go test cmd', again, only when -short flag isn't provided).
On a 2017 MacBook Pro, a successful long test takes under 15 seconds,
which should be within scope of all long tests that are selected by
'go test std cmd'. We may further adjust when and where the test runs
by default based on our experience.
Fixes #36852.
Fixes #41409.
Fixes #43687.
Updates #43440.
Change-Id: I9eb85205fec7ec62e3f867831a0a82e3c767f618
Reviewed-on: https://go-review.googlesource.com/c/go/+/283643
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
diff --git a/src/cmd/internal/moddeps/moddeps_test.go b/src/cmd/internal/moddeps/moddeps_test.go
index 9ea2187..cba401c 100644
--- a/src/cmd/internal/moddeps/moddeps_test.go
+++ b/src/cmd/internal/moddeps/moddeps_test.go
@@ -8,6 +8,7 @@
"encoding/json"
"fmt"
"internal/testenv"
+ "io"
"io/fs"
"io/ioutil"
"os"
@@ -21,93 +22,34 @@
"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()
+// TestAllDependencies ensures dependencies of all
+// modules in GOROOT are in a consistent state.
+//
+// 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.
+//
+// See issues 36852, 41409, and 43687.
+// (Also see golang.org/issue/27348.)
+func TestAllDependencies(t *testing.T) {
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.
-//
-// This property allows offline development within the Go project, and ensures
-// that all dependency changes are presented in the usual code review process.
-//
-// 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) {
- 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 @@
}
})
}
+
+ // 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 @@
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 @@
}
}
}
+
+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
+}
