Skip to content

Commit

Permalink
cmd/go/internal/modload: use "pruned" instead of "lazy" to describe p…
Browse files Browse the repository at this point in the history
…runed module graphs

The level of support for pruning — not the lazy/eager loading behavior
— is the more fundamental property, and what matters in terms of what
invariants we need to maintain.

If the main module supports pruned module graphs we load its
dependencies lazily, and if it does not support pruned module graphs
we load its dependencies eagerly. However, in principle we could also
load the module graph lazily even in modules that do not support graph
pruning — we would just be more likely to overlook inconsistent
requirements introduced by hand-edits or bad VCS merges to the go.mod
file.

(After this change, a “lazy” module is just one in which we happen not
to have loaded the module graph, and an “eager” one is one in which we
happen to load the module graph more aggressively.)

Updates #36460
For #47397

Change-Id: I0d2ffd21acc913f72ff56b59a6bdc539ebc3d377
Reviewed-on: https://go-review.googlesource.com/c/go/+/345393
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
  • Loading branch information
Bryan C. Mills committed Aug 30, 2021
1 parent af9009a commit 61120c6
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 230 deletions.
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modload/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func ModuleInfo(ctx context.Context, path string) *modinfo.ModulePublic {
v string
ok bool
)
if rs.depth == lazy {
if rs.pruning == pruned {
v, ok = rs.rootSelected(path)
}
if !ok {
Expand Down
217 changes: 112 additions & 105 deletions src/cmd/go/internal/modload/buildlist.go

Large diffs are not rendered by default.

114 changes: 58 additions & 56 deletions src/cmd/go/internal/modload/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
// 2. Each module version in tryUpgrade is upgraded toward the indicated
// version as far as can be done without violating (1).
//
// 3. Each module version in rs.rootModules (or rs.graph, if rs.depth is eager)
// 3. Each module version in rs.rootModules (or rs.graph, if rs is unpruned)
// is downgraded from its original version only to the extent needed to
// satisfy (1), or upgraded only to the extent needed to satisfy (1) and
// (2).
Expand Down Expand Up @@ -69,10 +69,11 @@ func editRequirements(ctx context.Context, rs *Requirements, tryUpgrade, mustSel
}

var roots []module.Version
if rs.depth == eager {
// In an eager module, modules that provide packages imported by the main
// module may either be explicit roots or implicit transitive dependencies.
// We promote the modules in mustSelect to be explicit requirements.
if rs.pruning == unpruned {
// In a module without graph pruning, modules that provide packages imported
// by the main module may either be explicit roots or implicit transitive
// dependencies. We promote the modules in mustSelect to be explicit
// requirements.
var rootPaths []string
for _, m := range mustSelect {
if !MainModules.Contains(m.Path) && m.Version != "none" {
Expand Down Expand Up @@ -102,8 +103,8 @@ func editRequirements(ctx context.Context, rs *Requirements, tryUpgrade, mustSel
return nil, false, err
}
} else {
// In a lazy module, every module that provides a package imported by the
// main module must be retained as a root.
// In a module with a pruned graph, every module that provides a package
// imported by the main module must be retained as a root.
roots = mods
if !changed {
// Because the roots we just computed are unchanged, the entire graph must
Expand All @@ -126,7 +127,7 @@ func editRequirements(ctx context.Context, rs *Requirements, tryUpgrade, mustSel
direct[m.Path] = true
}
}
return newRequirements(rs.depth, roots, direct), changed, nil
return newRequirements(rs.pruning, roots, direct), changed, nil
}

// limiterForEdit returns a versionLimiter with its max versions set such that
Expand All @@ -149,11 +150,12 @@ func limiterForEdit(ctx context.Context, rs *Requirements, tryUpgrade, mustSelec
}
}

if rs.depth == eager {
// Eager go.mod files don't indicate which transitive dependencies are
// actually relevant to the main module, so we have to assume that any module
// that could have provided any package — that is, any module whose selected
// version was not "none" — may be relevant.
if rs.pruning == unpruned {
// go.mod files that do not support graph pruning don't indicate which
// transitive dependencies are actually relevant to the main module, so we
// have to assume that any module that could have provided any package —
// that is, any module whose selected version was not "none" — may be
// relevant.
for _, m := range mg.BuildList() {
restrictTo(m)
}
Expand All @@ -175,7 +177,7 @@ func limiterForEdit(ctx context.Context, rs *Requirements, tryUpgrade, mustSelec
}
}

if err := raiseLimitsForUpgrades(ctx, maxVersion, rs.depth, tryUpgrade, mustSelect); err != nil {
if err := raiseLimitsForUpgrades(ctx, maxVersion, rs.pruning, tryUpgrade, mustSelect); err != nil {
return nil, err
}

Expand All @@ -185,7 +187,7 @@ func limiterForEdit(ctx context.Context, rs *Requirements, tryUpgrade, mustSelec
restrictTo(m)
}

return newVersionLimiter(rs.depth, maxVersion), nil
return newVersionLimiter(rs.pruning, maxVersion), nil
}

// raiseLimitsForUpgrades increases the module versions in maxVersions to the
Expand All @@ -195,12 +197,12 @@ func limiterForEdit(ctx context.Context, rs *Requirements, tryUpgrade, mustSelec
//
// Versions not present in maxVersion are unrestricted, and it is assumed that
// they will not be promoted to root requirements (and thus will not contribute
// their own dependencies if the main module is lazy).
// their own dependencies if the main module supports graph pruning).
//
// These limits provide an upper bound on how far a module may be upgraded as
// part of an incidental downgrade, if downgrades are needed in order to select
// the versions in mustSelect.
func raiseLimitsForUpgrades(ctx context.Context, maxVersion map[string]string, depth modDepth, tryUpgrade []module.Version, mustSelect []module.Version) error {
func raiseLimitsForUpgrades(ctx context.Context, maxVersion map[string]string, pruning modPruning, tryUpgrade []module.Version, mustSelect []module.Version) error {
// allow raises the limit for m.Path to at least m.Version.
// If m.Path was already unrestricted, it remains unrestricted.
allow := func(m module.Version) {
Expand All @@ -213,9 +215,9 @@ func raiseLimitsForUpgrades(ctx context.Context, maxVersion map[string]string, d
}
}

var eagerUpgrades []module.Version
if depth == eager {
eagerUpgrades = tryUpgrade
var unprunedUpgrades []module.Version
if pruning == unpruned {
unprunedUpgrades = tryUpgrade
} else {
for _, m := range tryUpgrade {
if MainModules.Contains(m.Path) {
Expand All @@ -229,11 +231,11 @@ func raiseLimitsForUpgrades(ctx context.Context, maxVersion map[string]string, d
if err != nil {
return err
}
if summary.depth == eager {
// For efficiency, we'll load all of the eager upgrades as one big
if summary.pruning == unpruned {
// For efficiency, we'll load all of the unpruned upgrades as one big
// graph, rather than loading the (potentially-overlapping) subgraph for
// each upgrade individually.
eagerUpgrades = append(eagerUpgrades, m)
unprunedUpgrades = append(unprunedUpgrades, m)
continue
}

Expand All @@ -244,14 +246,14 @@ func raiseLimitsForUpgrades(ctx context.Context, maxVersion map[string]string, d
}
}

if len(eagerUpgrades) > 0 {
// Compute the max versions for eager upgrades all together.
// Since these modules are eager, we'll end up scanning all of their
if len(unprunedUpgrades) > 0 {
// Compute the max versions for unpruned upgrades all together.
// Since these modules are unpruned, we'll end up scanning all of their
// transitive dependencies no matter which versions end up selected,
// and since we have a large dependency graph to scan we might get
// a significant benefit from not revisiting dependencies that are at
// common versions among multiple upgrades.
upgradeGraph, err := readModGraph(ctx, eager, eagerUpgrades)
upgradeGraph, err := readModGraph(ctx, unpruned, unprunedUpgrades)
if err != nil {
// Compute the requirement path from a module path in tryUpgrade to the
// error, and the requirement path (if any) from rs.rootModules to the
Expand All @@ -268,7 +270,7 @@ func raiseLimitsForUpgrades(ctx context.Context, maxVersion map[string]string, d
}

if len(mustSelect) > 0 {
mustGraph, err := readModGraph(ctx, depth, mustSelect)
mustGraph, err := readModGraph(ctx, pruning, mustSelect)
if err != nil {
return err
}
Expand Down Expand Up @@ -300,7 +302,7 @@ func selectPotentiallyImportedModules(ctx context.Context, limiter *versionLimit
}

var initial []module.Version
if rs.depth == eager {
if rs.pruning == unpruned {
mg, err := rs.Graph(ctx)
if err != nil {
return nil, false, err
Expand All @@ -327,7 +329,7 @@ func selectPotentiallyImportedModules(ctx context.Context, limiter *versionLimit
// downgraded module may require a higher (but still allowed) version of
// another. The lower version may require extraneous dependencies that aren't
// actually relevant, so we need to compute the actual selected versions.
mg, err := readModGraph(ctx, rs.depth, mods)
mg, err := readModGraph(ctx, rs.pruning, mods)
if err != nil {
return nil, false, err
}
Expand All @@ -349,16 +351,16 @@ func selectPotentiallyImportedModules(ctx context.Context, limiter *versionLimit
// A versionLimiter tracks the versions that may be selected for each module
// subject to constraints on the maximum versions of transitive dependencies.
type versionLimiter struct {
// depth is the depth at which the dependencies of the modules passed to
// pruning is the pruning at which the dependencies of the modules passed to
// Select and UpgradeToward are loaded.
depth modDepth
pruning modPruning

// max maps each module path to the maximum version that may be selected for
// that path.
//
// Paths with no entry are unrestricted, and we assume that they will not be
// promoted to root dependencies (so will not contribute dependencies if the
// main module is lazy).
// main module supports graph pruning).
max map[string]string

// selected maps each module path to a version of that path (if known) whose
Expand Down Expand Up @@ -410,16 +412,16 @@ func (dq dqState) isDisqualified() bool {
// in the map are unrestricted. The limiter assumes that unrestricted paths will
// not be promoted to root dependencies.
//
// If depth is lazy, then if a module passed to UpgradeToward or Select is
// itself lazy, its unrestricted dependencies are skipped when scanning
// requirements.
func newVersionLimiter(depth modDepth, max map[string]string) *versionLimiter {
// If module graph pruning is in effect, then if a module passed to
// UpgradeToward or Select supports pruning, its unrestricted dependencies are
// skipped when scanning requirements.
func newVersionLimiter(pruning modPruning, max map[string]string) *versionLimiter {
selected := make(map[string]string)
for _, m := range MainModules.Versions() {
selected[m.Path] = m.Version
}
return &versionLimiter{
depth: depth,
pruning: pruning,
max: max,
selected: selected,
dqReason: map[module.Version]dqState{},
Expand All @@ -430,8 +432,8 @@ func newVersionLimiter(depth modDepth, max map[string]string) *versionLimiter {
// UpgradeToward attempts to upgrade the selected version of m.Path as close as
// possible to m.Version without violating l's maximum version limits.
//
// If depth is lazy and m itself is lazy, the the dependencies of unrestricted
// dependencies of m will not be followed.
// If module graph pruning is in effect and m itself supports pruning, the
// dependencies of unrestricted dependencies of m will not be followed.
func (l *versionLimiter) UpgradeToward(ctx context.Context, m module.Version) error {
selected, ok := l.selected[m.Path]
if ok {
Expand All @@ -443,7 +445,7 @@ func (l *versionLimiter) UpgradeToward(ctx context.Context, m module.Version) er
selected = "none"
}

if l.check(m, l.depth).isDisqualified() {
if l.check(m, l.pruning).isDisqualified() {
candidates, err := versions(ctx, m.Path, CheckAllowed)
if err != nil {
// This is likely a transient error reaching the repository,
Expand All @@ -460,7 +462,7 @@ func (l *versionLimiter) UpgradeToward(ctx context.Context, m module.Version) er
})
candidates = candidates[:i]

for l.check(m, l.depth).isDisqualified() {
for l.check(m, l.pruning).isDisqualified() {
n := len(candidates)
if n == 0 || cmpVersion(selected, candidates[n-1]) >= 0 {
// We couldn't find a suitable candidate above the already-selected version.
Expand All @@ -477,7 +479,7 @@ func (l *versionLimiter) UpgradeToward(ctx context.Context, m module.Version) er

// Select attempts to set the selected version of m.Path to exactly m.Version.
func (l *versionLimiter) Select(m module.Version) (conflict module.Version, err error) {
dq := l.check(m, l.depth)
dq := l.check(m, l.pruning)
if !dq.isDisqualified() {
l.selected[m.Path] = m.Version
}
Expand All @@ -487,14 +489,14 @@ func (l *versionLimiter) Select(m module.Version) (conflict module.Version, err
// check determines whether m (or its transitive dependencies) would violate l's
// maximum version limits if added to the module requirement graph.
//
// If depth is lazy and m itself is lazy, then the dependencies of unrestricted
// dependencies of m will not be followed. If the lazy loading invariants hold
// for the main module up to this point, the packages in those modules are at
// best only imported by tests of dependencies that are themselves loaded from
// outside modules. Although we would like to keep 'go test all' as reproducible
// as is feasible, we don't want to retain test dependencies that are only
// marginally relevant at best.
func (l *versionLimiter) check(m module.Version, depth modDepth) dqState {
// If pruning is in effect and m itself supports graph pruning, the dependencies
// of unrestricted dependencies of m will not be followed. If the graph-pruning
// invariants hold for the main module up to this point, the packages in those
// modules are at best only imported by tests of dependencies that are
// themselves loaded from outside modules. Although we would like to keep
// 'go test all' as reproducible as is feasible, we don't want to retain test
// dependencies that are only marginally relevant at best.
func (l *versionLimiter) check(m module.Version, pruning modPruning) dqState {
if m.Version == "none" || m == MainModules.mustGetSingleMainModule() {
// version "none" has no requirements, and the dependencies of Target are
// tautological.
Expand Down Expand Up @@ -525,20 +527,20 @@ func (l *versionLimiter) check(m module.Version, depth modDepth) dqState {
return l.disqualify(m, dqState{err: err})
}

if summary.depth == eager {
depth = eager
if summary.pruning == unpruned {
pruning = unpruned
}
for _, r := range summary.require {
if depth == lazy {
if pruning == pruned {
if _, restricted := l.max[r.Path]; !restricted {
// r.Path is unrestricted, so we don't care at what version it is
// selected. We assume that r.Path will not become a root dependency, so
// since m is lazy, r's dependencies won't be followed.
// since m supports pruning, r's dependencies won't be followed.
continue
}
}

if dq := l.check(r, depth); dq.isDisqualified() {
if dq := l.check(r, pruning); dq.isDisqualified() {
return l.disqualify(m, dq)
}

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modload/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestQueryImport(t *testing.T) {
RootMode = NoRoot

ctx := context.Background()
rs := newRequirements(eager, nil, nil)
rs := newRequirements(unpruned, nil, nil)

for _, tt := range importTests {
t.Run(strings.ReplaceAll(tt.path, "/", "_"), func(t *testing.T) {
Expand Down
26 changes: 13 additions & 13 deletions src/cmd/go/internal/modload/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {
MainModules = makeMainModules([]module.Version{mainModule}, []string{""}, []*modfile.File{nil}, []*modFileIndex{nil}, "")
goVersion := LatestGoVersion()
rawGoVersion.Store(mainModule, goVersion)
requirements = newRequirements(modDepthFromGoVersion(goVersion), nil, nil)
requirements = newRequirements(pruningForGoVersion(goVersion), nil, nil)
return requirements, false
}

Expand Down Expand Up @@ -712,11 +712,11 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {

// We need to add a 'go' version to the go.mod file, but we must assume
// that its existing contents match something between Go 1.11 and 1.16.
// Go 1.11 through 1.16 have eager requirements, but the latest Go
// version uses lazy requirements instead — so we need to convert the
// requirements to be lazy.
// Go 1.11 through 1.16 do not support graph pruning, but the latest Go
// version uses a pruned module graph — so we need to convert the
// requirements to support pruning.
var err error
rs, err = convertDepth(ctx, rs, lazy)
rs, err = convertPruning(ctx, rs, pruned)
if err != nil {
base.Fatalf("go: %v", err)
}
Expand Down Expand Up @@ -978,7 +978,7 @@ func requirementsFromModFiles(ctx context.Context, modFiles []*modfile.File) *Re
}
}
module.Sort(roots)
rs := newRequirements(modDepthFromGoVersion(MainModules.GoVersion()), roots, direct)
rs := newRequirements(pruningForGoVersion(MainModules.GoVersion()), roots, direct)
return rs
}

Expand Down Expand Up @@ -1485,12 +1485,13 @@ func keepSums(ctx context.Context, ld *loader, rs *Requirements, which whichSums
continue
}

if rs.depth == lazy && pkg.mod.Path != "" {
if rs.pruning == pruned && pkg.mod.Path != "" {
if v, ok := rs.rootSelected(pkg.mod.Path); ok && v == pkg.mod.Version {
// pkg was loaded from a root module, and because the main module is
// lazy we do not check non-root modules for conflicts for packages
// that can be found in roots. So we only need the checksums for the
// root modules that may contain pkg, not all possible modules.
// pkg was loaded from a root module, and because the main module has
// a pruned module graph we do not check non-root modules for
// conflicts for packages that can be found in roots. So we only need
// the checksums for the root modules that may contain pkg, not all
// possible modules.
for prefix := pkg.path; prefix != "."; prefix = path.Dir(prefix) {
if v, ok := rs.rootSelected(prefix); ok && v != "none" {
m := module.Version{Path: prefix, Version: v}
Expand All @@ -1514,8 +1515,7 @@ func keepSums(ctx context.Context, ld *loader, rs *Requirements, which whichSums
}

if rs.graph.Load() == nil {
// The module graph was not loaded, possibly because the main module is lazy
// or possibly because we haven't needed to load the graph yet.
// We haven't needed to load the module graph so far.
// Save sums for the root modules (or their replacements), but don't
// incur the cost of loading the graph just to find and retain the sums.
for _, m := range rs.rootModules {
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/go/internal/modload/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func listModules(ctx context.Context, rs *Requirements, args []string, mode List
path := arg[:i]
vers := arg[i+1:]
if vers == "upgrade" || vers == "patch" {
if _, ok := rs.rootSelected(path); !ok || rs.depth == eager {
if _, ok := rs.rootSelected(path); !ok || rs.pruning == unpruned {
needFullGraph = true
if !HasModRoot() {
base.Fatalf("go: cannot match %q: %v", arg, ErrNoModRoot)
Expand All @@ -114,7 +114,7 @@ func listModules(ctx context.Context, rs *Requirements, args []string, mode List
}
continue
}
if _, ok := rs.rootSelected(arg); !ok || rs.depth == eager {
if _, ok := rs.rootSelected(arg); !ok || rs.pruning == unpruned {
needFullGraph = true
if mode&ListVersions == 0 && !HasModRoot() {
base.Fatalf("go: cannot match %q without -versions or an explicit version: %v", arg, ErrNoModRoot)
Expand Down
Loading

0 comments on commit 61120c6

Please sign in to comment.