]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go/internal/mvs: prune spurious dependencies in Downgrade
authorBryan C. Mills <bcmills@google.com>
Fri, 19 Feb 2021 19:03:45 +0000 (14:03 -0500)
committerBryan C. Mills <bcmills@google.com>
Tue, 2 Mar 2021 17:08:12 +0000 (17:08 +0000)
Previously, mvs.Downgrade could introduce spurious dependencies if the
downgrade computed for one module lands on a “hidden” version (such as
a pseudo-version) due to a requirement introduced by the downgrade for
another module.

To eliminate those spurious dependencies, we can add one more call to
BuildList to recompute the “actual” downgraded versions, and then
including only those actual versions in the final call to BuildList.

For #36460

Change-Id: Icc6b54aa004907221b2bcbbae74598b0e4100776
Reviewed-on: https://go-review.googlesource.com/c/go/+/294294
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>
src/cmd/go/internal/mvs/mvs.go
src/cmd/go/internal/mvs/mvs_test.go
src/cmd/go/testdata/script/mod_get_downup_pseudo_artifact.txt

index ff2c5f963cd88f13baf94066752084e95a637965..e30a40c97e887d88ced0458a3ab617fb32ba6149 100644 (file)
@@ -492,6 +492,41 @@ List:
                downgraded = append(downgraded, r)
        }
 
+       // The downgrades we computed above only downgrade to versions enumerated by
+       // reqs.Previous. However, reqs.Previous omits some versions — such as
+       // pseudo-versions and retracted versions — that may be selected as transitive
+       // requirements of other modules.
+       //
+       // If one of those requirements pulls the version back up above the version
+       // identified by reqs.Previous, then the transitive dependencies of that that
+       // initially-downgraded version should no longer matter — in particular, we
+       // should not add new dependencies on module paths that nothing else in the
+       // updated module graph even requires.
+       //
+       // In order to eliminate those spurious dependencies, we recompute the build
+       // list with the actual versions of the downgraded modules as selected by MVS,
+       // instead of our initial downgrades.
+       // (See the downhiddenartifact and downhiddencross test cases).
+       actual, err := BuildList(target, &override{
+               target: target,
+               list:   downgraded,
+               Reqs:   reqs,
+       })
+       if err != nil {
+               return nil, err
+       }
+       actualVersion := make(map[string]string, len(actual))
+       for _, m := range actual {
+               actualVersion[m.Path] = m.Version
+       }
+
+       downgraded = downgraded[:0]
+       for _, m := range list {
+               if v, ok := actualVersion[m.Path]; ok {
+                       downgraded = append(downgraded, module.Version{Path: m.Path, Version: v})
+               }
+       }
+
        return BuildList(target, &override{
                target: target,
                list:   downgraded,
index 661f68be0840cfeb83023ad07b45cfd568e3f519..598ed666889517a112c01e78d5b6fcb95101fcd2 100644 (file)
@@ -282,8 +282,9 @@ downgrade A B1: A B1
 # And C1 requires B2.hidden, and B2.hidden also meets our requirements:
 # it is compatible with D1 and a strict downgrade from B3.
 #
-# BUG(?): B2.hidden does not require E1, so there is no need for E1
-# to appear in the final build list. Nonetheless, there it is.
+# Since neither the initial nor the final build list includes B1,
+# and the nothing in the final downgraded build list requires E at all,
+# no dependency on E1 (required by only B1) should be introduced.
 #
 name: downhiddenartifact
 A: B3 C2
@@ -298,7 +299,7 @@ D2:
 build A1: A1 B3 D2
 downgrade A1 D1: A1 B1 D1 E1
 build A: A B3 C2 D2
-downgrade A D1: A B2.hidden C1 D1 E1
+downgrade A D1: A B2.hidden C1 D1
 
 # Both B3 and C3 require D2.
 # If we downgrade D to D1, then in isolation B3 would downgrade to B1,
index d773f6bd4dd91830b2393f4f82270de3901704a4..c49615cecb34d22ee114a974af6ecc2469814f4e 100644 (file)
@@ -26,18 +26,15 @@ cp go.mod go.mod.orig
 go mod tidy
 cmp go.mod.orig go.mod
 
+# When we downgrade d.2 to d.1, no dependency on e should be added
+# because nothing else in the module or import graph requires it.
 go get -d example.net/d@v0.1.0
 
 go list -m all
 stdout '^example.net/b v0.2.1-0.20210219000000-000000000000 '
 stdout '^example.net/c v0.1.0 '
 stdout '^example.net/d v0.1.0 '
-
-       # BUG: A dependency on e is added even though nothing requires it.
-stdout '^example.net/e '
-
-go mod why -m example.net/e
-stdout '^\(main module does not need module example.net/e\)'
+! stdout '^example.net/e '
 
 -- go.mod --
 module example.net/a