cmd/go/internal/modload: in newRequirements, verify that rootModules is sorted

The comment for the Requirements.rootModules field requires that it be
"sorted and capped to length". I noticed that we were not capping it
correctly — we were capping the local variable (the rorotModules
argument itself) but not the struct field. That prompted me to
question whether we were also at some point failing to sort it
correctly, so I decided to add an explicit check.

With the explicit check, all tests continue to pass.

For #36460

Change-Id: I6687de8ef8ecc5129fa8810d678e5673752fd27b
Reviewed-on: https://go-review.googlesource.com/c/go/+/310790
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>
Reviewed-by: Michael Matloob <matloob@golang.org>

diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go
index 07d9fdfc54107ab00e33034b3390a391bceb91d6..8be5cc2a1bebf4f3fc174982fe5debbf92e6f28b 100644 (file)
--- a/src/cmd/go/internal/modload/buildlist.go
+++ b/src/cmd/go/internal/modload/buildlist.go
@@ -19,6 +19,7 @@ import (
        "sync/atomic"
 
        "golang.org/x/mod/module"
+       "golang.org/x/mod/semver"
 )
 
 // capVersionSlice returns s with its cap reduced to its length.
@@ -89,6 +90,7 @@ var requirements *Requirements
 // The dependencies of the roots will be loaded lazily at the first call to the
 // Graph method.
 //
+// The rootModules slice must be sorted according to module.Sort.
 // The caller must not modify the rootModules slice or direct map after passing
 // them to newRequirements.
 //
@@ -102,15 +104,20 @@ func newRequirements(depth modDepth, rootModules []module.Version, direct map[st
                if m.Path == "" || m.Version == "" {
                        panic(fmt.Sprintf("bad requirement: rootModules[%v] = %v", i, m))
                }
+               if i > 0 {
+                       prev := rootModules[i-1]
+                       if prev.Path > m.Path || (prev.Path == m.Path && semver.Compare(prev.Version, m.Version) > 0) {
+                               panic(fmt.Sprintf("newRequirements called with unsorted roots: %v", rootModules))
+                       }
+               }
        }
 
        rs := &Requirements{
                depth:          depth,
-               rootModules:    rootModules,
+               rootModules:    capVersionSlice(rootModules),
                maxRootVersion: make(map[string]string, len(rootModules)),
                direct:         direct,
        }
-       rootModules = capVersionSlice(rootModules)
 
        for _, m := range rootModules {
                if v, ok := rs.maxRootVersion[m.Path]; ok && cmpVersion(v, m.Version) >= 0 {