cmd/go: revert "in 'go get', only load retractions for resolved versions"
This reverts CL 269019.
Reason for revert: The TestScript/mod_gonoproxy test is failing
on linux-386-longtest and linux-amd64-longtest builders.
Change-Id: I7e132fb4fb5a9c00add28e5100a0e96a9250282c
Reviewed-on: https://go-review.googlesource.com/c/go/+/270521
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index e7cfce1..5b8eebf 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -419,7 +419,20 @@
pkgPatterns = append(pkgPatterns, q.pattern)
}
}
- r.checkPackagesAndRetractions(ctx, pkgPatterns)
+ if len(pkgPatterns) > 0 {
+ // We skipped over missing-package errors earlier: we want to resolve
+ // pathSets ourselves, but at that point we don't have enough context
+ // to log the package-import chains leading to the error. Reload the package
+ // import graph one last time to report any remaining unresolved
+ // dependencies.
+ pkgOpts := modload.PackageOpts{
+ LoadTests: *getT,
+ ResolveMissingImports: false,
+ AllowErrors: false,
+ }
+ modload.LoadPackages(ctx, pkgOpts, pkgPatterns...)
+ base.ExitIfErrors()
+ }
// We've already downloaded modules (and identified direct and indirect
// dependencies) by loading packages in findAndUpgradeImports.
@@ -466,6 +479,15 @@
modload.AllowWriteGoMod()
modload.WriteGoMod()
modload.DisallowWriteGoMod()
+
+ // Report warnings if any retracted versions are in the build list.
+ // This must be done after writing go.mod to avoid spurious '// indirect'
+ // comments. These functions read and write global state.
+ //
+ // TODO(golang.org/issue/40775): ListModules (called from reportRetractions)
+ // resets modload.loader, which contains information about direct dependencies
+ // that WriteGoMod uses. Refactor to avoid these kinds of global side effects.
+ reportRetractions(ctx)
}
// parseArgs parses command-line arguments and reports errors.
@@ -503,6 +525,43 @@
return queries
}
+// reportRetractions prints warnings if any modules in the build list are
+// retracted.
+func reportRetractions(ctx context.Context) {
+ // Query for retractions of modules in the build list.
+ // Use modload.ListModules, since that provides information in the same format
+ // as 'go list -m'. Don't query for "all", since that's not allowed outside a
+ // module.
+ buildList := modload.LoadedModules()
+ args := make([]string, 0, len(buildList))
+ for _, m := range buildList {
+ if m.Version == "" {
+ // main module or dummy target module
+ continue
+ }
+ args = append(args, m.Path+"@"+m.Version)
+ }
+ listU := false
+ listVersions := false
+ listRetractions := true
+ mods := modload.ListModules(ctx, args, listU, listVersions, listRetractions)
+ retractPath := ""
+ for _, mod := range mods {
+ if len(mod.Retracted) > 0 {
+ if retractPath == "" {
+ retractPath = mod.Path
+ } else {
+ retractPath = "<module>"
+ }
+ rationale := modload.ShortRetractionRationale(mod.Retracted[0])
+ fmt.Fprintf(os.Stderr, "go: warning: %s@%s is retracted: %s\n", mod.Path, mod.Version, rationale)
+ }
+ }
+ if modload.HasModRoot() && retractPath != "" {
+ fmt.Fprintf(os.Stderr, "go: run 'go get %s@latest' to switch to the latest unretracted version\n", retractPath)
+ }
+}
+
type resolver struct {
localQueries []*query // queries for absolute or relative paths
pathQueries []*query // package path literal queries in original order
@@ -529,6 +588,9 @@
work *par.Queue
+ queryModuleCache par.Cache
+ queryPackagesCache par.Cache
+ queryPatternCache par.Cache
matchInModuleCache par.Cache
}
@@ -613,7 +675,7 @@
return nil, false
}
-// queryModule wraps modload.Query, substituting r.checkAllowedOr to decide
+// queryModule wraps modload.Query, substituting r.checkAllowedor to decide
// allowed versions.
func (r *resolver) queryModule(ctx context.Context, mPath, query string, selected func(string) string) (module.Version, error) {
current := r.initialSelected(mPath)
@@ -1146,7 +1208,7 @@
}
mu.Lock()
- upgrades = append(upgrades, pathSet{path: path, pkgMods: pkgMods, err: err})
+ upgrades = append(upgrades, pathSet{pkgMods: pkgMods, err: err})
mu.Unlock()
return false
}
@@ -1473,87 +1535,6 @@
return false, cs.mod
}
-// checkPackagesAndRetractions reloads packages for the given patterns and
-// reports missing and ambiguous package errors. It also reports loads and
-// reports retractions for resolved modules and modules needed to build
-// named packages.
-//
-// We skip missing-package errors earlier in the process, since we want to
-// resolve pathSets ourselves, but at that point, we don't have enough context
-// to log the package-import chains leading to each error.
-func (r *resolver) checkPackagesAndRetractions(ctx context.Context, pkgPatterns []string) {
- defer base.ExitIfErrors()
-
- // Build a list of modules to load retractions for. Start with versions
- // selected based on command line queries.
- //
- // This is a subset of the build list. If the main module has a lot of
- // dependencies, loading retractions for the entire build list would be slow.
- relevantMods := make(map[module.Version]struct{})
- for path, reason := range r.resolvedVersion {
- relevantMods[module.Version{Path: path, Version: reason.version}] = struct{}{}
- }
-
- // Reload packages, reporting errors for missing and ambiguous imports.
- if len(pkgPatterns) > 0 {
- // LoadPackages will print errors (since it has more context) but will not
- // exit, since we need to load retractions later.
- pkgOpts := modload.PackageOpts{
- LoadTests: *getT,
- ResolveMissingImports: false,
- AllowErrors: true,
- }
- matches, pkgs := modload.LoadPackages(ctx, pkgOpts, pkgPatterns...)
- for _, m := range matches {
- if len(m.Errs) > 0 {
- base.SetExitStatus(1)
- break
- }
- }
- for _, pkg := range pkgs {
- if _, _, err := modload.Lookup("", false, pkg); err != nil {
- base.SetExitStatus(1)
- if ambiguousErr := (*modload.AmbiguousImportError)(nil); errors.As(err, &ambiguousErr) {
- for _, m := range ambiguousErr.Modules {
- relevantMods[m] = struct{}{}
- }
- }
- }
- if m := modload.PackageModule(pkg); m.Path != "" {
- relevantMods[m] = struct{}{}
- }
- }
- }
-
- // Load and report retractions.
- type retraction struct {
- m module.Version
- err error
- }
- retractions := make([]retraction, 0, len(relevantMods))
- for m := range relevantMods {
- retractions = append(retractions, retraction{m: m})
- }
- sort.Slice(retractions, func(i, j int) bool {
- return retractions[i].m.Path < retractions[j].m.Path
- })
- for i := 0; i < len(retractions); i++ {
- i := i
- r.work.Add(func() {
- err := modload.CheckRetractions(ctx, retractions[i].m)
- if retractErr := (*modload.ModuleRetractedError)(nil); errors.As(err, &retractErr) {
- retractions[i].err = err
- }
- })
- }
- <-r.work.Idle()
- for _, r := range retractions {
- if r.err != nil {
- fmt.Fprintf(os.Stderr, "go: warning: %v\n", r.err)
- }
- }
-}
-
// reportChanges logs resolved version changes to os.Stderr.
func (r *resolver) reportChanges(queries []*query) {
for _, q := range queries {
diff --git a/src/cmd/go/internal/modload/build.go b/src/cmd/go/internal/modload/build.go
index b9e3440..b9abb0b 100644
--- a/src/cmd/go/internal/modload/build.go
+++ b/src/cmd/go/internal/modload/build.go
@@ -123,13 +123,13 @@
return
}
- err := CheckRetractions(ctx, module.Version{Path: m.Path, Version: m.Version})
- var rerr *ModuleRetractedError
+ err := checkRetractions(ctx, module.Version{Path: m.Path, Version: m.Version})
+ var rerr *retractedError
if errors.As(err, &rerr) {
- if len(rerr.Rationale) == 0 {
+ if len(rerr.rationale) == 0 {
m.Retracted = []string{"retracted by module author"}
} else {
- m.Retracted = rerr.Rationale
+ m.Retracted = rerr.rationale
}
} else if err != nil && m.Error == nil {
m.Error = &modinfo.ModuleError{Err: err.Error()}
diff --git a/src/cmd/go/internal/modload/modfile.go b/src/cmd/go/internal/modload/modfile.go
index e9601c3..7a89632 100644
--- a/src/cmd/go/internal/modload/modfile.go
+++ b/src/cmd/go/internal/modload/modfile.go
@@ -59,7 +59,7 @@
if err := CheckExclusions(ctx, m); err != nil {
return err
}
- if err := CheckRetractions(ctx, m); err != nil {
+ if err := checkRetractions(ctx, m); err != nil {
return err
}
return nil
@@ -85,9 +85,9 @@
func (e *excludedError) Error() string { return "excluded by go.mod" }
func (e *excludedError) Is(err error) bool { return err == ErrDisallowed }
-// CheckRetractions returns an error if module m has been retracted by
+// checkRetractions returns an error if module m has been retracted by
// its author.
-func CheckRetractions(ctx context.Context, m module.Version) error {
+func checkRetractions(ctx context.Context, m module.Version) error {
if m.Version == "" {
// Main module, standard library, or file replacement module.
// Cannot be retracted.
@@ -165,28 +165,28 @@
}
}
if isRetracted {
- return module.VersionError(m, &ModuleRetractedError{Rationale: rationale})
+ return module.VersionError(m, &retractedError{rationale: rationale})
}
return nil
}
var retractCache par.Cache
-type ModuleRetractedError struct {
- Rationale []string
+type retractedError struct {
+ rationale []string
}
-func (e *ModuleRetractedError) Error() string {
+func (e *retractedError) Error() string {
msg := "retracted by module author"
- if len(e.Rationale) > 0 {
+ if len(e.rationale) > 0 {
// This is meant to be a short error printed on a terminal, so just
// print the first rationale.
- msg += ": " + ShortRetractionRationale(e.Rationale[0])
+ msg += ": " + ShortRetractionRationale(e.rationale[0])
}
return msg
}
-func (e *ModuleRetractedError) Is(err error) bool {
+func (e *retractedError) Is(err error) bool {
return err == ErrDisallowed
}
diff --git a/src/cmd/go/testdata/mod/example.com_retract_ambiguous_nested_v1.9.0-bad.txt b/src/cmd/go/testdata/mod/example.com_retract_ambiguous_nested_v1.9.0-bad.txt
deleted file mode 100644
index f8e623d..0000000
--- a/src/cmd/go/testdata/mod/example.com_retract_ambiguous_nested_v1.9.0-bad.txt
+++ /dev/null
@@ -1,10 +0,0 @@
--- .mod --
-module example.com/retract/ambiguous/nested
-
-go 1.16
-
-retract v1.9.0-bad // nested modules are bad
--- .info --
-{"Version":"v1.9.0-bad"}
--- nested.go --
-package nested
diff --git a/src/cmd/go/testdata/mod/example.com_retract_ambiguous_other_v1.0.0.txt b/src/cmd/go/testdata/mod/example.com_retract_ambiguous_other_v1.0.0.txt
deleted file mode 100644
index 5ee0139..0000000
--- a/src/cmd/go/testdata/mod/example.com_retract_ambiguous_other_v1.0.0.txt
+++ /dev/null
@@ -1,12 +0,0 @@
--- .mod --
-module example.com/retract/ambiguous/other
-
-go 1.16
-
-require example.com/retract/ambiguous v1.0.0
--- .info --
-{"Version":"v1.0.0"}
--- other.go --
-package other
-
-import _ "example.com/retract/ambiguous/nested"
diff --git a/src/cmd/go/testdata/mod/example.com_retract_ambiguous_v1.0.0.txt b/src/cmd/go/testdata/mod/example.com_retract_ambiguous_v1.0.0.txt
deleted file mode 100644
index c8eeb16..0000000
--- a/src/cmd/go/testdata/mod/example.com_retract_ambiguous_v1.0.0.txt
+++ /dev/null
@@ -1,9 +0,0 @@
--- .mod --
-module example.com/retract/ambiguous
-
-go 1.16
--- .info --
-{"Version":"v1.0.0"}
--- nested/nested.go --
-package nested
-
diff --git a/src/cmd/go/testdata/script/mod_get_retract.txt b/src/cmd/go/testdata/script/mod_get_retract.txt
index 13a47bc..da6c255 100644
--- a/src/cmd/go/testdata/script/mod_get_retract.txt
+++ b/src/cmd/go/testdata/script/mod_get_retract.txt
@@ -10,7 +10,7 @@
cp go.mod.orig go.mod
go mod edit -require example.com/retract/self/prev@v1.9.0
go get -d example.com/retract/self/prev
-stderr '^go: warning: example.com/retract/self/prev@v1.9.0: retracted by module author: self$'
+stderr '^go: warning: example.com/retract/self/prev@v1.9.0 is retracted: self$'
go list -m example.com/retract/self/prev
stdout '^example.com/retract/self/prev v1.9.0$'
@@ -25,7 +25,7 @@
# version is retracted.
cp go.mod.orig go.mod
go get -d example.com/retract@v1.0.0-bad
-stderr '^go: warning: example.com/retract@v1.0.0-bad: retracted by module author: bad$'
+stderr '^go: warning: example.com/retract@v1.0.0-bad is retracted: bad$'
go list -m example.com/retract
stdout '^example.com/retract v1.0.0-bad$'
@@ -33,26 +33,17 @@
# version is available.
cp go.mod.orig go.mod
go mod edit -require example.com/retract/self/prev@v1.9.0
-go get -d -u ./use
-stderr '^go: warning: example.com/retract/self/prev@v1.9.0: retracted by module author: self$'
+go get -d -u .
+stderr '^go: warning: example.com/retract/self/prev@v1.9.0 is retracted: self$'
go list -m example.com/retract/self/prev
stdout '^example.com/retract/self/prev v1.9.0$'
-# 'go get' should warn if a module needed to build named packages is retracted.
-# 'go get' should not warn about unrelated modules.
-go get -d ./empty
-! stderr retracted
-go get -d ./use
-stderr '^go: warning: example.com/retract/self/prev@v1.9.0: retracted by module author: self$'
-
-- go.mod.orig --
module example.com/use
go 1.15
--- use/use.go --
+-- use.go --
package use
import _ "example.com/retract/self/prev"
--- empty/empty.go --
-package empty
diff --git a/src/cmd/go/testdata/script/mod_get_retract_ambiguous.txt b/src/cmd/go/testdata/script/mod_get_retract_ambiguous.txt
deleted file mode 100644
index b49ba54..0000000
--- a/src/cmd/go/testdata/script/mod_get_retract_ambiguous.txt
+++ /dev/null
@@ -1,10 +0,0 @@
-! go get -d example.com/retract/ambiguous/other
-stderr 'ambiguous import: found package example.com/retract/ambiguous/nested in multiple modules:'
-stderr '^go: warning: example.com/retract/ambiguous/nested@v1.9.0-bad: retracted by module author: nested modules are bad$'
-
--- go.mod --
-module example.com/use
-
-go 1.16
-
-require example.com/retract/ambiguous/nested v1.9.0-bad
diff --git a/src/cmd/go/testdata/script/mod_retract_rationale.txt b/src/cmd/go/testdata/script/mod_retract_rationale.txt
index 4d3a3d6..584c3a3 100644
--- a/src/cmd/go/testdata/script/mod_retract_rationale.txt
+++ b/src/cmd/go/testdata/script/mod_retract_rationale.txt
@@ -1,6 +1,6 @@
# When there is no rationale, 'go get' should print a hard-coded message.
go get -d example.com/retract/rationale@v1.0.0-empty
-stderr '^go: warning: example.com/retract/rationale@v1.0.0-empty: retracted by module author$'
+stderr '^go: warning: example.com/retract/rationale@v1.0.0-empty is retracted: retracted by module author$'
# 'go list' should print the same hard-coded message.
go list -m -retracted -f '{{.Retracted}}' example.com/retract/rationale
@@ -9,7 +9,7 @@
# When there is a multi-line message, 'go get' should print the first line.
go get -d example.com/retract/rationale@v1.0.0-multiline1
-stderr '^go: warning: example.com/retract/rationale@v1.0.0-multiline1: retracted by module author: short description$'
+stderr '^go: warning: example.com/retract/rationale@v1.0.0-multiline1 is retracted: short description$'
! stderr 'detail'
# 'go list' should show the full message.
@@ -19,7 +19,7 @@
# 'go get' output should be the same whether the retraction appears at top-level
# or in a block.
go get -d example.com/retract/rationale@v1.0.0-multiline2
-stderr '^go: warning: example.com/retract/rationale@v1.0.0-multiline2: retracted by module author: short description$'
+stderr '^go: warning: example.com/retract/rationale@v1.0.0-multiline2 is retracted: short description$'
! stderr 'detail'
# Same for 'go list'.
@@ -29,7 +29,7 @@
# 'go get' should omit long messages.
go get -d example.com/retract/rationale@v1.0.0-long
-stderr '^go: warning: example.com/retract/rationale@v1.0.0-long: retracted by module author: \(rationale omitted: too long\)'
+stderr '^go: warning: example.com/retract/rationale@v1.0.0-long is retracted: \(rationale omitted: too long\)'
# 'go list' should show the full message.
go list -m -retracted -f '{{.Retracted}}' example.com/retract/rationale
@@ -38,7 +38,7 @@
# 'go get' should omit messages with unprintable characters.
go get -d example.com/retract/rationale@v1.0.0-unprintable
-stderr '^go: warning: example.com/retract/rationale@v1.0.0-unprintable: retracted by module author: \(rationale omitted: contains non-printable characters\)'
+stderr '^go: warning: example.com/retract/rationale@v1.0.0-unprintable is retracted: \(rationale omitted: contains non-printable characters\)'
# 'go list' should show the full message.
go list -m -retracted -f '{{.Retracted}}' example.com/retract/rationale
@@ -62,9 +62,9 @@
# 'go get' will only report the first retraction to avoid being too verbose.
go get -d example.com/retract/rationale@v1.0.0-order
-stderr '^go: warning: example.com/retract/rationale@v1.0.0-order: retracted by module author: degenerate range$'
+stderr '^go: warning: example.com/retract/rationale@v1.0.0-order is retracted: degenerate range$'
go get -d example.com/retract/rationale@v1.0.1-order
-stderr '^go: warning: example.com/retract/rationale@v1.0.1-order: retracted by module author: single version$'
+stderr '^go: warning: example.com/retract/rationale@v1.0.1-order is retracted: single version$'
-- go.mod --
module m
diff --git a/src/cmd/go/testdata/script/mod_retract_rename.txt b/src/cmd/go/testdata/script/mod_retract_rename.txt
index f54742c..b75bfe9 100644
--- a/src/cmd/go/testdata/script/mod_retract_rename.txt
+++ b/src/cmd/go/testdata/script/mod_retract_rename.txt
@@ -10,7 +10,7 @@
# 'go get' should warn about the retracted version.
go get -d
-stderr '^go: warning: example.com/retract/rename@v1.0.0-bad: retracted by module author: bad$'
+stderr '^go: warning: example.com/retract/rename@v1.0.0-bad is retracted: bad$'
# We can't upgrade, since this latest version has a different module path.
! go get -d example.com/retract/rename
