1
0
mirror of https://github.com/golang/go synced 2024-11-26 05:07:59 -07:00

cmd/go/internal/modload: split updateRoots into separate functions for updating and tidying

In CL 293689, I fused the mvs.Reqs calls that were formerly in MinReqs
and TidyBuildList into a single function, updateRoots, in the hope
that it expressed a fundamental operation. As I have been working on
the lazy equivalents, I have come to realize that these functions are
deeply related but fundamentally different.

In order to help me reason about the two different roles, I am making
the two functions separate once more, but leaving them colocated in
the code.

For #36460

Change-Id: I851d6d81fbfd84f39411e0d076ee72a9909c60ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/310629
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>
This commit is contained in:
Bryan C. Mills 2021-04-15 16:54:41 -04:00
parent 81fcb18df5
commit 69c94ad55f
3 changed files with 112 additions and 86 deletions

View File

@ -409,7 +409,7 @@ func expandGraph(ctx context.Context, rs *Requirements) (*Requirements, *ModuleG
// roots — but in a lazy module it may pull in previously-irrelevant
// transitive dependencies.
newRS, rsErr := updateRoots(ctx, rs.depth, rs.direct, nil, rs)
newRS, rsErr := updateRoots(ctx, rs.direct, rs)
if rsErr != nil {
// Failed to update roots, perhaps because of an error in a transitive
// dependency needed for the update. Return the original Requirements
@ -552,7 +552,15 @@ func tidyBuildList(ctx context.Context, ld *loader, initialRS *Requirements) *Re
// changed after loading packages.
}
tidy, err := updateRoots(ctx, depth, ld.requirements.direct, ld.pkgs, nil)
var (
tidy *Requirements
err error
)
if depth == lazy {
panic("internal error: 'go mod tidy' for lazy modules is not yet implemented")
} else {
tidy, err = tidyEagerRoots(ctx, ld.requirements, ld.pkgs)
}
if err != nil {
base.Fatalf("go: %v", err)
}
@ -573,13 +581,42 @@ func tidyBuildList(ctx context.Context, ld *loader, initialRS *Requirements) *Re
return tidy
}
// 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 rs.direct as a root.
func tidyEagerRoots(ctx context.Context, rs *Requirements, pkgs []*loadPkg) (*Requirements, error) {
var (
keep []module.Version
keptPath = map[string]bool{}
)
var (
rootPaths []string // module paths that should be included as roots
inRootPaths = map[string]bool{}
)
for _, pkg := range pkgs {
if !pkg.fromExternalModule() {
continue
}
if m := pkg.mod; !keptPath[m.Path] {
keep = append(keep, m)
keptPath[m.Path] = true
if rs.direct[m.Path] && !inRootPaths[m.Path] {
rootPaths = append(rootPaths, m.Path)
inRootPaths[m.Path] = true
}
}
}
min, err := mvs.Req(Target, rootPaths, &mvsReqs{roots: keep})
if err != nil {
return nil, err
}
return newRequirements(eager, min, rs.direct), nil
}
// updateRoots returns a set of root requirements that includes the selected
// version of every module path in direct as a root, and maintains the selected
// versions of every module selected in the graph of rs (if rs is non-nil), or
// every module that provides any package in pkgs (otherwise).
//
// If pkgs is non-empty and rs is non-nil, the packages are assumed to be loaded
// from the modules selected in the graph of rs.
// version of every module selected in the graph of rs.
//
// The roots are updated such that:
//
@ -587,45 +624,62 @@ func tidyBuildList(ctx context.Context, ld *loader, initialRS *Requirements) *Re
// (if it is not "none").
// 2. Each root is the selected version of its path. (We say that such a root
// set is “consistent”.)
// 3. The selected version of the module providing each package in pkgs remains
// selected.
// 4. If rs is non-nil, every version selected in the graph of rs remains selected.
func updateRoots(ctx context.Context, depth modDepth, direct map[string]bool, pkgs []*loadPkg, rs *Requirements) (*Requirements, error) {
// 3. Every version selected in the graph of rs remains selected.
func updateRoots(ctx context.Context, direct map[string]bool, rs *Requirements) (*Requirements, error) {
mg, err := rs.Graph(ctx)
if err != nil {
// We can't ignore errors in the module graph even if the user passed the -e
// flag to try to push past them. If we can't load the complete module
// dependencies, then we can't reliably compute a minimal subset of them.
return rs, err
}
if cfg.BuildMod != "mod" {
// Instead of actually updating the requirements, just check that no updates
// are needed.
if rs == nil {
// We're being asked to reconstruct the requirements from scratch,
// but we aren't even allowed to modify them.
return rs, errGoModDirty
}
for _, m := range rs.rootModules {
if m.Version != mg.Selected(m.Path) {
// The root version v is misleading: the actual selected version is higher.
return rs, errGoModDirty
}
}
for mPath := range direct {
if _, ok := rs.rootSelected(mPath); !ok {
// Module m is supposed to be listed explicitly, but isn't.
//
// Note that this condition is also detected (and logged with more
// detail) earlier during package loading, so it shouldn't actually be
// possible at this point — this is just a defense in depth.
return rs, errGoModDirty
}
}
// No explicit roots are missing and all roots are already at the versions
// we want to keep. Any other changes we would make are purely cosmetic,
// such as pruning redundant indirect dependencies. Per issue #34822, we
// ignore cosmetic changes when we cannot update the go.mod file.
return rs, nil
}
var (
rootPaths []string // module paths that should be included as roots
inRootPaths = map[string]bool{}
)
var keep []module.Version
if rs != nil {
mg, err := rs.Graph(ctx)
if err != nil {
// We can't ignore errors in the module graph even if the user passed the -e
// flag to try to push past them. If we can't load the complete module
// dependencies, then we can't reliably compute a minimal subset of them.
return rs, err
}
keep = mg.BuildList()[1:]
for _, root := range rs.rootModules {
// If the selected version of the root is the same as what was already
// listed in the go.mod file, retain it as a root (even if redundant) to
// avoid unnecessary churn. (See https://golang.org/issue/34822.)
//
// We do this even for indirect requirements, since we don't know why they
// were added and they could become direct at any time.
if !inRootPaths[root.Path] && mg.Selected(root.Path) == root.Version {
rootPaths = append(rootPaths, root.Path)
inRootPaths[root.Path] = true
}
}
} else {
kept := map[module.Version]bool{Target: true}
for _, pkg := range pkgs {
if pkg.mod.Path != "" && !kept[pkg.mod] {
keep = append(keep, pkg.mod)
kept[pkg.mod] = true
}
for _, root := range rs.rootModules {
// If the selected version of the root is the same as what was already
// listed in the go.mod file, retain it as a root (even if redundant) to
// avoid unnecessary churn. (See https://golang.org/issue/34822.)
//
// We do this even for indirect requirements, since we don't know why they
// were added and they could become direct at any time.
if !inRootPaths[root.Path] && mg.Selected(root.Path) == root.Version {
rootPaths = append(rootPaths, root.Path)
inRootPaths[root.Path] = true
}
}
@ -637,6 +691,7 @@ func updateRoots(ctx context.Context, depth modDepth, direct map[string]bool, pk
if go117LazyTODO {
// Update the above comment to reflect lazy loading once implemented.
}
keep := mg.BuildList()[1:]
for _, m := range keep {
if direct[m.Path] && !inRootPaths[m.Path] {
rootPaths = append(rootPaths, m.Path)
@ -644,47 +699,6 @@ func updateRoots(ctx context.Context, depth modDepth, direct map[string]bool, pk
}
}
if cfg.BuildMod != "mod" {
// Instead of actually updating the requirements, just check that no updates
// are needed.
if rs == nil {
// We're being asked to reconstruct the requirements from scratch,
// but we aren't even allowed to modify them.
return rs, errGoModDirty
}
for _, mPath := range rootPaths {
if _, ok := rs.rootSelected(mPath); !ok {
// Module m is supposed to be listed explicitly, but isn't.
//
// Note that this condition is also detected (and logged with more
// detail) earlier during package loading, so it shouldn't actually be
// possible at this point — this is just a defense in depth.
return rs, errGoModDirty
}
}
for _, m := range keep {
if v, ok := rs.rootSelected(m.Path); ok && v != m.Version {
// The root version v is misleading: the actual selected version is
// m.Version.
return rs, errGoModDirty
}
}
for _, m := range rs.rootModules {
if v, ok := rs.rootSelected(m.Path); ok && v != m.Version {
// The roots list both m.Version and some higher version of m.Path.
// The root for m.Version is misleading: the actual selected version is
// *at least* v.
return rs, errGoModDirty
}
}
// No explicit roots are missing and all roots are already at the versions
// we want to keep. Any other changes we would make are purely cosmetic,
// such as pruning redundant indirect dependencies. Per issue #34822, we
// ignore cosmetic changes when we cannot update the go.mod file.
return rs, nil
}
min, err := mvs.Req(Target, rootPaths, &mvsReqs{roots: keep})
if err != nil {
return rs, err
@ -695,7 +709,7 @@ func updateRoots(ctx context.Context, depth modDepth, direct map[string]bool, pk
// the root set is the same as the original root set in rs and recycle its
// module graph and build list, if they have already been loaded.
return newRequirements(depth, min, direct), nil
return newRequirements(rs.depth, min, direct), nil
}
// checkMultiplePaths verifies that a given module path is used as itself

View File

@ -691,7 +691,7 @@ func requirementsFromModFile(ctx context.Context, f *modfile.File) *Requirements
for _, n := range mPathCount {
if n > 1 {
var err error
rs, err = updateRoots(ctx, rs.depth, rs.direct, nil, rs)
rs, err = updateRoots(ctx, rs.direct, rs)
if err != nil {
base.Fatalf("go: %v", err)
}

View File

@ -864,6 +864,18 @@ func (pkg *loadPkg) isTest() bool {
return pkg.testOf != nil
}
// fromExternalModule reports whether pkg was loaded from a module other than
// the main module.
func (pkg *loadPkg) fromExternalModule() bool {
if pkg.mod.Path == "" {
return false // loaded from the standard library, not a module
}
if pkg.mod.Path == Target.Path {
return false // loaded from the main module.
}
return true
}
var errMissing = errors.New("cannot find package")
// loadFromRoots attempts to load the build graph needed to process a set of
@ -1030,7 +1042,7 @@ func (ld *loader) updateRequirements(ctx context.Context) error {
}
}
rs, err := updateRoots(ctx, rs.depth, direct, ld.pkgs, rs)
rs, err := updateRoots(ctx, direct, rs)
if err == nil {
ld.requirements = rs
}