From 4eca0c73eca109a4fde85fca7b5eac8954c7dfca Mon Sep 17 00:00:00 2001
From: Roger Peppe <rogpeppe@gmail.com>
Date: Mon, 11 Sep 2023 14:06:22 +0100
Subject: [PATCH] internal/mod/mvs: adapt to CUE semantics

Note: experimental feature.

The mvs tests rely on using module.Version in a way that
doesn't conform to the new requirements for that type.
Given that mvs doesn't actually care that it's specifically
dealing with module.Version per se, we make it generic
across any version-like value.
This allows us to avoid rewriting all the (fairly extensive) tests
and seems a reasonably natural fit for the generic nature of
the MVS algorithm itself.

For #2330.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: Ib1b16c3267ac98fde30347f118f0eb6b2764a473
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1168705
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.io>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
---
 internal/mod/module/module.go        |   2 +-
 internal/mod/module/versions.go      |  45 +++++++
 internal/mod/module/versions_test.go |   8 ++
 internal/mod/mvs/errors.go           |  55 ++++----
 internal/mod/mvs/graph.go            | 112 +++++++++++------
 internal/mod/mvs/mvs.go              | 182 ++++++++++++++-------------
 internal/mod/mvs/mvs_test.go         |  73 ++++++-----
 7 files changed, 289 insertions(+), 188 deletions(-)
 create mode 100644 internal/mod/module/versions.go
 create mode 100644 internal/mod/module/versions_test.go

diff --git a/internal/mod/module/module.go b/internal/mod/module/module.go
index 23b9d9a6b..206239d5c 100644
--- a/internal/mod/module/module.go
+++ b/internal/mod/module/module.go
@@ -103,7 +103,7 @@ func MustNewVersion(path string, vers string) Version {
 }
 
 // NewVersion forms a Version from the given path and version.
