aboutsummaryrefslogtreecommitdiff
path: root/src/cmd/go/internal/modload/edit.go
diff options
context:
space:
mode:
authorBryan C. Mills <bcmills@google.com>2021-03-25 23:26:56 -0400
committerBryan C. Mills <bcmills@google.com>2021-04-22 18:07:38 +0000
commit1a5665533be3641511b72dac5b91f1c7500e40b5 (patch)
treef00ef44974b3b32ce0dccd70f8c8cf44e0d17aa3 /src/cmd/go/internal/modload/edit.go
parent9c1b769d5fdd419bbaf416bc51981f0ba2af0831 (diff)
downloadgo-1a5665533be3641511b72dac5b91f1c7500e40b5.tar.gz
go-1a5665533be3641511b72dac5b91f1c7500e40b5.zip
cmd/go/internal/modload: migrate editBuildList to use a structured requirement graph
For #36460 Change-Id: Ic87d7e25402bb938d2872d33d26c4bf397776d1b Reviewed-on: https://go-review.googlesource.com/c/go/+/308517 Trust: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Diffstat (limited to 'src/cmd/go/internal/modload/edit.go')
-rw-r--r--src/cmd/go/internal/modload/edit.go446
1 files changed, 355 insertions, 91 deletions
diff --git a/src/cmd/go/internal/modload/edit.go b/src/cmd/go/internal/modload/edit.go
index 858fec5dd5..2921b38157 100644
--- a/src/cmd/go/internal/modload/edit.go
+++ b/src/cmd/go/internal/modload/edit.go
@@ -5,100 +5,321 @@
package modload
import (
+ "cmd/go/internal/mvs"
"context"
+ "reflect"
"sort"
- "cmd/go/internal/mvs"
-
"golang.org/x/mod/module"
"golang.org/x/mod/semver"
)
-// editBuildList returns an edited version of initial such that:
+// editRequirements returns an edited version of rs such that:
//
// 1. Each module version in mustSelect is selected.
//
// 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 initial 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).
+// 3. Each module version in rs.rootModules (or rs.graph, if rs.depth is eager)
+// 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).
//
// 4. No module is upgraded above the maximum version of its path found in the
-// combined dependency graph of list, tryUpgrade, and mustSelect.
-func editBuildList(ctx context.Context, initial, tryUpgrade, mustSelect []module.Version) ([]module.Version, error) {
- // Per https://research.swtch.com/vgo-mvs#algorithm_4:
- // “To avoid an unnecessary downgrade to E 1.1, we must also add a new
- // requirement on E 1.2. We can apply Algorithm R to find the minimal set of
- // new requirements to write to go.mod.”
- //
- // In order to generate those new requirements, we need consider versions for
- // every module in the existing build list, plus every module being directly
- // added by the edit. However, modules added only as dependencies of tentative
- // versions should not be retained if they end up being upgraded or downgraded
- // away due to versions in mustSelect.
-
- // When we downgrade modules in order to reach mustSelect, we don't want to
- // upgrade any existing module above the version that would be selected if we
- // just added all of the new requirements and *didn't* downgrade.
- //
- // So we'll do exactly that: just add all of the new requirements and not
- // downgrade, and return the resulting versions as an upper bound. This
- // intentionally limits our solution space so that edits that the user
- // percieves as “downgrades” will not also result in upgrades.
- max := make(map[string]string)
- maxes, err := mvs.Upgrade(Target, &mvsReqs{
- roots: append(capVersionSlice(initial[1:]), mustSelect...),
- }, tryUpgrade...)
+// dependency graph of rs, the combined dependency graph of the versions in
+// mustSelect, or the dependencies of each individual module version in
+// tryUpgrade.
+//
+// Generally, the module versions in mustSelect are due to the module or a
+// package within the module matching an explicit command line argument to 'go
+// get', and the versions in tryUpgrade are transitive dependencies that are
+// either being upgraded by 'go get -u' or being added to satisfy some
+// otherwise-missing package import.
+func editRequirements(ctx context.Context, rs *Requirements, tryUpgrade, mustSelect []module.Version) (edited *Requirements, changed bool, err error) {
+ limiter, err := limiterForEdit(ctx, rs, tryUpgrade, mustSelect)
if err != nil {
- return nil, err
- }
- for _, m := range maxes {
- max[m.Path] = m.Version
- }
- // The versions in mustSelect override whatever we would naively select —
- // we will downgrade other modules as needed in order to meet them.
- for _, m := range mustSelect {
- max[m.Path] = m.Version
+ return rs, false, err
}
- limiter := newVersionLimiter(max)
-
var conflicts []Conflict
for _, m := range mustSelect {
- dq := limiter.check(m)
- switch {
- case dq.err != nil:
- return nil, err
- case dq.conflict != module.Version{}:
+ conflict, err := limiter.Select(m)
+ if err != nil {
+ return rs, false, err
+ }
+ if conflict.Path != "" {
conflicts = append(conflicts, Conflict{
Source: m,
- Dep: dq.conflict,
+ Dep: conflict,
Constraint: module.Version{
- Path: dq.conflict.Path,
- Version: limiter.max[dq.conflict.Path],
+ Path: conflict.Path,
+ Version: limiter.max[conflict.Path],
},
})
}
- limiter.selected[m.Path] = m.Version
}
if len(conflicts) > 0 {
- return nil, &ConstraintError{Conflicts: conflicts}
+ return rs, false, &ConstraintError{Conflicts: conflicts}
+ }
+
+ mods, changed, err := selectPotentiallyImportedModules(ctx, limiter, rs, tryUpgrade)
+ if err != nil {
+ return rs, false, err
+ }
+
+ 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.
+ var rootPaths []string
+ for _, m := range mustSelect {
+ if m.Version != "none" && m.Path != Target.Path {
+ rootPaths = append(rootPaths, m.Path)
+ }
+ }
+ if !changed && len(rootPaths) == 0 {
+ // The build list hasn't changed and we have no new roots to add.
+ // We don't need to recompute the minimal roots for the module.
+ return rs, false, nil
+ }
+
+ for _, m := range mods {
+ if v, ok := rs.rootSelected(m.Path); ok && (v == m.Version || rs.direct[m.Path]) {
+ // m.Path was formerly a root, and either its version hasn't changed or
+ // we believe that it provides a package directly imported by a package
+ // or test in the main module. For now we'll assume that it is still
+ // relevant enough to remain a root. If we actually load all of the
+ // packages and tests in the main module (which we are not doing here),
+ // we can revise the explicit roots at that point.
+ rootPaths = append(rootPaths, m.Path)
+ }
+ }
+
+ roots, err = mvs.Req(Target, rootPaths, &mvsReqs{roots: mods})
+ if err != nil {
+ 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.
+ roots = mods
+ if !changed {
+ // Because the roots we just computed are unchanged, the entire graph must
+ // be the same as it was before. Save the original rs, since we have
+ // probably already loaded its requirement graph.
+ return rs, false, nil
+ }
+ }
+
+ // A module that is not even in the build list necessarily cannot provide
+ // any imported packages. Mark as direct only the direct modules that are
+ // still in the build list.
+ //
+ // TODO(bcmills): Would it make more sense to leave the direct map as-is
+ // but allow it to refer to modules that are no longer in the build list?
+ // That might complicate updateRoots, but it may be cleaner in other ways.
+ direct := make(map[string]bool, len(rs.direct))
+ for _, m := range roots {
+ if rs.direct[m.Path] {
+ direct[m.Path] = true
+ }
+ }
+ return newRequirements(rs.depth, roots, direct), changed, nil
+}
+
+// limiterForEdit returns a versionLimiter with its max versions set such that
+// the max version for every module path in mustSelect is the version listed
+// there, and the max version for every other module path is the maximum version
+// of its path found in the dependency graph of rs, the combined dependency
+// graph of the versions in mustSelect, or the dependencies of each individual
+// module version in tryUpgrade.
+func limiterForEdit(ctx context.Context, rs *Requirements, tryUpgrade, mustSelect []module.Version) (*versionLimiter, error) {
+ mg, err := rs.Graph(ctx)
+ if err != nil {
+ return nil, err
+ }
+
+ maxVersion := map[string]string{} // module path → version
+ restrictTo := func(m module.Version) {
+ v, ok := maxVersion[m.Path]
+ if !ok || cmpVersion(v, m.Version) > 0 {
+ maxVersion[m.Path] = m.Version
+ }
+ }
+
+ 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.
+ for _, m := range mg.BuildList() {
+ restrictTo(m)
+ }
+ } else {
+ // The go.mod file explicitly records every module that provides a package
+ // imported by the main module.
+ //
+ // If we need to downgrade an existing root or a new root found in
+ // tryUpgrade, we don't want to allow that downgrade to incidentally upgrade
+ // a module imported by the main module to some arbitrary version.
+ // However, we don't particularly care about arbitrary upgrades to modules
+ // that are (at best) only providing packages imported by tests of
+ // dependencies outside the main module.
+ for _, m := range rs.rootModules {
+ restrictTo(module.Version{
+ Path: m.Path,
+ Version: mg.Selected(m.Path),
+ })
+ }
+ }
+
+ if err := raiseLimitsForUpgrades(ctx, maxVersion, rs.depth, tryUpgrade, mustSelect); err != nil {
+ return nil, err
+ }
+
+ // The versions in mustSelect override whatever we would naively select —
+ // we will downgrade other modules as needed in order to meet them.
+ for _, m := range mustSelect {
+ restrictTo(m)
+ }
+
+ return newVersionLimiter(rs.depth, maxVersion), nil
+}
+
+// raiseLimitsForUpgrades increases the module versions in maxVersions to the
+// versions that would be needed to allow each of the modules in tryUpgrade
+// (individually) and all of the modules in mustSelect (simultaneously) to be
+// added as roots.
+//
+// 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).
+//
+// 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 {
+ // 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) {
+ v, ok := maxVersion[m.Path]
+ if !ok {
+ return // m.Path is unrestricted.
+ }
+ if cmpVersion(v, m.Version) < 0 {
+ maxVersion[m.Path] = m.Version
+ }
+ }
+
+ var eagerUpgrades []module.Version
+ if depth == eager {
+ eagerUpgrades = tryUpgrade
+ } else {
+ for _, m := range tryUpgrade {
+ if m.Path == Target.Path {
+ // Target is already considered to be higher than any possible m, so we
+ // won't be upgrading to it anyway and there is no point scanning its
+ // dependencies.
+ continue
+ }
+
+ summary, err := goModSummary(m)
+ if err != nil {
+ return err
+ }
+ if summary.depth() == eager {
+ // For efficiency, we'll load all of the eager upgrades as one big
+ // graph, rather than loading the (potentially-overlapping) subgraph for
+ // each upgrade individually.
+ eagerUpgrades = append(eagerUpgrades, m)
+ continue
+ }
+
+ for _, r := range summary.require {
+ allow(r)
+ }
+ }
}
- // For each module, we want to get as close as we can to either the upgrade
- // version or the previously-selected version in the build list, whichever is
- // higher. We can compute those in either order, but the upgrades will tend to
- // be higher than the build list, so we arbitrarily start with those.
+ 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
+ // 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)
+ if err != nil {
+ if go117LazyTODO {
+ // Compute the requirement path from a module path in tryUpgrade to the
+ // error, and the requirement path (if any) from rs.rootModules to the
+ // tryUpgrade module path. Return a *mvs.BuildListError showing the
+ // concatenation of the paths (with an upgrade in the middle).
+ }
+ return err
+ }
+
+ for _, r := range upgradeGraph.BuildList() {
+ // Upgrading to m would upgrade to r, and the caller requested that we
+ // try to upgrade to m, so it's ok to upgrade to r.
+ allow(r)
+ }
+ }
+
+ if len(mustSelect) > 0 {
+ mustGraph, err := readModGraph(ctx, depth, mustSelect)
+ if err != nil {
+ return err
+ }
+
+ for _, r := range mustGraph.BuildList() {
+ // Some module in mustSelect requires r, so we must allow at least r.Version
+ // unless it conflicts with an entry in mustSelect.
+ allow(r)
+ }
+ }
+
+ return nil
+}
+
+// selectPotentiallyImportedModules increases the limiter-selected version of
+// every module in rs that potentially provides a package imported (directly or
+// indirectly) by the main module, and every module in tryUpgrade, toward the
+// highest version seen in rs or tryUpgrade, but not above the maximums enforced
+// by the limiter.
+//
+// It returns the list of module versions selected by the limiter, sorted by
+// path, along with a boolean indicating whether that list is different from the
+// list of modules read from rs.
+func selectPotentiallyImportedModules(ctx context.Context, limiter *versionLimiter, rs *Requirements, tryUpgrade []module.Version) (mods []module.Version, changed bool, err error) {
for _, m := range tryUpgrade {
- if err := limiter.upgradeToward(ctx, m); err != nil {
- return nil, err
+ if err := limiter.UpgradeToward(ctx, m); err != nil {
+ return nil, false, err
+ }
+ }
+
+ var initial []module.Version
+ if rs.depth == eager {
+ mg, err := rs.Graph(ctx)
+ if err != nil {
+ return nil, false, err
}
+ initial = mg.BuildList()[1:]
+ } else {
+ initial = rs.rootModules
}
for _, m := range initial {
- if err := limiter.upgradeToward(ctx, m); err != nil {
- return nil, err
+ if err := limiter.UpgradeToward(ctx, m); err != nil {
+ return nil, false, err
+ }
+ }
+
+ mods = make([]module.Version, 0, len(limiter.selected))
+ for path, v := range limiter.selected {
+ if v != "none" && path != Target.Path {
+ mods = append(mods, module.Version{Path: path, Version: v})
}
}
@@ -107,46 +328,46 @@ func editBuildList(ctx context.Context, initial, tryUpgrade, mustSelect []module
// 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.
- adjusted := make([]module.Version, 0, len(maxes))
- for _, m := range maxes[1:] {
- if v, ok := limiter.selected[m.Path]; ok {
- adjusted = append(adjusted, module.Version{Path: m.Path, Version: v})
- }
- }
- consistent, err := mvs.BuildList(Target, &mvsReqs{roots: adjusted})
+ mg, err := readModGraph(ctx, rs.depth, mods)
if err != nil {
- return nil, err
+ return nil, false, err
}
-
- // We have the correct selected versions. Now we need to re-run MVS with only
- // the actually-selected versions in order to eliminate extraneous
- // dependencies from lower-than-selected ones.
- compacted := consistent[:0]
- for _, m := range consistent[1:] {
- if _, ok := limiter.selected[m.Path]; ok {
- // The fact that the limiter has a version for m.Path indicates that we
- // care about retaining that path, even if the version was upgraded for
- // consistency.
- compacted = append(compacted, m)
+ mods = make([]module.Version, 0, len(limiter.selected))
+ for path, _ := range limiter.selected {
+ if path != Target.Path {
+ if v := mg.Selected(path); v != "none" {
+ mods = append(mods, module.Version{Path: path, Version: v})
+ }
}
}
+ module.Sort(mods)
+
+ changed = !reflect.DeepEqual(mods, initial)
- return mvs.BuildList(Target, &mvsReqs{roots: compacted})
+ return mods, changed, err
}
// 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
+ // Select and UpgradeToward are loaded.
+ depth modDepth
+
// max maps each module path to the maximum version that may be selected for
- // that path. Paths with no entry are unrestricted.
+ // 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).
max map[string]string
// selected maps each module path to a version of that path (if known) whose
// transitive dependencies do not violate any max version. The version kept
- // is the highest one found during any call to upgradeToward for the given
+ // is the highest one found during any call to UpgradeToward for the given
// module path.
//
- // If a higher acceptable version is found during a call to upgradeToward for
+ // If a higher acceptable version is found during a call to UpgradeToward for
// some *other* module path, that does not update the selected version.
// Ignoring those versions keeps the downgrades computed for two modules
// together close to the individual downgrades that would be computed for each
@@ -183,18 +404,32 @@ func (dq dqState) isDisqualified() bool {
return dq != dqState{}
}
-func newVersionLimiter(max map[string]string) *versionLimiter {
+// newVersionLimiter returns a versionLimiter that restricts the module paths
+// that appear as keys in max.
+//
+// max maps each module path to its maximum version; paths that are not present
+// 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 {
return &versionLimiter{
- selected: map[string]string{Target.Path: Target.Version},
+ depth: depth,
max: max,
+ selected: map[string]string{Target.Path: Target.Version},
dqReason: map[module.Version]dqState{},
requiring: map[module.Version][]module.Version{},
}
}
-// upgradeToward attempts to upgrade the selected version of m.Path as close as
+// UpgradeToward attempts to upgrade the selected version of m.Path as close as
// possible to m.Version without violating l's maximum version limits.
-func (l *versionLimiter) upgradeToward(ctx context.Context, m module.Version) error {
+//
+// If depth is lazy and m itself is lazy, the 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 {
if cmpVersion(selected, m.Version) >= 0 {
@@ -205,7 +440,7 @@ func (l *versionLimiter) upgradeToward(ctx context.Context, m module.Version) er
selected = "none"
}
- if l.check(m).isDisqualified() {
+ if l.check(m, l.depth).isDisqualified() {
candidates, err := versions(ctx, m.Path, CheckAllowed)
if err != nil {
// This is likely a transient error reaching the repository,
@@ -222,7 +457,7 @@ func (l *versionLimiter) upgradeToward(ctx context.Context, m module.Version) er
})
candidates = candidates[:i]
- for l.check(m).isDisqualified() {
+ for l.check(m, l.depth).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.
@@ -237,9 +472,26 @@ func (l *versionLimiter) upgradeToward(ctx context.Context, m module.Version) er
return nil
}
+// 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)
+ if !dq.isDisqualified() {
+ l.selected[m.Path] = m.Version
+ }
+ return dq.conflict, dq.err
+}
+
// check determines whether m (or its transitive dependencies) would violate l's
// maximum version limits if added to the module requirement graph.
-func (l *versionLimiter) check(m module.Version) dqState {
+//
+// 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 m.Version == "none" || m == Target {
// version "none" has no requirements, and the dependencies of Target are
// tautological.
@@ -270,8 +522,20 @@ func (l *versionLimiter) check(m module.Version) dqState {
return l.disqualify(m, dqState{err: err})
}
+ if summary.depth() == eager {
+ depth = eager
+ }
for _, r := range summary.require {
- if dq := l.check(r); dq.isDisqualified() {
+ if depth == lazy {
+ 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.
+ continue
+ }
+ }
+
+ if dq := l.check(r, depth); dq.isDisqualified() {
return l.disqualify(m, dq)
}