]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go/internal/mvs: retain modules required by older versions
authorBryan C. Mills <bcmills@google.com>
Wed, 17 Jul 2019 16:53:47 +0000 (12:53 -0400)
committerBryan C. Mills <bcmills@google.com>
Thu, 18 Jul 2019 19:42:57 +0000 (19:42 +0000)
Fixes #29773
Updates #31248

Change-Id: Ic1923119c8cf3a60c586df1b270c3af0c9095f29
Reviewed-on: https://go-review.googlesource.com/c/go/+/186537
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
src/cmd/go/internal/mvs/mvs.go
src/cmd/go/internal/mvs/mvs_test.go
src/cmd/go/testdata/script/mod_indirect.txt [new file with mode: 0644]
src/cmd/go/testdata/script/mod_indirect_main.txt [new file with mode: 0644]

index 568efbd8b2663a5c0ea0893c546882c8d05befa6..f9292a05e8eec20bfdbd54fe0f2627e0a378b3a6 100644 (file)
@@ -216,8 +216,8 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) (m
                }
        }
 
-       // Construct the list by traversing the graph again, replacing older
-       // modules with required minimum versions.
+       // The final list is the minimum version of each module found in the graph.
+
        if v := min[target.Path]; v != target.Version {
                // TODO(jayconrod): there is a special case in modload.mvsReqs.Max
                // that prevents us from selecting a newer version of a module
@@ -228,19 +228,18 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) (m
        }
 
        list := []module.Version{target}