-// The version must be canonical or empty.
+// The version must be canonical, empty or "none".
 // If the path doesn't have a major version suffix, one will be added
 // if the version isn't empty; if the version is empty, it's an error.
 func NewVersion(path string, vers string) (Version, error) {
diff --git a/internal/mod/module/versions.go b/internal/mod/module/versions.go
new file mode 100644
index 000000000..513e2a3f3
--- /dev/null
+++ b/internal/mod/module/versions.go
@@ -0,0 +1,45 @@
+package module
+
+import (
+	"golang.org/x/mod/semver"
+)
+
+// Versions implements mvs.Versions[Version].
+type Versions struct{}
+
+// New implements mvs.Versions[Version].Version.
+func (Versions) Version(v Version) string {
+	return v.Version()
+}
+
+// New implements mvs.Versions[Version].Path.
+func (Versions) Path(v Version) string {
+	return v.Path()
+}
+
+// New implements mvs.Versions[Version].New.
+func (Versions) New(p, v string) (Version, error) {
+	return NewVersion(p, v)
+}
+
+// Max implements mvs.Reqs.Max.
+//
+// It is consistent with semver.Compare except that as a special case,
+// the version "" is considered higher than all other versions. The main
+// module (also known as the target) has no version and must be chosen
+// over other versions of the same module in the module dependency
+// graph.
+//
+// See [mvs.Reqs] for more detail.
+func (Versions) Max(v1, v2 string) string {
+	if v1 == "none" || v2 == "" {
+		return v2
+	}
+	if v2 == "none" || v1 == "" {
+		return v1
+	}
+	if semver.Compare(v1, v2) > 0 {
+		return v1
+	}
+	return v2
+}
diff --git a/internal/mod/module/versions_test.go b/internal/mod/module/versions_test.go
new file mode 100644
index 000000000..e346d1c2e
--- /dev/null
+++ b/internal/mod/module/versions_test.go
@@ -0,0 +1,8 @@
+package module_test
+
+import (
+	"cuelang.org/go/internal/mod/module"
+	"cuelang.org/go/internal/mod/mvs"
+)
+
+var _ mvs.Versions[module.Version] = module.Versions{}
diff --git a/internal/mod/mvs/errors.go b/internal/mod/mvs/errors.go
index fbdfee5e9..446fe9ee2 100644
--- a/internal/mod/mvs/errors.go
+++ b/internal/mod/mvs/errors.go
@@ -1,5 +1,3 @@
-//go:build ignore
-
 // Copyright 2020 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
@@ -9,20 +7,19 @@ package mvs
 import (
 	"fmt"
 	"strings"
-
-	"golang.org/x/mod/module"
 )
 
 // BuildListError decorates an error that occurred gathering requirements
 // while constructing a build list. BuildListError prints the chain
 // of requirements to the module where the error occurred.
-type BuildListError struct {
+type BuildListError[V comparable] struct {
 	Err   error
-	stack []buildListErrorElem
+	stack []buildListErrorElem[V]
+	vs    Versions[V]
 }
 
-type buildListErrorElem struct {
-	m module.Version
+type buildListErrorElem[V comparable] struct {
+	m V
 
 	// nextReason is the reason this module depends on the next module in the
 	// stack. Typically either "requires", or "updating to".
@@ -37,44 +34,45 @@ type buildListErrorElem struct {
 // explicit upgrade or downgrade (as opposed to an existing requirement in a
 // go.mod file). A nil isVersionChange function indicates that none of the path
 // steps are due to explicit version changes.
-func NewBuildListError(err error, path []module.Version, isVersionChange func(from, to module.Version) bool) *BuildListError {
-	stack := make([]buildListErrorElem, 0, len(path))
+func NewBuildListError[V comparable](err error, path []V, vs Versions[V], isVersionChange func(from, to V) bool) *BuildListError[V] {
+	stack := make([]buildListErrorElem[V], 0, len(path))
 	for len(path) > 1 {
 		reason := "requires"
 		if isVersionChange != nil && isVersionChange(path[0], path[1]) {
 			reason = "updating to"
 		}
-		stack = append(stack, buildListErrorElem{
+		stack = append(stack, buildListErrorElem[V]{
 			m:          path[0],
 			nextReason: reason,
 		})
 		path = path[1:]
 	}
-	stack = append(stack, buildListErrorElem{m: path[0]})
+	stack = append(stack, buildListErrorElem[V]{m: path[0]})
 
-	return &BuildListError{
+	return &BuildListError[V]{
 		Err:   err,
 		stack: stack,
+		vs:    vs,
 	}
 }
 
 // Module returns the module where the error occurred. If the module stack
 // is empty, this returns a zero value.
-func (e *BuildListError) Module() module.Version {
+func (e *BuildListError[V]) Module() V {
 	if len(e.stack) == 0 {
-		return module.Version{}
+		return *new(V)
 	}
 	return e.stack[len(e.stack)-1].m
 }
 
-func (e *BuildListError) Error() string {
+func (e *BuildListError[V]) Error() string {
 	b := &strings.Builder{}
 	stack := e.stack
 
 	// Don't print modules at the beginning of the chain without a
 	// version. These always seem to be the main module or a
 	// synthetic module ("target@").
-	for len(stack) > 0 && stack[0].m.Version == "" {
+	for len(stack) > 0 && e.vs.Version(stack[0].m) == "" {
 		stack = stack[1:]
 	}
 
@@ -82,24 +80,15 @@ func (e *BuildListError) Error() string {
 		b.WriteString(e.Err.Error())
 	} else {
 		for _, elem := range stack[:len(stack)-1] {
-			fmt.Fprintf(b, "%s %s\n\t", elem.m, elem.nextReason)
+			fmt.Fprintf(b, "%v %s\n\t", elem.m, elem.nextReason)
 		}
-		// Ensure that the final module path and version are included as part of the
-		// error message.
 		m := stack[len(stack)-1].m
-		if mErr, ok := e.Err.(*module.ModuleError); ok {
-			actual := module.Version{Path: mErr.Path, Version: mErr.Version}
-			if v, ok := mErr.Err.(*module.InvalidVersionError); ok {
-				actual.Version = v.Version
-			}
-			if actual == m {
-				fmt.Fprintf(b, "%v", e.Err)
-			} else {
-				fmt.Fprintf(b, "%s (replaced by %s): %v", m, actual, mErr.Err)
-			}
-		} else {
-			fmt.Fprintf(b, "%v", module.VersionError(m, e.Err))
-		}
+		fmt.Fprintf(b, "%v: %v", m, e.Err)
+		// TODO the original mvs code was careful to ensure that the final module path
+		// and version were included as part of the error message, but it did that
+		// by checking for mod/module-specific error types, but we don't want this
+		// package to depend on module. We could potentially do it by making those
+		// errors implement interface types defined in this package.
 	}
 	return b.String()
 }
diff --git a/internal/mod/mvs/graph.go b/internal/mod/mvs/graph.go
index fe56888c9..c250ae3f8 100644
--- a/internal/mod/mvs/graph.go
+++ b/internal/mod/mvs/graph.go
@@ -1,5 +1,3 @@
-//go:build ignore
-
 // Copyright 2020 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
@@ -8,22 +6,35 @@ package mvs
 
 import (
 	"fmt"
+	"sort"
 
-	"cuelang.org/go/mod/mvs/internal/slices"
-
-	"golang.org/x/mod/module"
+	"cuelang.org/go/internal/mod/mvs/internal/slices"
 )
 
+// Versions is an interface that should be provided by implementations
+// to define the mvs algorithm in terms of their own version type V, where
+// a version type holds a (module path, module version) pair.
+type Versions[V any] interface {
+	// New creates a new instance of V holding the
+	// given module path and version.
+	New(path, version string) (V, error)
+	// Path returns the path part of V.
+	Path(v V) string
+	// Version returns the version part of V.
+	Version(v V) string
+}
+
 // Graph implements an incremental version of the MVS algorithm, with the
 // requirements pushed by the caller instead of pulled by the MVS traversal.
-type Graph struct {
+type Graph[V comparable] struct {
+	v     Versions[V]
 	cmp   func(v1, v2 string) int
-	roots []module.Version
+	roots []V
 
-	required map[module.Version][]module.Version
+	required map[V][]V
 
-	isRoot   map[module.Version]bool // contains true for roots and false for reachable non-roots
-	selected map[string]string       // path → version
+	isRoot   map[V]bool        // contains true for roots and false for reachable non-roots
+	selected map[string]string // path → version
 }
 
 // NewGraph returns an incremental MVS graph containing only a set of root
@@ -31,19 +42,20 @@ type Graph struct {
 //
 // The caller must ensure that the root slice is not modified while the Graph
 // may be in use.
-func NewGraph(cmp func(v1, v2 string) int, roots []module.Version) *Graph {
-	g := &Graph{
+func NewGraph[V comparable](v Versions[V], cmp func(string, string) int, roots []V) *Graph[V] {
+	g := &Graph[V]{
+		v:        v,
 		cmp:      cmp,
 		roots:    slices.Clip(roots),
-		required: make(map[module.Version][]module.Version),
-		isRoot:   make(map[module.Version]bool),
+		required: make(map[V][]V),
+		isRoot:   make(map[V]bool),
 		selected: make(map[string]string),
 	}
 
 	for _, m := range roots {
 		g.isRoot[m] = true
-		if g.cmp(g.Selected(m.Path), m.Version) < 0 {
-			g.selected[m.Path] = m.Version
+		if g.cmp(g.Selected(g.v.Path(m)), g.v.Version(m)) < 0 {
+			g.selected[g.v.Path(m)] = g.v.Version(m)
 		}
 	}
 
@@ -58,7 +70,7 @@ func NewGraph(cmp func(v1, v2 string) int, roots []module.Version) *Graph {
 //
 // If any of the modules in reqs has the same path as g's target,
 // the target must have higher precedence than the version in req.
-func (g *Graph) Require(m module.Version, reqs []module.Version) {
+func (g *Graph[V]) Require(m V, reqs []V) {
 	// To help catch disconnected-graph bugs, enforce that all required versions
 	// are actually reachable from the roots (and therefore should affect the
 	// selected versions of the modules they name).
@@ -81,8 +93,8 @@ func (g *Graph) Require(m module.Version, reqs []module.Version) {
 			g.isRoot[dep] = false
 		}
 
-		if g.cmp(g.Selected(dep.Path), dep.Version) < 0 {
-			g.selected[dep.Path] = dep.Version
+		if g.cmp(g.Selected(g.v.Path(dep)), g.v.Version(dep)) < 0 {
+			g.selected[g.v.Path(dep)] = g.v.Version(dep)
 		}
 	}
 }
@@ -93,7 +105,7 @@ func (g *Graph) Require(m module.Version, reqs []module.Version) {
 //
 // The caller must not modify the returned slice, but may safely append to it
 // and may rely on it not to be modified.
-func (g *Graph) RequiredBy(m module.Version) (reqs []module.Version, ok bool) {
+func (g *Graph[V]) RequiredBy(m V) (reqs []V, ok bool) {
 	reqs, ok = g.required[m]
 	return reqs, ok
 }
@@ -101,7 +113,7 @@ func (g *Graph) RequiredBy(m module.Version) (reqs []module.Version, ok bool) {
 // Selected returns the selected version of the given module path.
 //
 // If no version is selected, Selected returns version "none".
-func (g *Graph) Selected(path string) (version string) {
+func (g *Graph[V]) Selected(path string) (version string) {
 	v, ok := g.selected[path]
 	if !ok {
 		return "none"
@@ -114,12 +126,12 @@ func (g *Graph) Selected(path string) (version string) {
 //
 // The order of the remaining elements in the list is deterministic
 // but arbitrary.
-func (g *Graph) BuildList() []module.Version {
+func (g *Graph[V]) BuildList() []V {
 	seenRoot := make(map[string]bool, len(g.roots))
 
-	var list []module.Version
+	var list []V
 	for _, r := range g.roots {
-		if seenRoot[r.Path] {
+		if seenRoot[g.v.Path(r)] {
 			// Multiple copies of the same root, with the same or different versions,
 			// are a bit of a degenerate case: we will take the transitive
 			// requirements of both roots into account, but only the higher one can
@@ -129,31 +141,51 @@ func (g *Graph) BuildList() []module.Version {
 			continue
 		}
 
-		if v := g.Selected(r.Path); v != "none" {
-			list = append(list, module.Version{Path: r.Path, Version: v})
+		if v := g.Selected(g.v.Path(r)); v != "none" {
+			list = append(list, g.newVersion(g.v.Path(r), v))
 		}
-		seenRoot[r.Path] = true
+		seenRoot[g.v.Path(r)] = true
 	}
 	uniqueRoots := list
 
 	for path, version := range g.selected {
 		if !seenRoot[path] {
-			list = append(list, module.Version{Path: path, Version: version})
+			list = append(list, g.newVersion(path, version))
 		}
 	}
-	module.Sort(list[len(uniqueRoots):])
-
+	g.sortVersions(list[len(uniqueRoots):])
 	return list
 }
 
+func (g *Graph[V]) sortVersions(vs []V) {
+	sort.Slice(vs, func(i, j int) bool {
+		v0, v1 := vs[i], vs[j]
+		if p0, p1 := g.v.Path(v0), g.v.Path(v1); p0 != p1 {
+			return p0 < p1
+		}
+		return g.cmp(g.v.Version(v0), g.v.Version(v1)) < 0
+	})
+}
+
+func (g *Graph[V]) newVersion(path string, vers string) V {
+	v, err := g.v.New(path, vers)
+	if err != nil {
+		// Note: can't happen because all paths and versions passed to
+		// g.newVersion have already come from valid paths and versions
+		// returned from a Versions implementation.
+		panic(err)
+	}
+	return v
+}
+
 // WalkBreadthFirst invokes f once, in breadth-first order, for each module
 // version other than "none" that appears in the graph, regardless of whether
 // that version is selected.
-func (g *Graph) WalkBreadthFirst(f func(m module.Version)) {
-	var queue []module.Version
-	enqueued := make(map[module.Version]bool)
+func (g *Graph[V]) WalkBreadthFirst(f func(m V)) {
+	var queue []V
+	enqueued := make(map[V]bool)
 	for _, m := range g.roots {
-		if m.Version != "none" {
+		if g.v.Version(m) != "none" {
 			queue = append(queue, m)
 			enqueued[m] = true
 		}
@@ -167,7 +199,7 @@ func (g *Graph) WalkBreadthFirst(f func(m module.Version)) {
 
 		reqs, _ := g.RequiredBy(m)
 		for _, r := range reqs {
-			if !enqueued[r] && r.Version != "none" {
+			if !enqueued[r] && g.v.Version(r) != "none" {
 				queue = append(queue, r)
 				enqueued[r] = true
 			}
@@ -178,14 +210,14 @@ func (g *Graph) WalkBreadthFirst(f func(m module.Version)) {
 // FindPath reports a shortest requirement path starting at one of the roots of
 // the graph and ending at a module version m for which f(m) returns true, or
 // nil if no such path exists.
-func (g *Graph) FindPath(f func(module.Version) bool) []module.Version {
+func (g *Graph[V]) FindPath(f func(V) bool) []V {
 	// firstRequires[a] = b means that in a breadth-first traversal of the
 	// requirement graph, the module version a was first required by b.
-	firstRequires := make(map[module.Version]module.Version)
+	firstRequires := make(map[V]V)
 
 	queue := g.roots
 	for _, m := range g.roots {
-		firstRequires[m] = module.Version{}
+		firstRequires[m] = *new(V)
 	}
 
 	for len(queue) > 0 {
@@ -195,10 +227,10 @@ func (g *Graph) FindPath(f func(module.Version) bool) []module.Version {
 		if f(m) {
 			// Construct the path reversed (because we're starting from the far
 			// endpoint), then reverse it.
-			path := []module.Version{m}
+			path := []V{m}
 			for {
 				m = firstRequires[m]
-				if m.Path == "" {
+				if g.v.Path(m) == "" {
 					break
 				}
 				path = append(path, m)
diff --git a/internal/mod/mvs/mvs.go b/internal/mod/mvs/mvs.go
index 3fcdc8c78..299b28eac 100644
--- a/internal/mod/mvs/mvs.go
+++ b/internal/mod/mvs/mvs.go
@@ -1,14 +1,9 @@
-//go:build ignore
-
 // Copyright 2018 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 // Package mvs implements Minimal Version Selection.
 // See https://research.swtch.com/vgo-mvs.
-//
-// THIS IS AN EXPERIMENTAL API. DO NOT IMPORT: THE
-// API MAY CHANGE WITHOUT WARNING.
 package mvs
 
 import (
@@ -17,25 +12,25 @@ import (
 	"sort"
 	"sync"
 
-	"cuelang.org/go/mod/mvs/internal/par"
-
-	"golang.org/x/mod/module"
+	"cuelang.org/go/internal/mod/mvs/internal/par"
 )
 
 // A Reqs is the requirement graph on which Minimal Version Selection (MVS) operates.
 //
 // The version strings are opaque except for the special version "none"
-// (see the documentation for module.Version). In particular, MVS does not
+// (see the documentation for V). In particular, MVS does not
 // assume that the version strings are semantic versions; instead, the Max method
 // gives access to the comparison operation.
 //
 // It must be safe to call methods on a Reqs from multiple goroutines simultaneously.
 // Because a Reqs may read the underlying graph from the network on demand,
 // the MVS algorithms parallelize the traversal to overlap network delays.
-type Reqs interface {
+type Reqs[V comparable] interface {
+	Versions[V]
+
 	// Required returns the module versions explicitly required by m itself.
 	// The caller must not modify the returned list.
-	Required(m module.Version) ([]module.Version, error)
+	Required(m V) ([]V, error)
 
 	// Max returns the maximum of v1 and v2 (it returns either v1 or v2).
 	//
@@ -49,8 +44,8 @@ type Reqs interface {
 }
 
 // An UpgradeReqs is a Reqs that can also identify available upgrades.
-type UpgradeReqs interface {
-	Reqs
+type UpgradeReqs[V comparable] interface {
+	Reqs[V]
 
 	// Upgrade returns the upgraded version of m,
 	// for use during an UpgradeAll operation.
@@ -63,16 +58,16 @@ type UpgradeReqs interface {
 	// Upgrade returns a non-nil error.
 	// TODO(rsc): Upgrade must be able to return errors,
 	// but should "no latest version" just return m instead?
-	Upgrade(m module.Version) (module.Version, error)
+	Upgrade(m V) (V, error)
 }
 
 // A DowngradeReqs is a Reqs that can also identify available downgrades.
-type DowngradeReqs interface {
-	Reqs
+type DowngradeReqs[V comparable] interface {
+	Reqs[V]
 
 	// Previous returns the version of m.Path immediately prior to m.Version,
 	// or "none" if no such version is known.
-	Previous(m module.Version) (module.Version, error)
+	Previous(m V) (V, error)
 }
 
 // BuildList returns the build list for the target module.
@@ -91,11 +86,11 @@ type DowngradeReqs interface {
 // of the list are sorted by path.
 //
 // See https://research.swtch.com/vgo-mvs for details.
-func BuildList(targets []module.Version, reqs Reqs) ([]module.Version, error) {
+func BuildList[V comparable](targets []V, reqs Reqs[V]) ([]V, error) {
 	return buildList(targets, reqs, nil)
 }
 
-func buildList(targets []module.Version, reqs Reqs, upgrade func(module.Version) (module.Version, error)) ([]module.Version, error) {
+func buildList[V comparable](targets []V, reqs Reqs[V], upgrade func(V) (V, error)) ([]V, error) {
 	cmp := func(v1, v2 string) int {
 		if reqs.Max(v1, v2) != v1 {
 			return -1
@@ -108,22 +103,22 @@ func buildList(targets []module.Version, reqs Reqs, upgrade func(module.Version)
 
 	var (
 		mu       sync.Mutex
-		g        = NewGraph(cmp, targets)
-		upgrades = map[module.Version]module.Version{}
-		errs     = map[module.Version]error{} // (non-nil errors only)
+		g        = NewGraph(Versions[V](reqs), cmp, targets)
+		upgrades = map[V]V{}
+		errs     = map[V]error{} // (non-nil errors only)
 	)
 
 	// Explore work graph in parallel in case reqs.Required
 	// does high-latency network operations.
-	var work par.Work[module.Version]
+	var work par.Work[V]
 	for _, target := range targets {
 		work.Add(target)
 	}
-	work.Do(10, func(m module.Version) {
+	work.Do(10, func(m V) {
 
-		var required []module.Version
+		var required []V
 		var err error
-		if m.Version != "none" {
+		if reqs.Version(m) != "none" {
 			required, err = reqs.Required(m)
 		}
 
@@ -143,7 +138,7 @@ func buildList(targets []module.Version, reqs Reqs, upgrade func(module.Version)
 		}
 		if u != m {
 			upgrades[m] = u
-			required = append([]module.Version{u}, required...)
+			required = append([]V{u}, required...)
 		}
 		g.Require(m, required)
 		mu.Unlock()
@@ -156,7 +151,7 @@ func buildList(targets []module.Version, reqs Reqs, upgrade func(module.Version)
 	// If there was an error, find the shortest path from the target to the
 	// node where the error occurred so we can report a useful error message.
 	if len(errs) > 0 {
-		errPath := g.FindPath(func(m module.Version) bool {
+		errPath := g.FindPath(func(m V) bool {
 			return errs[m] != nil
 		})
 		if len(errPath) == 0 {
@@ -164,13 +159,13 @@ func buildList(targets []module.Version, reqs Reqs, upgrade func(module.Version)
 		}
 
 		err := errs[errPath[len(errPath)-1]]
-		isUpgrade := func(from, to module.Version) bool {
+		isUpgrade := func(from, to V) bool {
 			if u, ok := upgrades[from]; ok {
 				return u == to
 			}
 			return false
 		}
-		return nil, NewBuildListError(err, errPath, isUpgrade)
+		return nil, NewBuildListError(err, errPath, g.v, isUpgrade)
 	}
 
 	// The final list is the minimum version of each module found in the graph.
@@ -188,8 +183,8 @@ func buildList(targets []module.Version, reqs Reqs, upgrade func(module.Version)
 // Req returns the minimal requirement list for the target module,
 // with the constraint that all module paths listed in base must
 // appear in the returned list.
-func Req(mainModule module.Version, base []string, reqs Reqs) ([]module.Version, error) {
-	list, err := BuildList([]module.Version{mainModule}, reqs)
+func Req[V comparable](mainModule V, base []string, reqs Reqs[V]) ([]V, error) {
+	list, err := BuildList([]V{mainModule}, reqs)
 	if err != nil {
 		return nil, err
 	}
@@ -200,16 +195,16 @@ func Req(mainModule module.Version, base []string, reqs Reqs) ([]module.Version,
 
 	max := map[string]string{}
 	for _, m := range list {
-		max[m.Path] = m.Version
+		max[reqs.Path(m)] = reqs.Version(m)
 	}
 
 	// Compute postorder, cache requirements.
-	var postorder []module.Version
-	reqCache := map[module.Version][]module.Version{}
+	var postorder []V
+	reqCache := map[V][]V{}
 	reqCache[mainModule] = nil
 
-	var walk func(module.Version) error
-	walk = func(m module.Version) error {
+	var walk func(V) error
+	walk = func(m V) error {
 		_, ok := reqCache[m]
 		if ok {
 			return nil
@@ -234,8 +229,8 @@ func Req(mainModule module.Version, base []string, reqs Reqs) ([]module.Version,
 	}
 
 	// Walk modules in reverse post-order, only adding those not implied already.
-	have := map[module.Version]bool{}
-	walk = func(m module.Version) error {
+	have := map[V]bool{}
+	walk = func(m V) error {
 		if have[m] {
 			return nil
 		}
@@ -246,13 +241,17 @@ func Req(mainModule module.Version, base []string, reqs Reqs) ([]module.Version,
 		return nil
 	}
 	// First walk the base modules that must be listed.
-	var min []module.Version
+	var min []V
 	haveBase := map[string]bool{}
 	for _, path := range base {
 		if haveBase[path] {
 			continue
 		}
-		m := module.Version{Path: path, Version: max[path]}
+		m, err := reqs.New(path, max[path])
+		if err != nil {
+			// Can't happen because arguments to New above are known to be OK.
+			panic(err)
+		}
 		min = append(min, m)
 		walk(m)
 		haveBase[path] = true
@@ -260,7 +259,7 @@ func Req(mainModule module.Version, base []string, reqs Reqs) ([]module.Version,
 	// Now the reverse postorder to bring in anything else.
 	for i := len(postorder) - 1; i >= 0; i-- {
 		m := postorder[i]
-		if max[m.Path] != m.Version {
+		if max[reqs.Path(m)] != reqs.Version(m) {
 			// Older version.
 			continue
 		}
@@ -270,16 +269,16 @@ func Req(mainModule module.Version, base []string, reqs Reqs) ([]module.Version,
 		}
 	}
 	sort.Slice(min, func(i, j int) bool {
-		return min[i].Path < min[j].Path
+		return reqs.Path(min[i]) < reqs.Path(min[j])
 	})
 	return min, nil
 }
 
 // UpgradeAll returns a build list for the target module
 // in which every module is upgraded to its latest version.
-func UpgradeAll(target module.Version, reqs UpgradeReqs) ([]module.Version, error) {
-	return buildList([]module.Version{target}, reqs, func(m module.Version) (module.Version, error) {
-		if m.Path == target.Path {
+func UpgradeAll[V comparable](target V, reqs UpgradeReqs[V]) ([]V, error) {
+	return buildList([]V{target}, Reqs[V](reqs), func(m V) (V, error) {
+		if reqs.Path(m) == reqs.Path(target) {
 			return target, nil
 		}
 
@@ -289,7 +288,7 @@ func UpgradeAll(target module.Version, reqs UpgradeReqs) ([]module.Version, erro
 
 // Upgrade returns a build list for the target module
 // in which the given additional modules are upgraded.
-func Upgrade(target module.Version, reqs UpgradeReqs, upgrade ...module.Version) ([]module.Version, error) {
+func Upgrade[V comparable](target V, reqs UpgradeReqs[V], upgrade ...V) ([]V, error) {
 	list, err := reqs.Required(target)
 	if err != nil {
 		return nil, err
@@ -297,25 +296,30 @@ func Upgrade(target module.Version, reqs UpgradeReqs, upgrade ...module.Version)
 
 	pathInList := make(map[string]bool, len(list))
 	for _, m := range list {
-		pathInList[m.Path] = true
+		pathInList[reqs.Path(m)] = true
 	}
-	list = append([]module.Version(nil), list...)
+	list = append([]V(nil), list...)
 
 	upgradeTo := make(map[string]string, len(upgrade))
 	for _, u := range upgrade {
-		if !pathInList[u.Path] {
-			list = append(list, module.Version{Path: u.Path, Version: "none"})
+		if !pathInList[reqs.Path(u)] {
+			newv, err := reqs.New(reqs.Path(u), "none")
+			if err != nil {
+				// Can't happen because arguments to New above are known to be OK.
+				panic(err)
+			}
+			list = append(list, newv)
 		}
-		if prev, dup := upgradeTo[u.Path]; dup {
-			upgradeTo[u.Path] = reqs.Max(prev, u.Version)
+		if prev, dup := upgradeTo[reqs.Path(u)]; dup {
+			upgradeTo[reqs.Path(u)] = reqs.Max(prev, reqs.Version(u))
 		} else {
-			upgradeTo[u.Path] = u.Version
+			upgradeTo[reqs.Path(u)] = reqs.Version(u)
 		}
 	}
 
-	return buildList([]module.Version{target}, &override{target, list, reqs}, func(m module.Version) (module.Version, error) {
-		if v, ok := upgradeTo[m.Path]; ok {
-			return module.Version{Path: m.Path, Version: v}, nil
+	return buildList[V]([]V{target}, &override[V]{target, list, reqs}, func(m V) (V, error) {
+		if v, ok := upgradeTo[reqs.Path(m)]; ok {
+			return reqs.New(reqs.Path(m), v)
 		}
 		return m, nil
 	})
@@ -328,7 +332,7 @@ func Upgrade(target module.Version, reqs UpgradeReqs, upgrade ...module.Version)
 // The versions to be downgraded may be unreachable from reqs.Latest and
 // reqs.Previous, but the methods of reqs must otherwise handle such versions
 // correctly.
-func Downgrade(target module.Version, reqs DowngradeReqs, downgrade ...module.Version) ([]module.Version, error) {
+func Downgrade[V comparable](target V, reqs DowngradeReqs[V], downgrade ...V) ([]V, error) {
 	// Per https://research.swtch.com/vgo-mvs#algorithm_4:
 	// “To avoid an unnecessary downgrade to E 1.1, we must also add a new
 	// requirement on E 1.2. We can apply Algorithm R to find the minimal set of
@@ -336,7 +340,7 @@ func Downgrade(target module.Version, reqs DowngradeReqs, downgrade ...module.Ve
 	//
 	// In order to generate those new requirements, we need to identify versions
 	// for every module in the build list — not just reqs.Required(target).
-	list, err := BuildList([]module.Version{target}, reqs)
+	list, err := BuildList[V]([]V{target}, reqs)
 	if err != nil {
 		return nil, err
 	}
@@ -344,21 +348,21 @@ func Downgrade(target module.Version, reqs DowngradeReqs, downgrade ...module.Ve
 
 	max := make(map[string]string)
 	for _, r := range list {
-		max[r.Path] = r.Version
+		max[reqs.Path(r)] = reqs.Version(r)
 	}
 	for _, d := range downgrade {
-		if v, ok := max[d.Path]; !ok || reqs.Max(v, d.Version) != d.Version {
-			max[d.Path] = d.Version
+		if v, ok := max[reqs.Path(d)]; !ok || reqs.Max(v, reqs.Version(d)) != reqs.Version(d) {
+			max[reqs.Path(d)] = reqs.Version(d)
 		}
 	}
 
 	var (
-		added    = make(map[module.Version]bool)
-		rdeps    = make(map[module.Version][]module.Version)
-		excluded = make(map[module.Version]bool)
+		added    = make(map[V]bool)
+		rdeps    = make(map[V][]V)
+		excluded = make(map[V]bool)
 	)
-	var exclude func(module.Version)
-	exclude = func(m module.Version) {
+	var exclude func(V)
+	exclude = func(m V) {
 		if excluded[m] {
 			return
 		}
@@ -367,13 +371,13 @@ func Downgrade(target module.Version, reqs DowngradeReqs, downgrade ...module.Ve
 			exclude(p)
 		}
 	}
-	var add func(module.Version)
-	add = func(m module.Version) {
+	var add func(V)
+	add = func(m V) {
 		if added[m] {
 			return
 		}
 		added[m] = true
-		if v, ok := max[m.Path]; ok && reqs.Max(m.Version, v) != v {
+		if v, ok := max[reqs.Path(m)]; ok && reqs.Max(reqs.Version(m), v) != v {
 			// m would upgrade an existing dependency — it is not a strict downgrade,
 			// and because it was already present as a dependency, it could affect the
 			// behavior of other relevant packages.
@@ -405,7 +409,7 @@ func Downgrade(target module.Version, reqs DowngradeReqs, downgrade ...module.Ve
 		}
 	}
 
-	downgraded := make([]module.Version, 0, len(list)+1)
+	downgraded := make([]V, 0, len(list)+1)
 	downgraded = append(downgraded, target)
 List:
 	for _, r := range list {
@@ -424,10 +428,15 @@ List:
 			// included when iterating over prior versions using reqs.Previous.
 			// Insert it into the right place in the iteration.
 			// If v is excluded, p should be returned again by reqs.Previous on the next iteration.
-			if v := max[r.Path]; reqs.Max(v, r.Version) != v && reqs.Max(p.Version, v) != p.Version {
-				p.Version = v
+			if v := max[reqs.Path(r)]; reqs.Max(v, reqs.Version(r)) != v && reqs.Max(reqs.Version(p), v) != reqs.Version(p) {
+				p0, err := reqs.New(reqs.Path(p), v)
+				if err != nil {
+					// Can't happen because arguments to New above are known to be OK.
+					panic(err)
+				}
+				p = p0
 			}
-			if p.Version == "none" {
+			if reqs.Version(p) == "none" {
 				continue List
 			}
 			add(p)
@@ -451,7 +460,7 @@ List:
 	// 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([]module.Version{target}, &override{
+	actual, err := BuildList[V]([]V{target}, &override[V]{
 		target: target,
 		list:   downgraded,
 		Reqs:   reqs,
@@ -461,30 +470,35 @@ List:
 	}
 	actualVersion := make(map[string]string, len(actual))
 	for _, m := range actual {
-		actualVersion[m.Path] = m.Version
+		actualVersion[reqs.Path(m)] = reqs.Version(m)
 	}
 
 	downgraded = downgraded[:0]
 	for _, m := range list {
-		if v, ok := actualVersion[m.Path]; ok {
-			downgraded = append(downgraded, module.Version{Path: m.Path, Version: v})
+		if v, ok := actualVersion[reqs.Path(m)]; ok {
+			m1, err := reqs.New(reqs.Path(m), v)
+			if err != nil {
+				// Can't happen because arguments to New above are known to be OK.
+				panic(err)
+			}
+			downgraded = append(downgraded, m1)
 		}
 	}
 
-	return BuildList([]module.Version{target}, &override{
+	return BuildList[V]([]V{target}, &override[V]{
 		target: target,
 		list:   downgraded,
 		Reqs:   reqs,
 	})
 }
 
-type override struct {
-	target module.Version
-	list   []module.Version
-	Reqs
+type override[V comparable] struct {
+	target V
+	list   []V
+	Reqs[V]
 }
 
-func (r *override) Required(m module.Version) ([]module.Version, error) {
+func (r *override[V]) Required(m V) ([]V, error) {
 	if m == r.target {
 		return r.list, nil
 	}
diff --git a/internal/mod/mvs/mvs_test.go b/internal/mod/mvs/mvs_test.go
index d441c1ed2..f9e99770b 100644
--- a/internal/mod/mvs/mvs_test.go
+++ b/internal/mod/mvs/mvs_test.go
@@ -1,5 +1,3 @@
-//go:build ignore
-
 // Copyright 2018 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
@@ -11,8 +9,6 @@ import (
 	"reflect"
 	"strings"
 	"testing"
-
-	"golang.org/x/mod/module"
 )
 
 var tests = `
@@ -459,17 +455,17 @@ func Test(t *testing.T) {
 			})
 		}
 	}
-	m := func(s string) module.Version {
-		return module.Version{Path: s[:1], Version: s[1:]}
+	m := func(s string) version {
+		return version{s[:1], s[1:]}
 	}
-	ms := func(list []string) []module.Version {
-		var mlist []module.Version
+	ms := func(list []string) []version {
+		var mlist []version
 		for _, s := range list {
 			mlist = append(mlist, m(s))
 		}
 		return mlist
 	}
-	checkList := func(t *testing.T, desc string, list []module.Version, err error, val string) {
+	checkList := func(t *testing.T, desc string, list []version, err error, val string) {
 		if err != nil {
 			t.Fatalf("%s: %v", desc, err)
 		}
@@ -509,7 +505,7 @@ func Test(t *testing.T) {
 				t.Fatalf("build takes one argument: %q", line)
 			}
 			fns = append(fns, func(t *testing.T) {
-				list, err := BuildList([]module.Version{m(kf[1])}, reqs)
+				list, err := BuildList[version]([]version{m(kf[1])}, reqs)
 				checkList(t, key, list, err, val)
 			})
 			continue
@@ -518,7 +514,7 @@ func Test(t *testing.T) {
 				t.Fatalf("upgrade* takes one argument: %q", line)
 			}
 			fns = append(fns, func(t *testing.T) {
-				list, err := UpgradeAll(m(kf[1]), reqs)
+				list, err := UpgradeAll[version](m(kf[1]), reqs)
 				checkList(t, key, list, err, val)
 			})
 			continue
@@ -527,7 +523,7 @@ func Test(t *testing.T) {
 				t.Fatalf("upgrade takes at least one argument: %q", line)
 			}
 			fns = append(fns, func(t *testing.T) {
-				list, err := Upgrade(m(kf[1]), reqs, ms(kf[2:])...)
+				list, err := Upgrade[version](m(kf[1]), reqs, ms(kf[2:])...)
 				if err == nil {
 					// Copy the reqs map, but substitute the upgraded requirements in
 					// place of the target's original requirements.
@@ -537,7 +533,7 @@ func Test(t *testing.T) {
 					}
 					upReqs[m(kf[1])] = list
 
-					list, err = Req(m(kf[1]), nil, upReqs)
+					list, err = Req[version](m(kf[1]), nil, upReqs)
 				}
 				checkList(t, key, list, err, val)
 			})
@@ -547,7 +543,7 @@ func Test(t *testing.T) {
 				t.Fatalf("upgrade takes at least one argument: %q", line)
 			}
 			fns = append(fns, func(t *testing.T) {
-				list, err := Upgrade(m(kf[1]), reqs, ms(kf[2:])...)
+				list, err := Upgrade[version](m(kf[1]), reqs, ms(kf[2:])...)
 				checkList(t, key, list, err, val)
 			})
 			continue
@@ -556,7 +552,7 @@ func Test(t *testing.T) {
 				t.Fatalf("downgrade takes at least one argument: %q", line)
 			}
 			fns = append(fns, func(t *testing.T) {
-				list, err := Downgrade(m(kf[1]), reqs, ms(kf[1:])...)
+				list, err := Downgrade[version](m(kf[1]), reqs, ms(kf[1:])...)
 				checkList(t, key, list, err, val)
 			})
 			continue
@@ -565,17 +561,17 @@ func Test(t *testing.T) {
 				t.Fatalf("req takes at least one argument: %q", line)
 			}
 			fns = append(fns, func(t *testing.T) {
-				list, err := Req(m(kf[1]), kf[2:], reqs)
+				list, err := Req[version](m(kf[1]), kf[2:], reqs)
 				checkList(t, key, list, err, val)
 			})
 			continue
 		}
 		if len(kf) == 1 && 'A' <= key[0] && key[0] <= 'Z' {
-			var rs []module.Version
+			var rs []version
 			for _, f := range strings.Fields(val) {
 				r := m(f)
 				if reqs[r] == nil {
-					reqs[r] = []module.Version{}
+					reqs[r] = []version{}
 				}
 				rs = append(rs, r)
 			}
@@ -587,7 +583,19 @@ func Test(t *testing.T) {
 	flush()
 }
 
-type reqsMap map[module.Version][]module.Version
+type reqsMap map[version][]version
+
+func (r reqsMap) Path(v version) string {
+	return v.path
+}
+
+func (r reqsMap) Version(v version) string {
+	return v.version
+}
+
+func (r reqsMap) New(p, v string) (version, error) {
+	return version{p, v}, nil
+}
 
 func (r reqsMap) Max(v1, v2 string) string {
 	if v1 == "none" || v2 == "" {
@@ -602,36 +610,41 @@ func (r reqsMap) Max(v1, v2 string) string {
 	return v1
 }
 
-func (r reqsMap) Upgrade(m module.Version) (module.Version, error) {
-	u := module.Version{Version: "none"}
+func (r reqsMap) Upgrade(m version) (version, error) {
+	u := version{"", "none"}
 	for k := range r {
-		if k.Path == m.Path && r.Max(u.Version, k.Version) == k.Version && !strings.HasSuffix(k.Version, ".hidden") {
+		if k.path == m.path && r.Max(u.version, k.version) == k.version && !strings.HasSuffix(k.version, ".hidden") {
 			u = k
 		}
 	}
-	if u.Path == "" {
-		return module.Version{}, fmt.Errorf("missing module: %v", module.Version{Path: m.Path})
+	if u.path == "" {
+		return version{}, fmt.Errorf("missing module: %v", m.path)
 	}
 	return u, nil
 }
 
-func (r reqsMap) Previous(m module.Version) (module.Version, error) {
-	var p module.Version
+func (r reqsMap) Previous(m version) (version, error) {
+	var p version
 	for k := range r {
-		if k.Path == m.Path && p.Version < k.Version && k.Version < m.Version && !strings.HasSuffix(k.Version, ".hidden") {
+		if k.path == m.path && p.version < k.version && k.version < m.version && !strings.HasSuffix(k.version, ".hidden") {
 			p = k
 		}
 	}
-	if p.Path == "" {
-		return module.Version{Path: m.Path, Version: "none"}, nil
+	if p.path == "" {
+		return version{m.path, "none"}, nil
 	}
 	return p, nil
 }
 
-func (r reqsMap) Required(m module.Version) ([]module.Version, error) {
+func (r reqsMap) Required(m version) ([]version, error) {
 	rr, ok := r[m]
 	if !ok {
 		return nil, fmt.Errorf("missing module: %v", m)
 	}
 	return rr, nil
 }
+
+type version struct {
+	path    string
+	version string
+}
-- 
2.51.2