Skip to content

Commit abdaa0f

Browse files
rscgopherbot
authored andcommitted
cmd/go: make toolchain less special in MVS
We were using the omission of toolchain from the MVS graph as a signal that toolchain was not mentioned on the go get line, but not including it in the graph causes various problems, and it may be reintroduced to the graph during operations like pruning conversion, after which its presence is not a good signal about whether it was mentioned on the go get command line. Fix all this irregularity by explicitly telling WriteGoMod whether the command line mentioned toolchain instead. For #57001. Change-Id: I74084637c177c30918fdb114a0d9030cdee7324e Reviewed-on: https://go-review.googlesource.com/c/go/+/499575 TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Russ Cox <[email protected]> Run-TryBot: Russ Cox <[email protected]> Reviewed-by: Bryan Mills <[email protected]>
1 parent 1079a5c commit abdaa0f

File tree

6 files changed

+207
-157
lines changed

6 files changed

+207
-157
lines changed

src/cmd/go/internal/modget/get.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,14 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
304304
}
305305

306306
dropToolchain, queries := parseArgs(ctx, args)
307+
opts := modload.WriteOpts{
308+
DropToolchain: dropToolchain,
309+
}
310+
for _, q := range queries {
311+
if q.pattern == "toolchain" {
312+
opts.ExplicitToolchain = true
313+
}
314+
}
307315

308316
r := newResolver(ctx, queries)
309317
r.performLocalQueries(ctx)
@@ -372,14 +380,10 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
372380
}
373381
r.checkPackageProblems(ctx, pkgPatterns)
374382

375-
if dropToolchain {
376-
modload.OverrideRoots(ctx, []module.Version{{Path: "toolchain", Version: "none"}})
377-
}
378-
379383
// Everything succeeded. Update go.mod.
380384
oldReqs := reqsFromGoMod(modload.ModFile())
381385

