roots := rs.rootModules
rootsUpgraded := false
+ spotCheckRoot := map[module.Version]bool{}
+
// “The selected version of the module providing each package marked with
// either pkgInAll or pkgIsRoot is included as a root.”
needSort := false
// relevant to consumers of the main module either), and its dependencies
// should already be in the module graph — included in the dependencies of
// the package that imported it.
-
- if go117LazyTODO {
- // It is possible that one of the packages we just imported came from a
- // module with an incomplete or erroneous go.mod file — for example,
- // perhaps the author forgot to 'git add' their updated go.mod file
- // after adding a new package import. If that happens, ideally we want
- // to detect the missing requirements and fix them up here.
- //
- // However, we should ignore transitive dependencies of external tests:
- // the go.mod file for the module containing the test itself is expected
- // to provide all of the relevant dependencies, and we explicitly don't
- // want to pull in requirements on *irrelevant* requirements that happen
- // to occur in the go.mod files for these transitive-test-only
- // dependencies.
- }
-
continue
}
- if _, ok := rs.rootSelected(pkg.mod.Path); !ok {
+ if _, ok := rs.rootSelected(pkg.mod.Path); ok {
+ // It is possible that the main module's go.mod file is incomplete or
+ // otherwise erroneous — for example, perhaps the author forgot to 'git
+ // add' their updated go.mod file after adding a new package import, or
+ // perhaps they made an edit to the go.mod file using a third-party tool
+ // ('git merge'?) that doesn't maintain consistency for module
+ // dependencies. If that happens, ideally we want to detect the missing
+ // requirements and fix them up here.
+ //
+ // However, we also need to be careful not to be too aggressive. For
+ // transitive dependencies of external tests, the go.mod file for the
+ // module containing the test itself is expected to provide all of the
+ // relevant dependencies, and we explicitly don't want to pull in
+ // requirements on *irrelevant* requirements that happen to occur in the
+ // go.mod files for these transitive-test-only dependencies. (See the test
+ // in mod_lazy_test_horizon.txt for a concrete example.
+ //
+ // The “goldilocks zone” seems to be to spot-check exactly the same
+ // modules that we promote to explicit roots: namely, those that provide
+ // packages transitively imported by the main module, and those that
+ // provide roots of the package-import graph. That will catch erroneous
+ // edits to the main module's go.mod file and inconsistent requirements in
+ // dependencies that provide imported packages, but will ignore erroneous
+ // or misleading requirements in dependencies that aren't obviously
+ // relevant to the packages in the main module.
+ spotCheckRoot[pkg.mod] = true
+ } else {
roots = append(roots, pkg.mod)
rootsUpgraded = true
// The roots slice was initially sorted because rs.rootModules was sorted,
} else {
// Since none of the roots have been upgraded, we have no reason to
// suspect that they are inconsistent with the requirements of any other
- // roots. Only look at the full module graph if we've already loaded it.
- mg, _ = rs.graph.Load().(*ModuleGraph) // May be nil.
+ // roots. Only look at the full module graph if we've already loaded it;
+ // otherwise, just spot-check the explicit requirements of the roots from
+ // which we loaded packages.
+ if rs.graph.Load() != nil {
+ // We've already loaded the full module graph, which includes the
+ // requirements of all of the root modules — even the transitive
+ // requirements, if they are eager!
+ mg, _ = rs.Graph(ctx)
+ } else if cfg.BuildMod == "vendor" {
+ // We can't spot-check the requirements of other modules because we
+ // don't in general have their go.mod files available in the vendor
+ // directory. (Fortunately this case is impossible, because mg.graph is
+ // always non-nil in vendor mode!)
+ panic("internal error: rs.graph is unexpectedly nil with -mod=vendor")
+ } else if !spotCheckRoots(ctx, rs, spotCheckRoot) {
+ // We spot-checked the explicit requirements of the roots that are
+ // relevant to the packages we've loaded. Unfortunately, they're
+ // inconsistent in some way; we need to load the full module graph
+ // so that we can fix the roots properly.
+ var err error
+ mg, err = rs.Graph(ctx)
+ if err != nil {
+ return rs, err
+ }
+ }
}
roots = make([]module.Version, 0, len(rs.rootModules))
return newRequirements(lazy, roots, direct), nil
}
+// spotCheckRoots reports whether the versions of the roots in rs satisfy the
+// explicit requirements of the modules in mods.
+func spotCheckRoots(ctx context.Context, rs *Requirements, mods map[module.Version]bool) bool {
+ ctx, cancel := context.WithCancel(ctx)
+ defer cancel()
+
+ work := par.NewQueue(runtime.GOMAXPROCS(0))
+ for m := range mods {
+ m := m
+ work.Add(func() {
+ if ctx.Err() != nil {
+ return
+ }
+
+ summary, err := goModSummary(m)
+ if err != nil {
+ cancel()
+ return
+ }
+
+ for _, r := range summary.require {
+ if v, ok := rs.rootSelected(r.Path); ok && cmpVersion(v, r.Version) < 0 {
+ cancel()
+ return
+ }
+ }
+ })
+ }
+ <-work.Idle()
+
+ if ctx.Err() != nil {
+ // Either we failed a spot-check, or the caller no longer cares about our
+ // answer anyway.
+ return false
+ }
+
+ return true
+}
+
// tidyEagerRoots returns a minimal set of root requirements that maintains the
// selected version of every module that provided a package in pkgs, and
// includes the selected version of every such module in direct as a root.
--- /dev/null
+# If the root requirements in a lazy module are inconsistent
+# (for example, due to a bad hand-edit or git merge),
+# they can go unnoticed as long as the module with the violated
+# requirement is not used.
+# When we load a package from that module, we should spot-check its
+# requirements and either emit an error or update the go.mod file.
+
+cp go.mod go.mod.orig
+
+
+# If we load package x from x.1, we only check the requirements of x,
+# which are fine: loading succeeds.
+
+go list -deps ./usex
+stdout '^example.net/x$'
+cmp go.mod go.mod.orig
+
+
+# However, if we load needx2, we should load the requirements of needx2.
+# Those requirements indicate x.2, not x.1, so the module graph is
+# inconsistent and needs to be fixed.
+
+! go list -deps ./useneedx2
+stderr '^go: updates to go.mod needed; to update it:\n\tgo mod tidy$'
+
+! go list -deps example.net/needx2
+stderr '^go: updates to go.mod needed; to update it:\n\tgo mod tidy$'
+
+
+# The command printed in the error message should fix the problem.
+
+go mod tidy
+go list -deps ./useneedx2
+stdout '^example.net/m/useneedx2$'
+stdout '^example.net/needx2$'
+stdout '^example.net/x$'
+
+go list -m all
+stdout '^example.net/needx2 v0\.1\.0 '
+stdout '^example.net/x v0\.2\.0 '
+
+
+-- go.mod --
+module example.net/m
+
+go 1.17
+
+require (
+ example.net/needx2 v0.1.0
+ example.net/x v0.1.0
+)
+
+replace (
+ example.net/needx2 v0.1.0 => ./needx2.1
+ example.net/x v0.1.0 => ./x.1
+ example.net/x v0.2.0 => ./x.2
+)
+-- useneedx2/useneedx2.go --
+package useneedx2
+
+import _ "example.net/needx2"
+-- usex/usex.go --
+package usex
+
+import _ "example.net/x"
+
+-- x.1/go.mod --
+module example.com/x
+
+go 1.17
+-- x.1/x.go --
+package x
+
+-- x.2/go.mod --
+module example.com/x
+
+go 1.17
+-- x.2/x.go --
+package x
+
+const AddedInV2 = true
+
+-- needx2.1/go.mod --
+module example.com/x
+
+go 1.17
+
+require example.net/x v0.2.0
+-- needx2.1/needx2.go --
+// Package needx2 needs x v0.2.0 or higher.
+package needx2
+
+import "example.net/x"
+
+var _ = x.AddedInV2