[release-branch.go1.17] cmd/go/internal/modload: scan dependencies of root paths when raising version limits in editRequirements
Updates #47979
Fixes #48156
Change-Id: I1d9d854cda1378e20c70e6c6789b77e42e467ca7
Reviewed-on: https://go-review.googlesource.com/c/go/+/347290
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>
(cherry picked from commit 409434d62364cb362f0f17d0c7769dc680b2da99)
Reviewed-on: https://go-review.googlesource.com/c/go/+/348411
diff --git a/src/cmd/go/internal/modload/edit.go b/src/cmd/go/internal/modload/edit.go
index dfc366d..47f236c 100644
--- a/src/cmd/go/internal/modload/edit.go
+++ b/src/cmd/go/internal/modload/edit.go
@@ -190,8 +190,8 @@
// 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.
+// (individually or in any combination) 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
@@ -213,18 +213,42 @@
}
}
- var eagerUpgrades []module.Version
+ var (
+ eagerUpgrades []module.Version
+ isLazyRootPath map[string]bool
+ )
if depth == eager {
eagerUpgrades = tryUpgrade
} else {
+ isLazyRootPath = make(map[string]bool, len(maxVersion))
+ for p := range maxVersion {
+ isLazyRootPath[p] = true
+ }
for _, m := range tryUpgrade {
+ isLazyRootPath[m.Path] = true
+ }
+ for _, m := range mustSelect {
+ isLazyRootPath[m.Path] = true
+ }
+
+ allowedRoot := map[module.Version]bool{}
+
+ var allowRoot func(m module.Version) error
+ allowRoot = func(m module.Version) error {
+ if allowedRoot[m] {
+ return nil
+ }
+ allowedRoot[m] = true
+
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
+ return nil
}
+ allow(m)
+
summary, err := goModSummary(m)
if err != nil {
return err
@@ -234,13 +258,27 @@
// graph, rather than loading the (potentially-overlapping) subgraph for
// each upgrade individually.
eagerUpgrades = append(eagerUpgrades, m)
- continue
+ return nil
}
-
- allow(m)
for _, r := range summary.require {
- allow(r)
+ if isLazyRootPath[r.Path] {
+ // r could become a root as the result of an upgrade or downgrade,
+ // in which case its dependencies will not be pruned out.
+ // We need to allow those dependencies to be upgraded too.
+ if err := allowRoot(r); err != nil {
+ return err
+ }
+ } else {
+ // r will not become a root, so its dependencies don't matter.
+ // Allow only r itself.
+ allow(r)
+ }
}
+ return nil
+ }
+
+ for _, m := range tryUpgrade {
+ allowRoot(m)
}
}
@@ -269,16 +307,41 @@
}
}
- if len(mustSelect) > 0 {
- mustGraph, err := readModGraph(ctx, depth, mustSelect)
+ // Explicitly allow any (transitive) upgrades implied by mustSelect.
+ nextRoots := append([]module.Version(nil), mustSelect...)
+ for nextRoots != nil {
+ module.Sort(nextRoots)
+ rs := newRequirements(depth, nextRoots, nil)
+ nextRoots = nil
+
+ rs, mustGraph, err := expandGraph(ctx, rs)
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.
+ // Some module in mustSelect requires r, so we must allow at least
+ // r.Version (unless it conflicts with another entry in mustSelect, in
+ // which case we will error out either way).
allow(r)
+
+ if isLazyRootPath[r.Path] {
+ if v, ok := rs.rootSelected(r.Path); ok && r.Version == v {
+ // r is already a root, so its requirements are already included in
+ // the build list.
+ continue
+ }
+
+ // The dependencies in mustSelect may upgrade (or downgrade) an existing
+ // root to match r, which will remain as a root. However, since r is not
+ // a root of rs, its dependencies have been pruned out of this build
+ // list. We need to add it back explicitly so that we allow any
+ // transitive upgrades that r will pull in.
+ if nextRoots == nil {
+ nextRoots = rs.rootModules // already capped
+ }
+ nextRoots = append(nextRoots, r)
+ }
}
}
diff --git a/src/cmd/go/testdata/script/mod_get_issue47979.txt b/src/cmd/go/testdata/script/mod_get_issue47979.txt
new file mode 100644
index 0000000..f5d4304
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_issue47979.txt
@@ -0,0 +1,117 @@
+# Regression test for https://golang.org/issue/47979:
+#
+# An argument to 'go get' that results in an upgrade to a different existing
+# root should be allowed, and should not panic the 'go' command.
+
+cp go.mod go.mod.orig
+
+
+# Transitive upgrades from upgraded roots should not prevent
+# 'go get -u' from performing upgrades.
+
+cp go.mod.orig go.mod
+go get -u -d .
+cmp go.mod go.mod.want
+
+
+# 'go get' of a specific version should allow upgrades of
+# every dependency (transitively) required by that version,
+# including dependencies that are pulled into the module
+# graph by upgrading other root requirements
+# (in this case, example.net/indirect).
+
+cp go.mod.orig go.mod
+go get -d example.net/a@v0.2.0
+cmp go.mod go.mod.want
+
+
+-- go.mod --
+module golang.org/issue47979
+
+go 1.17
+
+replace (
+ example.net/a v0.1.0 => ./a1
+ example.net/a v0.2.0 => ./a2
+ example.net/indirect v0.1.0 => ./indirect1
+ example.net/indirect v0.2.0 => ./indirect2
+ example.net/other v0.1.0 => ./other
+ example.net/other v0.2.0 => ./other
+)
+
+require (
+ example.net/a v0.1.0
+ example.net/other v0.1.0
+)
+
+require example.net/indirect v0.1.0 // indirect
+-- go.mod.want --
+module golang.org/issue47979
+
+go 1.17
+
+replace (
+ example.net/a v0.1.0 => ./a1
+ example.net/a v0.2.0 => ./a2
+ example.net/indirect v0.1.0 => ./indirect1
+ example.net/indirect v0.2.0 => ./indirect2
+ example.net/other v0.1.0 => ./other
+ example.net/other v0.2.0 => ./other
+)
+
+require (
+ example.net/a v0.2.0
+ example.net/other v0.2.0
+)
+
+require example.net/indirect v0.2.0 // indirect
+-- issue.go --
+package issue
+
+import _ "example.net/a"
+-- useother/useother.go --
+package useother
+
+import _ "example.net/other"
+-- a1/go.mod --
+module example.net/a
+
+go 1.17
+
+require example.net/indirect v0.1.0
+-- a1/a.go --
+package a
+-- a2/go.mod --
+module example.net/a
+
+go 1.17
+
+require example.net/indirect v0.2.0
+-- a2/a.go --
+package a
+
+import "example.net/indirect"
+-- indirect1/go.mod --
+module example.net/indirect
+
+go 1.17
+
+require example.net/other v0.1.0
+-- indirect1/indirect.go --
+package indirect
+-- indirect2/go.mod --
+module example.net/indirect
+
+go 1.17
+
+require example.net/other v0.2.0
+-- indirect2/indirect.go --
+package indirect
+
+import "example.net/other"
+-- other/go.mod --
+module example.net/other
+
+go 1.17
+-- other/other.go --
+package other