382-
if err := modload.WriteGoMod(ctx, modload.WriteOpts{}); err != nil {
386+
if err := modload.WriteGoMod(ctx, opts); err != nil {
383387
if tooNew, ok := err.(*gover.TooNewError); ok {
384388
// This can happen for 'go get go@newversion'
385389
// when all the required modules are old enough

src/cmd/go/internal/modload/buildlist.go

+49-16
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,24 @@
55
package modload
66

77
import (
8-
"cmd/go/internal/base"
9-
"cmd/go/internal/cfg"
10-
"cmd/go/internal/gover"
11-
"cmd/go/internal/mvs"
12-
"cmd/go/internal/par"
13-
"cmd/go/internal/slices"
148
"context"
159
"errors"
1610
"fmt"
1711
"os"
1812
"reflect"
1913
"runtime"
2014
"runtime/debug"
15+
"slices"
2116
"strings"
2217
"sync"
2318
"sync/atomic"
2419

20+
"cmd/go/internal/base"
21+
"cmd/go/internal/cfg"
22+
"cmd/go/internal/gover"
23+
"cmd/go/internal/mvs"
24+
"cmd/go/internal/par"
25+
2526
"golang.org/x/mod/module"
2627
)
2728

@@ -91,6 +92,15 @@ type cachedGraph struct {
9192
// accept and/or return an explicit parameter.
9293
var requirements *Requirements
9394

95+
func mustHaveGoRoot(roots []module.Version) {
96+
for _, m := range roots {
97+
if m.Path == "go" {
98+
return
99+
}
100+
}
101+
panic("go: internal error: missing go root module")
102+
}
103+
94104
// newRequirements returns a new requirement set with the given root modules.
95105
// The dependencies of the roots will be loaded lazily at the first call to the
96106
// Graph method.
@@ -102,6 +112,8 @@ var requirements *Requirements
102112
// If vendoring is in effect, the caller must invoke initVendor on the returned
103113
// *Requirements before any other method.
104114
func newRequirements(pruning modPruning, rootModules []module.Version, direct map[string]bool) *Requirements {
115+
mustHaveGoRoot(rootModules)
116+
105117
if pruning == workspace {
106118
return &Requirements{
107119
pruning: pruning,
@@ -114,25 +126,28 @@ func newRequirements(pruning modPruning, rootModules []module.Version, direct ma
114126
if workFilePath != "" && pruning != workspace {
115127
panic("in workspace mode, but pruning is not workspace in newRequirements")
116128
}
117-
118129
for i, m := range rootModules {
119130
if m.Version == "" && MainModules.Contains(m.Path) {
120131
panic(fmt.Sprintf("newRequirements called with untrimmed build list: rootModules[%v] is a main module", i))
121132
}
122133
if m.Path == "" || m.Version == "" {
123134
panic(fmt.Sprintf("bad requirement: rootModules[%v] = %v", i, m))
124135
}
125-
if i > 0 {
126-
prev := rootModules[i-1]
127-
if prev.Path > m.Path || (prev.Path == m.Path && gover.ModCompare(m.Path, prev.Version, m.Version) > 0) {
128-
panic(fmt.Sprintf("newRequirements called with unsorted roots: %v", rootModules))
129-
}
130-
}
136+
}
137+
138+
// Allow unsorted root modules, because go and toolchain
139+
// are treated as the final graph roots but not trimmed from the build list,
140+
// so they always appear at the beginning of the list.
141+
r := slices.Clip(slices.Clone(rootModules))
142+
gover.ModSort(r)
143+
if !reflect.DeepEqual(r, rootModules) {
144+
fmt.Fprintln(os.Stderr, "RM", rootModules)
145+
panic("unsorted")
131146
}
132147

133148
rs := &Requirements{
134149
pruning: pruning,
135-
rootModules: slices.Clip(rootModules),
150+
rootModules: rootModules,
136151
maxRootVersion: make(map[string]string, len(rootModules)),
137152
direct: direct,
138153
}
@@ -157,7 +172,7 @@ func (rs *Requirements) String() string {
157172
func (rs *Requirements) initVendor(vendorList []module.Version) {
158173
rs.graphOnce.Do(func() {
159174
mg := &ModuleGraph{
160-
g: mvs.NewGraph(cmpVersion, MainModules.GraphRoots()),
175+
g: mvs.NewGraph(cmpVersion, MainModules.Versions()),
161176
}
162177

163178
if MainModules.Len() != 1 {
@@ -278,6 +293,7 @@ var readModGraphDebugOnce sync.Once
278293
// Unlike LoadModGraph, readModGraph does not attempt to diagnose or update
279294
// inconsistent roots.
280295
func readModGraph(ctx context.Context, pruning modPruning, roots []module.Version, unprune map[module.Version]bool) (*ModuleGraph, error) {
296+
mustHaveGoRoot(roots)
281297
if pruning == pruned {
282298
// Enable diagnostics for lazy module loading
283299
// (https://golang.org/ref/mod#lazy-loading) only if the module graph is
@@ -301,13 +317,20 @@ func readModGraph(ctx context.Context, pruning modPruning, roots []module.Versio
301317
})
302318
}
303319

320+
var graphRoots []module.Version
321+
if inWorkspaceMode() {
322+
graphRoots = roots
323+
} else {
324+
graphRoots = MainModules.Versions()
325+
}
304326
var (
305327
mu sync.Mutex // guards mg.g and hasError during loading
306328
hasError bool
307329
mg = &ModuleGraph{
308-
g: mvs.NewGraph(cmpVersion, MainModules.GraphRoots()),
330+
g: mvs.NewGraph(cmpVersion, graphRoots),
309331
}
310332
)
333+
311334
if pruning != workspace {
312335
if inWorkspaceMode() {
313336
panic("pruning is not workspace in workspace mode")
@@ -380,6 +403,7 @@ func readModGraph(ctx context.Context, pruning modPruning, roots []module.Versio
380403
})
381404
}
382405

406+
mustHaveGoRoot(roots)
383407
for _, m := range roots {
384408
enqueue(m, pruning)
385409
}
@@ -789,6 +813,10 @@ func tidyPrunedRoots(ctx context.Context, mainModule module.Version, old *Requir
789813
roots = append(roots, module.Version{Path: "go", Version: v})
790814
pathIsRoot["go"] = true
791815
}
816+
if v, ok := old.rootSelected("toolchain"); ok {
817+
roots = append(roots, module.Version{Path: "toolchain", Version: v})
818+
pathIsRoot["toolchain"] = true
819+
}
792820
// We start by adding roots for every package in "all".
793821
//
794822
// Once that is done, we may still need to add more roots to cover upgraded or
@@ -1254,6 +1282,11 @@ func tidyUnprunedRoots(ctx context.Context, mainModule module.Version, old *Requ
12541282
)
12551283
if v, ok := old.rootSelected("go"); ok {
12561284
keep = append(keep, module.Version{Path: "go", Version: v})
1285+
keptPath["go"] = true
1286+
}
1287+
if v, ok := old.rootSelected("toolchain"); ok {
1288+
keep = append(keep, module.Version{Path: "toolchain", Version: v})
1289+
keptPath["toolchain"] = true
12571290
}
12581291
for _, pkg := range pkgs {
12591292
if !pkg.fromExternalModule() {

0 commit comments

Comments
 (0)