qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-26 05:48:05 -07:00
Code Releases Activity

cmd/go/internal/modload: remove the addedModuleFor map

Browse Source 
At one point this map checked for infinite loops during package iteration.
The last write to the map was mistakenly removed in CL 251445.

However, looking at the code before that change, the map-based
termination strategy was never quite right to begin with: it checked
whether we had ever added any module for the given package, not
whether we had already added the module being proposed right now. (For
packages within nested modules, we could try adding multiple different
modules for a given package without looping.)

Moreover, the "looping trying to add package" failure message was only
marginally helpful. Users are capable of noticing that an invocation
of the 'go' command is taking too long, and will report a bug for an
infinite loop just as readily as a "looping trying to add package"
error.

We could try to add this tracking back in, but it's no substitute for
a proper proof of convergence, and the code is simpler without it.
Instead I'm going to add a proper proof of convergence — or, barring
that, a more accurate and useful check for failure to converge. In the
meantime, this invariantly-empty map isn't doing anybody any good.

For #36460

Change-Id: I2c111d4b4bf59159af0d7e62d1c0ef4ce0a43a71
Reviewed-on: https://go-review.googlesource.com/c/go/+/312929
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>
This commit is contained in:
Bryan C. Mills 2021-04-23 01:13:50 -04:00
parent c3e2ed711c
commit 768a39975d
1 changed files with 2 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/cmd/go/internal/modload/load.go
View File

@ -890,7 +890,6 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader {
work: par.NewQueue(runtime.GOMAXPROCS(0)),
}
addedModuleFor := make(map[string]bool)
for {
ld.reset()
@ -931,7 +930,7 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader {
// We've loaded as much as we can without resolving missing imports.
break
}
modAddedBy := ld.resolveMissingImports(ctx, addedModuleFor)
modAddedBy := ld.resolveMissingImports(ctx)
if len(modAddedBy) == 0 {
break
}
@ -1057,7 +1056,7 @@ func (ld *loader) updateRequirements(ctx context.Context, add []module.Version)
// The newly-resolved packages are added to the addedModuleFor map, and
// resolveMissingImports returns a map from each new module version to
// the first missing package that module would resolve.
func (ld *loader) resolveMissingImports(ctx context.Context, addedModuleFor map[string]bool) (modAddedBy map[module.Version]*loadPkg) {
func (ld *loader) resolveMissingImports(ctx context.Context) (modAddedBy map[module.Version]*loadPkg) {
var needPkgs []*loadPkg
for _, pkg := range ld.pkgs {
if pkg.err == nil {
@ -1089,11 +1088,6 @@ func (ld *loader) resolveMissingImports(ctx context.Context, addedModuleFor map[
}
fmt.Fprintf(os.Stderr, "go: found %s in %s %s\n", pkg.path, pkg.mod.Path, pkg.mod.Version)
if addedModuleFor[pkg.path] {
// TODO(bcmills): This should only be an error if pkg.mod is the same
// version we already tried to add previously.
base.Fatalf("go: %s: looping trying to add package", pkg.stackText())
}
if modAddedBy[pkg.mod] == nil {
modAddedBy[pkg.mod] = pkg
}