-       listed := map[string]bool{target.Path: true}
-       for i := 0; i < len(list); i++ {
-               n := modGraph[list[i]]
+       for path, vers := range min {
+               if path != target.Path {
+                       list = append(list, module.Version{Path: path, Version: vers})
+               }
+
+               n := modGraph[module.Version{Path: path, Version: vers}]
                required := n.required
                for _, r := range required {
                        v := min[r.Path]
                        if r.Path != target.Path && reqs.Max(v, r.Version) != v {
                                panic(fmt.Sprintf("mistake: version %q does not satisfy requirement %+v", v, r)) // TODO: Don't panic.
                        }
-                       if !listed[r.Path] {
-                               list = append(list, module.Version{Path: r.Path, Version: v})
-                               listed[r.Path] = true
-                       }
                }
        }
 
index cab4bb241bc701fcee70af4712e4285db2eef1e8..ea2796699160e5f162d74a08a97e6fef59b441d4 100644 (file)
@@ -29,7 +29,7 @@ D5: E2
 G1: C4
 A2: B1 C4 D4
 build A: A B1 C2 D4 E2 F1
-upgrade* A: A B1 C4 D5 E2 G1
+upgrade* A: A B1 C4 D5 E2 F1 G1
 upgrade A C4: A B1 C4 D4 E2 F1 G1
 downgrade A2 D2: A2 C4 D2
 
@@ -38,7 +38,7 @@ A: B1 C2
 B1: D3
 C2: B2
 B2:
-build A: A B2 C2
+build A: A B2 C2 D3
 
 # Cross-dependency between D and E.
 # No matter how it arises, should get result of merging all build lists via max,
@@ -157,7 +157,18 @@ D1: E2
 E1: D2
 build A: A B C D2 E2
 
-# Upgrade from B1 to B2 should drop the transitive dep on D.
+# golang.org/issue/31248:
+# Even though we select X2, the requirement on I1
+# via X1 should be preserved.
+name: cross8
+M: A1 B1
+A1: X1
+B1: X2
+X1: I1
+X2: 
+build M: M A1 B1 I1 X2
+
+# Upgrade from B1 to B2 should not drop the transitive dep on D.
 name: drop
 A: B1 C1
 B1: D1
@@ -165,14 +176,14 @@ B2:
 C2:
 D2:
 build A: A B1 C1 D1
-upgrade* A: A B2 C2
+upgrade* A: A B2 C2 D2
 
 name: simplify
 A: B1 C1
 B1: C2
 C1: D1
 C2:
-build A: A B1 C2
+build A: A B1 C2 D1
 
 name: up1
 A: B1 C1
@@ -254,8 +265,9 @@ build A: A B1
 upgrade A B2: A B2
 upgrade* A: A B3
 
+# golang.org/issue/29773:
 # Requirements of older versions of the target
-# must not be carried over.
+# must be carried over.
 name: cycle2
 A: B1
 A1: C1
@@ -265,8 +277,8 @@ B2: A2
 C1: A2
 C2:
 D2:
-build A: A B1
-upgrade* A: A B2
+build A: A B1 C1 D1
+upgrade* A: A B2 C2 D2
 
 # Requirement minimization.
 
diff --git a/src/cmd/go/testdata/script/mod_indirect.txt b/src/cmd/go/testdata/script/mod_indirect.txt
new file mode 100644 (file)
index 0000000..87a3f0b
--- /dev/null
@@ -0,0 +1,81 @@
+env GO111MODULE=on
+
+# golang.org/issue/31248: module requirements imposed by dependency versions
+# older than the selected version must still be taken into account.
+
+env GOFLAGS=-mod=readonly
+
+# Indirect dependencies required via older-than-selected versions must exist in
+# the module graph, but do not need to be listed explicitly in the go.mod file
+# (since they are implied).
+go mod graph
+stdout i@v0.1.0
+
+# The modules must also appear in the build list, not just the graph.
+go list -m all
+stdout '^i v0.1.0'
+
+# The packages provided by those dependencies must resolve.
+go list all
+stdout '^i$'
+
+-- go.mod --
+module main
+
+go 1.13
+
+require (
+       a v0.0.0
+       b v0.0.0
+       c v0.0.0
+)
+
+// Apply replacements so that the test can be self-contained.
+// (It's easier to see all of the modules here than to go
+// rooting around in testdata/mod.)
+replace (
+       a => ./a
+       b => ./b
+       c => ./c
+       x v0.1.0 => ./x1
+       x v0.2.0 => ./x2
+       i => ./i
+)
+-- main.go --
+package main
+
+import (
+       _ "a"
+       _ "b"
+       _ "c"
+)
+
+func main() {}
+-- a/go.mod --
+module a
+go 1.13
+require x v0.1.0
+-- a/a.go --
+package a
+-- b/go.mod --
+module b
+go 1.13
+require x v0.2.0
+-- b/b.go --
+package b
+-- c/go.mod --
+module c
+go 1.13
+-- c/c.go --
+package c
+import _ "i"
+-- x1/go.mod --
+module x
+go1.13
+require i v0.1.0
+-- x2/go.mod --
+module x
+go1.13
+-- i/go.mod --
+-- i/i.go --
+package i
diff --git a/src/cmd/go/testdata/script/mod_indirect_main.txt b/src/cmd/go/testdata/script/mod_indirect_main.txt
new file mode 100644 (file)
index 0000000..eeb93f1
--- /dev/null
@@ -0,0 +1,65 @@
+env GO111MODULE=on
+
+# Regression test for golang.org/issue/29773: 'go list -m' was not following
+# dependencies through older versions of the main module.
+
+go list -f '{{with .Module}}{{.Path}}{{with .Version}} {{.}}{{end}}{{end}}' all
+cmp stdout pkgmods.txt
+
+go list -m all
+cmp stdout mods.txt
+
+go mod graph
+cmp stdout graph.txt
+
+-- go.mod --
+module golang.org/issue/root
+
+go 1.12
+
+replace (
+       golang.org/issue/mirror v0.1.0 => ./mirror-v0.1.0
+       golang.org/issue/pkg v0.1.0 => ./pkg-v0.1.0
+       golang.org/issue/root v0.1.0 => ./root-v0.1.0
+)
+
+require golang.org/issue/mirror v0.1.0
+
+-- root.go --
+package root
+
+import _ "golang.org/issue/mirror"
+
+-- mirror-v0.1.0/go.mod --
+module golang.org/issue/mirror
+
+require golang.org/issue/root v0.1.0
+
+-- mirror-v0.1.0/mirror.go --
+package mirror
+
+import _ "golang.org/issue/pkg"
+
+-- pkg-v0.1.0/go.mod --
+module golang.org/issue/pkg
+
+-- pkg-v0.1.0/pkg.go --
+package pkg
+
+-- root-v0.1.0/go.mod --
+module golang.org/issue/root
+
+require golang.org/issue/pkg v0.1.0
+
+-- pkgmods.txt --
+golang.org/issue/mirror v0.1.0
+golang.org/issue/pkg v0.1.0
+golang.org/issue/root
+-- mods.txt --
+golang.org/issue/root
+golang.org/issue/mirror v0.1.0 => ./mirror-v0.1.0
+golang.org/issue/pkg v0.1.0 => ./pkg-v0.1.0
+-- graph.txt --
+golang.org/issue/root golang.org/issue/mirror@v0.1.0
+golang.org/issue/mirror@v0.1.0 golang.org/issue/root@v0.1.0
+golang.org/issue/root@v0.1.0 golang.org/issue/pkg@v0.1.0